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-deploy: Fix shim check for snapshotter configuration #8733

Conversation

fidencio
Copy link
Member

We want to check whether the shim is part of the "plain text" shims passed to the daemonset (meaning, checking against $SHIMS). Before this fix we were checking against $shims, which is an array of shims instead of a string, resulting on a broken check.

Fixes: #8732

We want to check whether the shim is part of the "plain text" shims
passed to the daemonset (meaning, checking against `$SHIMS`).  Before
this fix we were checking against `$shims`, which is an array of shims
instead of a string, resulting on a broken check.

Fixes: kata-containers#8732

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio fidencio added force-skip-ci Stop the CI running for a PR (remember: if you break it, you fix it!) ok-to-test labels Dec 26, 2023
@fidencio
Copy link
Member Author

/test

@katacontainersbot katacontainersbot added the size/tiny Smallest and simplest task label Dec 26, 2023
fidencio added a commit to fidencio/cc-operator that referenced this pull request Dec 26, 2023
This PR has been opened in order to test the changes needed for
configuring containerd snapshotters per runtime handlers, and depends on
a PR that's not yet merged on Kata Containers:
kata-containers/kata-containers#8733

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
In the way the script is without this patch, we're trying to set
```toml
[`$shim`]
snapshotter = $snapshotter
```

However, what we actually want to set is the full runtime table instead
of shim.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
fidencio added a commit to fidencio/cc-operator that referenced this pull request Dec 26, 2023
This PR has been opened in order to test the changes needed for
configuring containerd snapshotters per runtime handlers, and depends on
a PR that's not yet merged on Kata Containers:
kata-containers/kata-containers#8733

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Otherwise `jq` will complain about:
```sh
jq: error: nydus/0 is not defined at <top-level>, line 1:
.plugins."io.containerd.grpc.v1.cri".containerd.runtimes."kata-clh".snapshotter=nydus
jq: 1 compile error
```

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
fidencio added a commit to fidencio/cc-operator that referenced this pull request Dec 26, 2023
This PR has been opened in order to test the changes needed for
configuring containerd snapshotters per runtime handlers, and depends on
a PR that's not yet merged on Kata Containers:
kata-containers/kata-containers#8733

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio
Copy link
Member Author

/test

Copy link
Member

@ChengyuZhu6 ChengyuZhu6 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @fidencio

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@fidencio fidencio merged commit 35f88df into kata-containers:main Dec 27, 2023
167 of 176 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force-skip-ci Stop the CI running for a PR (remember: if you break it, you fix it!) ok-to-test size/tiny Smallest and simplest task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kata-deploy: Fix shim check for snapshotter configuration
5 participants