cmd: Add kraft setup command #257

StefanJum · 2023-03-04T08:18:59Z

The kraft setup command will install the required dependencies for building and running unikernels. It will also check the virtualization support of the environment, check if we are running kraft from inside a conainter or a virtual machine and give a warning about enabling nested virtualization

Prerequisite checklist

Read the contribution guidelines regarding submitting new changes to the project;
Tested your changes against relevant architectures and platforms;
Ran make fmt on your commit series before opening this PR;
Updated relevant documentation.

Description of changes

craciunoiuc · 2023-03-04T09:36:53Z

conainter -> container in the commit message 😄

StefanJum · 2023-03-04T09:43:08Z

conainter -> container in the commit message smile

Done, sorry 😆

craciunoiuc

Hey @StefanJum

Added some quick notes from my side.

One thing I don't get here is what setup should be doing.

I was under the impression that it should set up the environment, like the config options and other os-specific options.

The installation I thought was handled by the install script:
https://get.kraftkit.sh

Also tagging @nderjung

cmd/kraft/kraft.go

cmd/kraft/setup/setup.go

craciunoiuc · 2023-03-04T09:39:08Z

cmd/kraft/setup/setup.go

+	req := []string{
+		"build-essential",
+		"libncurses-dev",
+		"libyaml-dev",
+		"flex",
+		"git",
+		"wget",
+		"socat",
+		"bison",
+		"unzip",
+		"uuid-runtime",
+		"python3",
+		"python3-setuptools",
+		"python3-pip",
+		"qemu-kvm",
+		"qemu-system-x86",
+		"sgabios",
+	}


I think maybe we should restrict those by versions? Not sure.

At least a lower-bound we should set

Are these requirements actually necessary? I believe the list is now much smaller now. Plus this should be "switchable" the user may not necessarily want qemu :) so a VMM option, determination, should append this or prompt for it

cmd/kraft/setup/setup.go

StefanJum · 2023-03-04T09:50:02Z

Hey @StefanJum

Added some quick notes from my side.

One thing I don't get here is what setup should be doing.

I was under the impression that it should set up the environment, like the config options and other os-specific options.

The installation I thought was handled by the install script: https://get.kraftkit.sh

Also tagging @nderjung

Thanks, I'll check the comments.

From what I understood kraft setup should prepare the environment for building and running unikernels.
That would mean installing the dependencies and detecting the virtualization support (xen, kvm etc.) in order to use that when configuring/running the applications.
I think the best way to do that is by saving the found virtualization solution in the kraftkit config file, but the installation of all requirements would still be necessary as a first step.

Does the install script add all the requirements for building and running unikernels or just for building/running kraft?

@nderjung please correct me if I'm wrong.

craciunoiuc · 2023-03-04T09:58:54Z

Actually you are right.

What I wanted to say is that maybe we should be kind-of merging these two together somehow.

From my perspective when you first run a tool you should have all dependencies already solved. I would just add an extra check in the install script to ask "Do you want to install building and running dependencies"

I'm fine either way. Let's see what Alex has to say

nderjung

Hi @StefanJum. Thanks for putting this together.

To be inline with how logic is approached in the cmd/ directory, each subcommand program essentially calls functionality exposed by the "KraftKit framework". I think the setup cmd should be similar and therefore the logic for actually performing the checks, running any installation, etc. should occur as an exposed API from a different package. The goal here is to keep the file cmd/setup/setup.go as simple as possible.

I suggest re-working the logic into a new package, kraftkit.sh/internal/setup. as is suggested by the corresponding issue for this feature.

Towards a re-usable API, i'd recommend starting simply and with something like the following:

cmd/setup/setup.go:

import "kraftkit.sh/internal/setup"

type Setup struct {
  OS   string `long:"os" usage:"Force the host OS"`
  Arch string `long:"arch" usage:"Force the architecture of the host"`
  VMM  string `long:"vmm" usage:"Force the use of a specific VMM on the host"`
  PM   string `long:"pm" usage:"Force the use of a specific package manager on the host"`
}

