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

Upgrade containerd to v1.7 #785

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

henry118
Copy link
Member

Upgrade containerd to v1.7

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 April 26, 2024 22:04
@henry118 henry118 force-pushed the ctrd17 branch 14 times, most recently from b95e66e to 0d8d85d Compare May 1, 2024 07:23
"github.com/firecracker-microvm/firecracker-containerd/internal/vm"
ioproxy "github.com/firecracker-microvm/firecracker-containerd/proto/service/ioproxy/ttrpc"
"github.com/gogo/protobuf/types"
"google.golang.org/protobuf/types/known/emptypb"
Copy link
Contributor

Choose a reason for hiding this comment

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

containerd has defined a types package to make this migration easier. This should also help with any future migration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. @swagatbora90 PTAL

Signed-off-by: Henry Wang <[email protected]>
command:
- make -C snapshotter integ-test FICD_DM_POOL=build_${BUILDKITE_BUILD_NUMBER}_snapshotter
timeout_in_minutes: 10
# - label: ":running: snapshotter isolated tests"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we intend to enable these tests in the future? Please cut a issue so that we can track it once this PR is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not. The snapshotter is no longer needed. But i just commented this block in case we'd like to revoke this decision later.

@@ -273,31 +273,31 @@ $(CNI_BIN_ROOT):

BRIDGE_BIN?=$(BINPATH)/bridge
$(BRIDGE_BIN):
GOBIN=$(dir $@) GO111MODULE=off go get -u github.com/containernetworking/plugins/plugins/main/bridge
GOBIN=$(dir $@) go install github.com/containernetworking/plugins/plugins/main/bridge@v1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to pin the version here. Can we not use @latest? This will need separate update mechanism otherwise

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 had to pin the plugins to some older versions to pass integ-tests, which still have the CNI configured for older versions

Copy link
Contributor

Choose a reason for hiding this comment

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

Besides CNI, there are some other packages such as git-validation etc that have their versions pinned.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. For toolchain binaries, it's better to pin to a known good version otherwise upgrade may (surprisingly) break our build.

Copy link
Contributor

@swagatbora90 swagatbora90 left a comment

Choose a reason for hiding this comment

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

LGTM.

@henry118 henry118 merged commit 6b62972 into firecracker-microvm:main May 2, 2024
5 checks passed
@henry118 henry118 deleted the ctrd17 branch May 2, 2024 21:08
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