Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kata-manager: Allow hypervisor to be changed #9059

Merged

Conversation

jodh-intel
Copy link
Contributor

@jodh-intel jodh-intel commented Feb 8, 2024

kata-manager: Allow hypervisor to be changed

Add new options to allow the configured hypervisor to be changed:

  • -L: List available packaged hypervisor config names (filename fragments).
  • -e: List available local hypervisor config names (filename fragments).
  • -H <hypervisor>: Install Kata then switch to the specified hypervisor (by filename fragment).
  • -S <hypervisor>: Switch to the specified hypervisor (by filename fragment [Errors if Kata not installed]).

For example, to install Kata and configure it to use Cloud Hypervisor
with the golang Kata runtime:

$ kata-manager.sh -H clh

To switch back to the default hypervisor:

$ kata-manager.sh -S default

To show details of the available packaged configs:

$ kata-manager.sh -L

To show details of the local configs:

$ kata-manager.sh -e

Notes:

  • This change only applies to the current default (golang) Kata runtime.

  • Although this is mainly for users wishing to switch hypervisor (by
    changing the Kata config file to another of the packaged config files
    provided for specific hypervisors), strictly it allows users to change
    to any config file. For example, if the user has a config file called
    /etc/kata-containers/configuration-my-custom-config.toml, they could
    switch to this by running:

    $ kata-manager.sh -S my-custom-config

Fixes: #8305.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

@katacontainersbot katacontainersbot added the size/large Task of significant size label Feb 8, 2024
@jodh-intel
Copy link
Contributor Author

Note that running with -L won't work unless you are using a very new installation of Kata (newer than the official 3.3.0-alpha0 release: https://github.com/kata-containers/kata-containers/releases. The reason being that #8483 isn't yet in any official releases.

/cc @fidencio, @amshinde, @cmaf, @dborquez.

@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 5c84b0c to a45b9a5 Compare February 9, 2024 11:17
@katacontainersbot katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/large Task of significant size labels Feb 9, 2024
@jodh-intel
Copy link
Contributor Author

Two thoughts:

  • I don't particularly like the option "names": -L and -e are related but you'd never know from the option letters. Maybe we should switch to using long options with getopt(1) for greater clarity?
  • We could drop one of -H or -S and just have a single option that will dtrt. Thoughts?

@jodh-intel
Copy link
Contributor Author

I've updated the branch today to add in -e (show local config details). But the change also removes the hard-coded default hypervisor from the script which now derives it from the packaged config file directory.

@jodh-intel
Copy link
Contributor Author

As demonstrated by the doc changes on this PR, using the kata-manager script from GitHub is unwieldy. I've raised #9066 so we can consider adding it to the release.

@jodh-intel jodh-intel requested a review from a team as a code owner February 9, 2024 16:45
@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch 3 times, most recently from 94056f5 to 3dcd534 Compare February 9, 2024 17:08
Copy link
Contributor

@cmaf cmaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jodh-intel this LGTM although I have not tested it locally

@jodh-intel
Copy link
Contributor Author

Thanks, @cmaf!

@jodh-intel this LGTM although I have not tested it locally

What you can do is run the code using curl. For example, to see the help statement:

$ bash -c "$(curl -fsSL https://raw.githubusercontent.com/kata-containers/kata-containers/3dcd5341637fd60d7b9f86602acb0312dcf005d0/utils/kata-manager.sh) -h"

Note: The commit in the curl URL is the latest commit currently on this PR (3dcd534).

See https://github.com/kata-containers/kata-containers/blob/main/utils/README.md for other options not added on this PR.

@jodh-intel jodh-intel added safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test labels Feb 15, 2024
@jodh-intel
Copy link
Contributor Author

/test

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jodh-intel, I have a few suggestions, none of those are code related so far.
Please, let me know your thoights.


### Choose a Hypervisor

Kata works with different [hypervisors](../docs/hypervisors.md). When you install a Kata system, the default hypervisor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the motivation on linking to the hypervisors page, but the user will not know which name to use to select, for instance, "Cloud Hypervisor".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a way to have the list of supported hypervisors being displayed without installing kata.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not explained in the commits, but I tried to minimise what we hard-code in the script as it's better to rely on the structure of the packaged config files to determine the logic. Further, it will simplify our lives when we tackle #8677 and #9060.