// [...]

var sopts []setup.SetupOption

if opts.OS != "" {
  sopts = append(sopts, setup.WithOS(opts.WithOS))
}

// [...etc...]

if err := setup.Setup(ctx, ...sopts); err != nil {
  return err
}

internal/setup/setup.go:

type Setup struct {
  arch *HostArch
  os   *HostOS
  pm   *HostPackageManager
  vmm  *HostVMM
}

func Setup(ctx context.Context, sopts ...SetupOption) error {
  s := Setup{}
  for _, opts := range sopts {
    if err := opt(&s); err != nil {
      return err
    }
  }

  var err error

  if s.arch == nil {
    s.arch, err = DetectHostArch()
    if err != nil {
      return err
    }
  }
    
  // [...]
}

internal/setup/options.go:

// [...]

type SetupOption func(*Setup) error

func WithOS(os string) SetupOption {
 // This should check if the provided `os` variable is valid in format and
 // in support. E.g. providing `ubuntu` and `linux/ubuntu` should work,
 // but perhaps providing `windows`, `windows/choco` would not.
}

Additionally, each os/distribution should be a "provider" following an implementation to the following abstract interface:

type HostPackageManager interface {
  Install(context.Context, ...string) error
}

The above interface needs a bit more thought, as @craciunoiuc has mentioned, because versions need to be listed. I can imagine the following could be a way to provide the listing:

internal/setup/setup_ubuntu.go:

var DistUbuntu = [...]HostPackage{
  HostPackage{"package",  GT, "1.0.0"},
};

That said, I would be surprised if there were no existing Go packages that do not do something similar to what we're trying to achieve.

Additional comments:

Please can you use the updated copyright preamble, here is an example of what it looks like;
Use iostreams.G(ctx).Out when redirecting information to the user via fmt.Fprintf, this is the global access point to os.Stdout, which keeps its configuration and possible redirection globally configurable;
Check config.G(ctx).NoPrompt to determine whether to prompt the user for questions, e.g. Would you like to continue? [Yn];
Use kraftkit.sh/exec for executing commands and not the built-in os/exec package.

nderjung · 2023-03-05T23:26:28Z

To address the comments by @craciunoiuc, the get.kraftkit.sh program can and should recommend additional programs through the relevant package manager, e.g. "APT recommends". Whilst this is the recommended method for installation and any dependencies, a built-in command, kraft setup, can not only determine if and which binaries are necessary for a unikraft build, e.g. we can essentially run "which" internally, but should also help and aid in "setting up" the host environment. Along these lines, we can additionally check the host for virtualization features.

In my mind, following an installation from get.kraftkit.sh, would be an immediate execution of kraft setup, which guides the user through host-setup. In a non-standard install, e.g. by pulling the binary directly from the releases page, the kraft setup command enables access to such installation.

I also imagine an extension to kraft setup in the future which would guide the user through setting up the global configuration with potential values. We can detect a "first installation" by whether the configuration file has been written. We could inject inside of cmd/kraft/kraft.go a check, for example:

import "kraftkit.sh/internal/setup"

// [...]

func main() {
  // [...]
  
  if isFirstSetup, _ := setup.IsFirstUse(ctx); isFirstSetup {
    if err := setup.Setup(ctx); err != nil {
      fmt.Printf("could not perform first-use setup: %v", err)
      os.Exit(1)
    }
  }

  // [...]

  cmdfactory.Main(ctx)
}

craciunoiuc · 2023-03-06T08:52:16Z

Yes, you are right. I forgot that many people actually pull the binary from the releases page.

The `kraft setup` command will install the required dependencies for building and running unikernels. It will also check the virtualization support of the environment, check if we are running kraft from inside a container or a virtual machine and give a warning about enabling nested virtualization Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com>

