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

Install cri-tools #1235

Closed
wants to merge 1 commit into from
Closed

Install cri-tools #1235

wants to merge 1 commit into from

Conversation

cartermckinnon
Copy link
Member

Issue #, if available:

Closes #797 .

Description of changes:

Adds cri-tools, which includes crictl and critest. The latest version of the package will be installed at build, and the package is not version-locked, because it's not in the critical path.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

> make
...
2023-03-28T11:50:21-07:00:     amazon-ebs: Downloading packages:
2023-03-28T11:50:22-07:00:     amazon-ebs: cri-tools-1.25.0-1.amzn2.0.1.x86_64.rpm                    |  17 MB   00:00
2023-03-28T11:50:22-07:00:     amazon-ebs: Running transaction check
2023-03-28T11:50:22-07:00:     amazon-ebs: Running transaction test
2023-03-28T11:50:22-07:00:     amazon-ebs: Transaction test succeeded
2023-03-28T11:50:22-07:00:     amazon-ebs: Running transaction
2023-03-28T11:50:24-07:00:     amazon-ebs:   Installing : cri-tools-1.25.0-1.amzn2.0.1.x86_64                          1/1
2023-03-28T11:50:24-07:00:     amazon-ebs:   Verifying  : cri-tools-1.25.0-1.amzn2.0.1.x86_64                          1/1
2023-03-28T11:50:24-07:00:     amazon-ebs:
2023-03-28T11:50:24-07:00:     amazon-ebs: Installed:
2023-03-28T11:50:24-07:00:     amazon-ebs:   cri-tools.x86_64 0:1.25.0-1.amzn2.0.1
2023-03-28T11:50:24-07:00:     amazon-ebs:
2023-03-28T11:50:24-07:00:     amazon-ebs: Complete!

@cartermckinnon cartermckinnon mentioned this pull request Mar 28, 2023
Closed
dims
dims approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@dims dims left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

mmerkes
mmerkes reviewed Mar 28, 2023
View reviewed changes
scripts/install-worker.sh
@@ -343,6 +343,9 @@ sudo systemctl daemon-reload
# Disable the kubelet until the proper dropins have been configured
sudo systemctl disable kubelet

# install crictl, critest
sudo yum install -y cri-tools
Copy link
Member

Choose a reason for hiding this comment

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

Does this package get updated in the yum repo along with containerd? We'll undoubtedly get some CVEs from this, so want to make sure that it gets updated within reason.

Copy link
Member Author

@cartermckinnon cartermckinnon Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I thought this was versioned with containerd, but it's actually a k8s project. Seems to be versioned with k8s on major + minor. I asked around in the k8s slack, not sure if we should only install something matching the kubelet version. Also not sure how it's ending up in the AL repos, so I don't know how diligently it's updated.

Copy link
Member

Choose a reason for hiding this comment

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

IMO, we need to understand how it's updated before we install it by default, or we'll just be doing scramble drill when it needs to be updated. As far as we know, that could be right away!

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll definitely get pinged to update it, but it's just a CLI tool; shouldn't ever be an emergency. I'll see what the sig-node folks say!

Copy link
Member

Choose a reason for hiding this comment

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

I'm more interested in how that yum repo gets updated. If it just points to some repo that sig-node folks update, that's fine. As long as we're regularly getting the latest, seems reasonable.

mmerkes
mmerkes approved these changes Mar 28, 2023
View reviewed changes
Copy link
Member

@mmerkes mmerkes left a comment

Choose a reason for hiding this comment

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

LGTM pending conversation from other comment.

@cartermckinnon
Copy link
Member Author

There doesn't seem to be consensus or strict guidance on whether you need to run the same major + minor version of cri-tools and kubelet, though my impression is that this is the safest route. Because we don't vend cri-tools with our other binaries, we can't guarantee that a needed version exists in the yum repos (we shouldn't just install what's available).

I'm going to close this PR, because a proper implementation will differ significantly.

@ddl-retornam ddl-retornam mentioned this pull request Jun 6, 2023
Merged
@cartermckinnon cartermckinnon deleted the install-cri-tools branch June 7, 2023 15:27
@mohag
Copy link

mohag commented Oct 23, 2023

There doesn't seem to be consensus or strict guidance on whether you need to run the same major + minor version of cri-tools and kubelet, though my impression is that this is the safest route. Because we don't vend cri-tools with our other binaries, we can't guarantee that a needed version exists in the yum repos (we shouldn't just install what's available).

I'm going to close this PR, because a proper implementation will differ significantly.

cri-tools does not get released with the rest of Kubernetes - it is possible that there is no version of cri-tools for a specific kubernetes version. (the CRI apiVersion support is the most important parameter - it should be the same as the Kubernetes version)

(with kubeadm the OS packages used to depend on the latest version of cri-tools, which broke deployment of clusters on supported containerd versions, since the cri-tools version was significantly newer, without support for some (beta) CRI APIs that the kubernetes version still supported - which meant that clusters required newer containerd versions to deploy than what was supposed to work) See kubernetes/release/#2866 for the kubeadm issue

@cartermckinnon cartermckinnon mentioned this pull request Oct 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmerkes mmerkes mmerkes approved these changes

@Issacwww Issacwww Issacwww approved these changes

@dims dims dims 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.

Install crictl for CLI interaction with containerd
5 participants
@cartermckinnon @mohag @dims @mmerkes @Issacwww