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.

Sign up for GitHub

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

Jump to bottom

Upgrade to containerd 2 #5507

Merged
merged 3 commits into from
Jan 14, 2025
Merged

Upgrade to containerd 2 #5507

merged 3 commits into from
Jan 14, 2025

Conversation

slonopotamus
Copy link
Contributor

@slonopotamus slonopotamus commented Nov 10, 2024 •
edited
Loading

This PR depends on

Non-import changes:

  1. stargz-snapshotter dependency was bumped to unreleased commit because of containerd api changes v0.16.2
  2. fuse-overlayfs-snapshotter dependency was bumped to v2.0.0
  3. Several more deps were automatically updated by go mod tidy
  4. ExpiresIn was changed to ExpiresInSeconds in authprovider.go
  5. Workaround for archive: use 1970-01-01 as the whiteout timestamp containerd/containerd#8764 was removed in overlay_linux.go
  6. Minor code cleanup in main_containerd_worker_windows.go
  7. Adjusted expected string in testUndeclaredArg in dockerfile_lint_test.go
  8. I dropped testSchema1Image test. It is very nontrivial to push CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE=1 env variable from a single place to both rootful and rootless tests (if you know such place, tell me!). Schema v1 is going away in containerd 2.1 anyway. buildkit still supports in currently. If this change is unacceptable... Well, need more time to find where exactly to put CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE=1 in buildkit test framework.

Closes #5514

@slonopotamus slonopotamus force-pushed the containerd-2 branch 10 times, most recently from a1ee378 to 11d49af Compare November 10, 2024 15:15
@slonopotamus slonopotamus marked this pull request as ready for review November 10, 2024 15:16
dmcgowan
dmcgowan approved these changes Dec 9, 2024
View reviewed changes
Copy link
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

I think this is ready to get in here and subsequently in moby/moby

@slonopotamus
Copy link
Contributor Author

Rebased to resolve conflicts.

@slonopotamus slonopotamus force-pushed the containerd-2 branch 2 times, most recently from 5fd22f1 to 8c7af4f Compare December 19, 2024 09:10
@AkihiroSuda
Copy link
Member

Needs rebase again

@slonopotamus slonopotamus force-pushed the containerd-2 branch 5 times, most recently from 9098dec to 04efbe7 Compare January 8, 2025 13:31
@AkihiroSuda
Copy link
Member

@tonistiigi PTAL

@AkihiroSuda
Copy link
Member

Needs rebase again 😥

@slonopotamus
Copy link
Contributor Author

Rebased again...

tonistiigi
tonistiigi reviewed Jan 14, 2025
View reviewed changes
client/client_test.go
@@ -7110,23 +7109,6 @@ func testMoveParentDir(t *testing.T, sb integration.Sandbox) {
require.True(t, ok)
}

// #296
func testSchema1Image(t *testing.T, sb integration.Sandbox) {
Copy link
Member

Choose a reason for hiding this comment

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

Reenable this test if schema1 is still supported. I'm not sure why library would check for env, but you can add it in integration pkg where it is running buildkitd if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are issues with how rootless tests are run (via sudo that drops env).

Copy link
Member

Choose a reason for hiding this comment

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

You can put env in the sudo args, or is it rootlesskit that drops? You can skip rootless if needed, or I guess you can just define this env in cmd/buildkitd/main. It is containerd env anyway so doesn't need to control buildkit (and should probably be in ctr in upstream and not in the library.

tonistiigi
tonistiigi reviewed Jan 14, 2025
View reviewed changes
go.mod
gopkg.in/yaml.v3 v3.0.1 // indirect
kernel.org/pub/linux/libs/security/libcap/psx v1.2.70 // indirect
sigs.k8s.io/yaml v1.4.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Talked with @dmcgowan and we should be able to get rid of these new yaml/cdi imports in a follow-up containerd update.

tonistiigi
tonistiigi reviewed Jan 14, 2025
View reviewed changes
go.mod Outdated Show resolved Hide resolved
slonopotamus and others added 3 commits January 13, 2025 16:41
@slonopotamus @tonistiigi
Drop schema v1 test
a4644fe 
Schema v1 support will be removed in containerd-2.1.

While we continue to support it when containerd is launched with CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE=1 env variable, the effort to keep the test working isn't worth it.

Signed-off-by: Marat Radchenko <[email protected]>
@slonopotamus @dmcgowan @tonistiigi
Upgrade to containerd 2
5be7edb 
Co-authored-by: Derek McGowan <[email protected]>
Signed-off-by: Marat Radchenko <[email protected]>
@tonistiigi
vendor: update nydus-snapshotter to v0.15
654a5c0 
Signed-off-by: Tonis Tiigi <[email protected]>
tonistiigi
tonistiigi approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Reenable schema1 test in follow-up

@tonistiigi tonistiigi merged commit 9eef9d7 into moby:master Jan 14, 2025
97 checks passed
@slonopotamus slonopotamus deleted the containerd-2 branch January 14, 2025 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ktock ktock ktock left review comments

@tonistiigi tonistiigi tonistiigi approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@dmcgowan dmcgowan dmcgowan approved these changes

@crazy-max crazy-max Awaiting requested review from crazy-max

Assignees

@tonistiigi tonistiigi

Labels
area/buildctl area/buildkitd area/client area/dependencies Pull requests that update a dependency file area/dockerfile area/examples area/executor area/exporter area/frontend area/llb area/remotecache area/session area/solver area/source area/testing area/util area/worker needs/follow-up
Projects
None yet
Milestone
v0.19.0
Development

Successfully merging this pull request may close these issues.

Upgrade to containerd 2.0
7 participants
@slonopotamus @dmcgowan @AkihiroSuda @crazy-max @tonistiigi @ktock @thompson-shaun