StefanJum · 2023-03-15T11:48:21Z

@nderjung I did a quick refactoring based on your comments, let me know how it looks.

nderjung

Hi @StefanJum, thanks for the update, I can see this is still WIP, however, see comments inline.

nderjung · 2023-03-15T12:39:51Z

cmd/kraft/setup/setup.go

+	WithOS   string `long:"with-os" usage:"Force the host OS"`
+	WithArch string `long:"with-arch" usage:"Force the architecture of the host"`
+	WithVMM  string `long:"with-vmm" usage:"Force the use of a specific VMM on the host"`
+	WithPM   string `long:"with-pm" usage:"Force the use of a specific package manager on the host"`


- WithOS string `long:"with-os" usage:"Force the host OS"` - WithArch string `long:"with-arch" usage:"Force the architecture of the host"` - WithVMM string `long:"with-vmm" usage:"Force the use of a specific VMM on the host"` - WithPM string `long:"with-pm" usage:"Force the use of a specific package manager on the host"` + OS string `long:"os" usage:"Force the host OS"` + Arch string `long:"arch" usage:"Force the architecture of the host"` + VMM string `long:"vmm" usage:"Force the use of a specific VMM on the host"` + PM string `long:"pm" usage:"Force the use of a specific package manager on the host"`

nderjung · 2023-03-15T12:40:04Z

cmd/kraft/setup/setup.go