I've now updated the branch so that the hypervisors.md page shows the short names, which allows the users to determine the names used by the script.

$ bash -c "$(curl -fsSL https://raw.githubusercontent.com/kata-containers/kata-containers/main/utils/kata-manager.sh) -H clh"
```

> **Note:** See the [List available hypervisors](#list-available-hypervisors) section
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note is not that much useful as we don't have a way to actually list the hypervisors before we install Kata Containers. :-/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc now lists the "short names".

However, I am wondering about removing the -H option since, effectively, that option just installs Kata, then calls -S. There isn't much benefit to providing -H since we still need to install Kata. Then the -S just does the config switch. wdyt to removing -H?

utils/README.md Outdated
Comment on lines 76 to 144
#### List available hypervisors

Run the following on an installed system:

```bash
$ bash -c "$(curl -fsSL https://raw.githubusercontent.com/kata-containers/kata-containers/main/utils/kata-manager.sh) -L"
```

#### Show the default packaged hypervisor

To show the default packaged hypervisor, run the following on an
installed system:

```bash
$ bash -c "$(curl -fsSL https://raw.githubusercontent.com/kata-containers/kata-containers/main/utils/kata-manager.sh) -L" | grep default
```

#### Show the locally configured hypervisor

`kata-manager.sh` will create a "local" copy of the packaged Kata configuration
file.

To show details of the _local_ copy of the configuration files on an
installed system, run:

```bash
$ bash -c "$(curl -fsSL https://raw.githubusercontent.com/kata-containers/kata-containers/main/utils/kata-manager.sh) -e"
```

> **Note:** This command can only be run once Kata has been installed.

> See the [configuration documentation](https://github.com/kata-containers/kata-containers#configuration)
> for further information.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if those 3 options could be combined into a single one which would output something like:

HYPERVISOR | ARGUMENT NAME | DEFAULT
Cloud Hypervisor | clh | yes
QEMU | qemu | no
,,,,

Or something towards that line ...
NOTE: After the next release e should remove all the mentions to $ bash -c ... and just point to the script directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if those 3 options could be combined into a single one

I'm not totally clear what you mean by this comment but I have updated the hypervisors doc to include more hypervisor config details including the "short name" used by kata-manager.

NOTE: After the next release e should remove all the mentions to $ bash -c ... and just point to the script directly.

Yep - see #9091.

@fidencio
Copy link
Member

fidencio commented Feb 15, 2024

A few more notes here:

ubuntu@kata-manager:~/kata-containers/utils$ ./kata-manager.sh -L
acrn;;packaged;golang
clh;;packaged;golang
dragonball;;packaged;golang
fc;;packaged;golang
qemu-nvidia-gpu;;packaged;golang
qemu-sev;;packaged;golang
qemu-snp;;packaged;golang
qemu-tdx;;packaged;golang
qemu;default;packaged;golang

This output is not user friendly, and dragonball is not golang. :-)
It's hard for people to know what each column means.

Also, what's the value of having packaged there?

@fidencio
Copy link
Member

I noticed you'be been changing the default configuration on /etc/kata-containers, wouldn't be easier to just set the default in /opt/kata/...? That's what we already do with kata-deploy.

@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 3dcd534 to 54fd20f Compare February 16, 2024 14:46
@jodh-intel
Copy link
Contributor Author

I noticed you'be been changing the default configuration on /etc/kata-containers, wouldn't be easier to just set the default in /opt/kata/...? That's what we already do with kata-deploy.

It is admittedly more "fiddly" to deal with, but kata-manager isn't just for kata-deploy environments, so it's safer and cleaner to modify the local config files in /etc/kata-containers/ as that also works for stateless environments, read-only disk environments, etc. So, once kata-manager installs Kata, it never touches the pristine files below /opt/kata/. Plus, I don't think we want to drop the concept that if you futz up your local config somehow, you can either move / rename your local config files, or even sudo rm -rf /etc/kata-containers/, and the system will reset to "factory defaults" 😄

@jodh-intel
Copy link
Contributor Author

A few more notes here:

ubuntu@kata-manager:~/kata-containers/utils$ ./kata-manager.sh -L
acrn;;packaged;golang
clh;;packaged;golang
dragonball;;packaged;golang
fc;;packaged;golang
qemu-nvidia-gpu;;packaged;golang
qemu-sev;;packaged;golang
qemu-snp;;packaged;golang
qemu-tdx;;packaged;golang
qemu;default;packaged;golang

This output is not user friendly

It's designed to be parseable, but we could add in a tab separated format too.

, and dragonball is not golang. :-)

Correct. See my note at #9059 (comment). That issue is caused by the fact that the Kata project hasn't produced a new release in four months. Related: #9064.

It's hard for people to know what each column means.

I can update the usage.

Also, what's the value of having packaged there?

I could drop that. It's there to show these are packaged config short names. We're not showing the full paths (since they may differ, are quite long and would make the output even worse unless you're happy with line wrapping or happen to have a wide terminal), so need an indicator as to whether these are the pristine packaged config files, or if they are the local config file copies.

@@ -737,6 +737,7 @@ configure_kata()
sudo install -o root -g root -m 0644 "$cfg_from" "$cfg_to"

sudo sed -i \
--follow-symlinks \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch @jodh-intel !
I wasnt aware myself about this behaviour myself. Reading about this, it shows that GNU sed has --follow-symlinks, while BSD sed does not. It will be useful to atleast document this here, in case someone tries to use this/port this to MacOS. We afterall do support macos to some extent.

We could mention something like "If you're on macOS or another BSD variant, you'll need to install GNU sed via your package manager (brew, pkg, etc)"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


> **Note:** For details of each hypervisor, see [3].

- If an invalid hypervisor name is specified with the '-H' option, an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I do not like the concept of designating any partcular hypervisor as default. I feel all the hypervisors should be treated the same with the user choosing one based on his/her needs.
(While I do understand that configuration.toml points to qemu conf by default, we should limit the usage of default and not designate any one hypervisor as default)

If there is a failure on configuration here, could we just go back to previous configuration instead of default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @amshinde - I don't disagree with what you've said, but we need some way to designate "the currently configured hypervisor". How about just configured?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jodh-intel Yes that wording works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amshinde - Switching everything to configured unfortunately results in a less clear approach. For example, how does the user "undo all changes" and revert to the pristine default configured hypervisor?

The current approach is:

$ kata-manager.sh -S default

But that just looks odd if we change the name to configured as it just doesn't express the same intent:

$ kata-manager.sh -S configured

That's confusing and ambiguous imho as it doesn't suggest the value specified to the -S option means "revert to the original configured hypervisor".

The best I can come up with as a compromise is:

$ kata-manager.sh -S pristine

wdyt?

/cc @fidencio

@@ -23,6 +23,8 @@ readonly containerd_slug="containerd/containerd"
readonly kata_project_url="https://github.com/${kata_slug}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding my comment here, but this is for the commiot message. Just reading the commit message, the usage of filename fragments is not really clear to me. Could we reword it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 54fd20f to a4f4999 Compare February 28, 2024 15:28
@jodh-intel
Copy link
Contributor Author

I've also changed the format so that it now uses tabs rather than semi-colons so that it's still parseable but looks a bit less "busy" 😄

@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from a4f4999 to 0429771 Compare February 29, 2024 16:42
@jodh-intel
Copy link
Contributor Author

/test

@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 0429771 to 9ddd823 Compare March 1, 2024 16:07
@jodh-intel
Copy link
Contributor Author

/test

Remove extraneous whitespace.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Ensure the usage statement lists all options in alphabetical order.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Add an info message just before the archive file is checked. This keeps
the user informed about what is happening as it can take a few seconds
to perform the checks on slower systems.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The `configure_kata()` function modifies the configuration file to
enable debug. But it was doing this by calling `sed -i` which, by
default, creates a new _file_ from the `configuration.toml` symbolic
link. This defeated the point of the symbolic link which is supposed to
resolve to the local copy of the pristine config file, so we now use
the GNU sed(1) specific `---follow-symlinks` option to retain the
sym-link.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel added the do-not-merge PR has problems or depends on another label Mar 4, 2024
Add new options to allow the configured hypervisor to be changed:

- `-L`: List available _packaged_ hypervisor config short names.
- `-e`: List available _local_ hypervisor config names.
- `-H <hypervisor>`: Install Kata then switch to the specified hypervisor.
- `-S <hypervisor>`: Switch to the specified hypervisor (by config short name [Errors if Kata not installed]).

For example, to install Kata and configure it to use Cloud Hypervisor
with the golang Kata runtime:

```bash
$ kata-manager.sh -H clh
```

To switch back to the default hypervisor:

```bash
$ kata-manager.sh -S default
```

To show details of the available packaged configs:

```bash
$ kata-manager.sh -L
```

To show details of the local configs:

```bash
$ kata-manager.sh -e
```

> **Notes:**
>
> - This change **only** applies to the current default (golang) Kata runtime.
>
> - Although this is mainly for users wishing to switch hypervisor (by
>   changing the Kata config file to another of the packaged config files
>   provided for specific hypervisors), strictly it allows users to change
>   to _any_ config file. For example, if the user has a config file called
>   `/etc/kata-containers/configuration-my-custom-config.toml`, they could
>   switch to this by running:
>
>   ```bash
>   $ kata-manager.sh -S my-custom-config
>   ```
>
> - The "config short names" are the hypervisor specific part of the configuration file name.
>   For example, the config short name for file `configuration-qemu.toml` is
>   `qemu` and the config short name for `configuration-clh.toml` is `clh`.

Fixes: kata-containers#8305.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Remove extraneous whitespace from hypervisors doc.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 9ddd823 to 5d21040 Compare March 4, 2024 12:21
Add details to the README for `kata-manager` showing how to list
available hypervisor configs (packaged and local), and switch between
the configurations. Also, update the hypervisors page to show a lot more
detail about the hypervisor configurations, including the "short name"
used by `kata-manager` for switching hypervisor config.

> **Note:**
>
> These changes only apply to the current default golang runtime.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the kata-manager-add-hypervisor-option branch from 5d21040 to 7af892f Compare March 4, 2024 12:24
@jodh-intel
Copy link
Contributor Author

/test

@jodh-intel jodh-intel removed the do-not-merge PR has problems or depends on another label Mar 4, 2024
@jodh-intel
Copy link
Contributor Author

jodh-intel commented Mar 4, 2024

The ARM CI (kata-containers-2.0-ubuntu20.04-ARM-main) is failing:

4:21:11 Reading package lists...
14:21:11 E: The repository 'http://packages.cloud.google.com/apt kubernetes-xenial-unstable Release' no longer has a Release file.
14:21:11 W: Target Packages (stable/binary-arm64/Packages) is configured multiple times in /etc/apt/sources.list:5 and /etc/apt/sources.list.d/docker.list:1
14:21:11 W: Target Packages (stable/binary-all/Packages) is configured multiple times in /etc/apt/sources.list:5 and /etc/apt/sources.list.d/docker.list:1
14:21:11 W: Target Packages (stable/binary-arm64/Packages) is configured multiple times in /etc/apt/sources.list:5 and /etc/apt/sources.list.d/docker.list:1
14:21:11 W: Target Packages (stable/binary-all/Packages) is configured multiple times in /etc/apt/sources.list:5 and /etc/apt/sources.list.d/docker.list:1
14:21:11 [setup_env_ubuntu.sh:15] ERROR: sudo -E apt update
14:21:11 [jenkins_job_build.sh:248] ERROR: ci/setup.sh
14:21:13 Build step 'Execute shell' marked build as failure

/cc @jongwu.

@jodh-intel jodh-intel merged commit b761a80 into kata-containers:main Mar 5, 2024
286 of 297 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test safe-to-test Add to PR after manually reviewing to allow certain extra checks to run size/huge Largest and most complex task (probably needs breaking into small pieces)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

utils: kata-manager: Add option to switch hypervisor (config file)
6 participants