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 1.6.0 release #575

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Upgrade to containerd 1.6.0 release #575

merged 1 commit into from
Mar 3, 2022

Conversation

henry118
Copy link
Member

Signed-off-by: Henry Wang [email protected]

#569 Upgrade containerd to 1.6.0 release. This also upgrade minimum version of golang to 1.16.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@henry118 henry118 requested a review from a team as a code owner February 23, 2022 22:29
@henry118
Copy link
Member Author

This ttrpc/protobuf issue was spotted after upgrading to containerd 1.6.0. We'll have to work around it by regenerating fc-cd protos. This has been addressed by this commit.

Now another issue has surfaced which is related to #210. It needs to be addressed before we can migrate to containerd 1.6.0.

@henry118
Copy link
Member Author

Fixed #210 by restoring the original logic of the Connect routine.

The original logic is actually the same as containerd 1.5, where the task pid is cached by containerd upon a task is created.

This is essentially the same as querying agent for the task pid.

The integ tests are passing after this change.

@kzys
Copy link
Contributor

kzys commented Feb 28, 2022

The changes look good to me. Can you include some of the comments on this PR to the commits themselves?

@henry118
Copy link
Member Author

henry118 commented Mar 1, 2022 •
edited
Loading

  • Squashed commits; and
  • Updated commit message to explain changes in this patch.
  • Rebased from main

@henry118
This patch upgrades containerd to 1.6.0.
c68b61c 
This patch also includes the required changes to address the
issues surfaced during the upgrade:

* ttrpc/protobuf compatibility issue (containerd firecracker-microvm#62) spotted
  after upgrading to containerd 1.6.0. It is addressed by
  regenerating fc-cd protos;

* Changed behavior of containerd shimTask's PID routine.
  It is fixed by restoring original logic of fc-cd shim's
  Connect routine, which is forwarding the request to the
  agent.

Signed-off-by: Henry Wang <[email protected]>
@henry118
Copy link
Member Author

henry118 commented Mar 2, 2022

Is there anything else to be addressed before merging this PR into containerd-1.6.0 branch?

@kzys
Copy link
Contributor

kzys commented Mar 2, 2022

Is there anything else to be addressed before merging this PR into containerd-1.6.0 branch?

Please update the AL2 pipeline to fix the test failure.

@kzys
Copy link
Contributor

kzys commented Mar 2, 2022 •
edited
Loading

Also is it possible to rebase your branch, instead of having the changes from main? It is hard to see the diff now.

@henry118 henry118 changed the base branch from containerd-1.6.0 to main March 3, 2022 00:01
@henry118
Copy link
Member Author

henry118 commented Mar 3, 2022

  1. Updated AL2 test hosts and all tests passed;
  2. Edited this PR to target the main branch.

@henry118 henry118 merged commit 45088e7 into firecracker-microvm:main Mar 3, 2022
@henry118 henry118 deleted the containerd-1.6.0 branch March 3, 2022 01:04
@Kern-- Kern-- mentioned this pull request Mar 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinvazquez austinvazquez austinvazquez approved these changes

@kzys kzys kzys approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@henry118 @kzys @austinvazquez