+	return cmdfactory.New(&Setup{}, cobra.Command{
+		Short:   "Setup the working environment for building and running unikernels",
+		Use:     "setup",
+		Aliases: []string{"sup"},


- Aliases: []string{"sup"},

nderjung · 2023-03-15T12:40:55Z

cmd/kraft/setup/setup.go

+		Short:   "Setup the working environment for building and running unikernels",
+		Use:     "setup",
+		Aliases: []string{"sup"},
+		Args:    cmdfactory.MaxDirArgs(0),


- Args: cmdfactory.MaxDirArgs(0), + Args: cobra. NoArgs(),

nderjung · 2023-03-15T12:41:47Z

cmd/kraft/setup/setup.go

+			# Setup the environment
+			$ kraft setup`),
+		Annotations: map[string]string{
+			"help:group": "build",


- "help:group": "build", + cmdfactory.AnnotationHelpGroup: "build",

nderjung · 2023-03-15T12:43:01Z

cmd/kraft/setup/setup.go

+		sopts = append(sopts, setup.WithOS(opts.WithOS))
+	} else {
+		sopts = append(sopts, setup.WithDetectHostOS())
+	}


- sopts = append(sopts, setup.WithOS(opts.WithOS)) - } else { - sopts = append(sopts, setup.WithDetectHostOS()) - } + sopts = append(sopts, setup.WithOS(opts.OS)) + }

nderjung · 2023-03-15T12:46:55Z

internal/setup/setup.go

+	WithOS   string `long:"with-os" usage:"Force the host OS"`
+	WithArch string `long:"with-arch" usage:"Force the architecture of the host"`
+	WithVMM  string `long:"with-vmm" usage:"Force the use of a specific VMM on the host"`
+	WithPM   string `long:"with-pm" usage:"Force the use of a specific package manager on the host"`


- WithOS string `long:"with-os" usage:"Force the host OS"` - WithArch string `long:"with-arch" usage:"Force the architecture of the host"` - WithVMM string `long:"with-vmm" usage:"Force the use of a specific VMM on the host"` - WithPM string `long:"with-pm" usage:"Force the use of a specific package manager on the host"` + os string + arch string + vmm string + pm string

nderjung · 2023-03-15T12:48:03Z

internal/setup/setup.go

+func DoSetup(ctx context.Context, sopts ...SetupOption) error {
+	s := Setup{}
+	for _, opts := range sopts {


- func DoSetup(ctx context.Context, sopts ...SetupOption) error { + func Setup(ctx context.Context, sopts ...SetupOption) error { s := Setup{} + for _, opts := range sopts {

nderjung · 2023-03-15T12:51:06Z

internal/setup/options.go

+func detect_arch() string {
+	os_info, _ := host.Info()
+	arch := os_info.KernelArch
+
+	return arch
+}
+
+func detect_vmm() string {
+	os_virt, role, _ := host.Virtualization()
+	vmm := os_virt + "/" + role
+
+	return vmm
+}


Functions in Go are never snake_case and are instead camelBack.

Additionally, the host.Virtualization() method is a nice find. Perhaps it's best to serialize all possible outputs into a global list of supported values.

nderjung · 2023-03-15T12:53:08Z

internal/setup/options.go

+func NotSupported(*Setup) error {
+	return nil
+}


- func NotSupported(*Setup) error { - return nil - } + const ( + ErrNotSupportedVMM = errors.New("virtualization system not supported") + ErrNotSupportedPM = errors.New("package manager not supported") + ErrNotSupportedOS = errors.New("OS not supported") + )

(and place at the top of the file)

nderjung · 2023-03-15T12:53:46Z

cmd/kraft/kraft.go

@@ -77,6 +78,7 @@ func New() *cobra.Command {
 	cmd.AddCommand(ps.New())
 	cmd.AddCommand(rm.New())
 	cmd.AddCommand(run.New())
+	cmd.AddCommand(setup.New())


Move this to the misc section

StefanJum requested review from nderjung and craciunoiuc March 4, 2023 08:19

StefanJum force-pushed the StefanJum/add-setup-cmd branch from a3aaa9b to 603dcb3 Compare March 4, 2023 09:42

craciunoiuc requested changes Mar 4, 2023

View reviewed changes

StefanJum force-pushed the StefanJum/add-setup-cmd branch 3 times, most recently from ee6a206 to 2c3d9f3 Compare March 4, 2023 09:57

StefanJum force-pushed the StefanJum/add-setup-cmd branch from 2c3d9f3 to b15cabe Compare March 4, 2023 10:02

nderjung requested changes Mar 5, 2023

View reviewed changes

StefanJum force-pushed the StefanJum/add-setup-cmd branch from b15cabe to 4014baa Compare March 11, 2023 13:14

StefanJum requested a review from nderjung March 15, 2023 11:48

nderjung requested changes Mar 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd: Add kraft setup command #257

cmd: Add kraft setup command #257

StefanJum commented Mar 4, 2023

craciunoiuc commented Mar 4, 2023

StefanJum commented Mar 4, 2023

craciunoiuc left a comment

craciunoiuc Mar 4, 2023

nderjung Mar 5, 2023

StefanJum commented Mar 4, 2023

craciunoiuc commented Mar 4, 2023

nderjung left a comment •

edited

nderjung commented Mar 5, 2023 •

edited

craciunoiuc commented Mar 6, 2023

StefanJum commented Mar 15, 2023

nderjung left a comment •

edited

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023

nderjung Mar 15, 2023 •

edited

nderjung Mar 15, 2023

cmd: Add kraft setup command #257

Are you sure you want to change the base?

cmd: Add kraft setup command #257

Conversation

StefanJum commented Mar 4, 2023

Prerequisite checklist

Description of changes

craciunoiuc commented Mar 4, 2023

StefanJum commented Mar 4, 2023

craciunoiuc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanJum commented Mar 4, 2023

craciunoiuc commented Mar 4, 2023

nderjung left a comment • edited

Choose a reason for hiding this comment

nderjung commented Mar 5, 2023 • edited

craciunoiuc commented Mar 6, 2023

StefanJum commented Mar 15, 2023

nderjung left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nderjung Mar 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nderjung left a comment •

edited

nderjung commented Mar 5, 2023 •

edited

nderjung left a comment •

edited

nderjung Mar 15, 2023 •

edited