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

pre-reqs: Update nydus snapshotter version #378

Conversation

stevenhorsman
Copy link
Member

- Bump the nydus snapshotter version to v0.13.13, in order to
pick the nydus-snapshotter caching workaround
containerd/nydus-snapshotter#593

Signed-off-by: stevenhorsman <steven@uk.ibm.com>
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.

lgtm, thanks @stevenhorsman!

@@ -13,7 +13,7 @@ official_containerd_version=${official_containerd_version:-"1.7.7"}
vfio_gpu_containerd_repo=${vfio_gpu_containerd_repo:-"https://github.com/confidential-containers/containerd"}
vfio_gpu_containerd_version=${vfio_gpu_containerd_version:-"1.7.0.0"}
nydus_snapshotter_repo=${nydus_snapshotter_repo:-"https://github.com/containerd/nydus-snapshotter"}
nydus_snapshotter_version=${nydus_snapshotter_version:-"v0.13.3-multiarch"}
nydus_snapshotter_version=${nydus_snapshotter_version:-"v0.13.13"}
Copy link
Contributor

Choose a reason for hiding this comment

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

is it necessary to keep updating the default when we have it specified in the Makefile? Having 2 places can be confusing...

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you, but didn't want to go into that on this PR. Do you thing an approach of us letting the set -o nounset fail if it's not passed in would make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ldoktor - I've added a second commit, so please check that out and see if you are happy and we can get @fidencio to re-review too.

- To avoid having to update the versions in multiple places (which was missed last release),
remove the default versions logic in payload.sh, as it is only called from the Makefile
If a version is incorrectly set, then just rely on
`set -o nounset` to warn the user about this

Signed-off-by: stevenhorsman <steven@uk.ibm.com>
Copy link

@mkulke mkulke left a comment

Choose a reason for hiding this comment

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

tested it, the nydus version installed by the operator is able to perform guest-pulls, even if images have been pulled by runc on the node before (pause). vice versa also works.

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm now.

@ldoktor ldoktor requested a review from fidencio May 15, 2024 11:45
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.

yay!

@fidencio fidencio merged commit 290ae5f into confidential-containers:main May 15, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants