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

Update to containerd v2 #1329

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

apostasie
Copy link

Follow-on #1305

Signed-off-by: apostasie <[email protected]>
@apostasie apostasie marked this pull request as ready for review July 31, 2024 18:25
@apostasie apostasie requested a review from a team as a code owner July 31, 2024 18:25
@apostasie apostasie changed the title [WIP] Update to containerd v2 Update to containerd v2 Jul 31, 2024
@apostasie
Copy link
Author

apostasie commented Jul 31, 2024

Ping @sondavidb: here is the adapted PR to merge into the custom branch.
Thanks again!

Given the way the CI is configured, it is not running on a PR against a branch.

Do we want to change that or was that done on purpose for some reason?

@sondavidb
Copy link
Contributor

Hey, thanks for this again!

Given the way the CI is configured, it is not running on a PR against a branch.

Not a particularly good reason, but we don't really do feature branches usually, so we can change this. I have a PR out for this at #1330.

However, we did some discussion internally and realized that, should nerdctl release v2 with this branch's commit, we will have to keep up this branch forever, so it would not be a temporary branch. This is fine if necessary, but we would prefer to not do so.

As nerdctl's only dependency is this function in fs/source, I'd like to see if nerdctl maintainers are interested in just keeping a local copy in nerdctl instead, so I've opened a PR in nerdctl. We can always revert this once we update to containerd v2, but as we don't want to be forced to keep this branch unless necessary, the nerdctl change is preferred.

@apostasie
Copy link
Author

Hey @sondavidb

Your PR got merged into nerdctl - which does solve my problem. Thanks a lot!

This PR here still has value of course - would you like me to close it and re-open it against master, and we go back to the previously agreed state (wait until containerd v2 is out, rebase)?

@sondavidb sondavidb changed the base branch from upgrade-containerd-v2 to main August 5, 2024 17:01
@sondavidb
Copy link
Contributor

sondavidb commented Aug 5, 2024

Your PR got merged into nerdctl

🎉

would you like me to close it and re-open it against master, and we go back to the previously agreed state (wait until containerd v2 is out, rebase)?

Sure, we can do that. I don't think you have to close this, though. I can edit your PR to main instead of the feature branch, so I've gone ahead and did that. Also deleted the feature branch entirely.

LMK if there's anything else you need. Thanks a bunch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants