-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
Revise “Container runtimes” page in light of dockershim removal #32738
Revise “Container runtimes” page in light of dockershim removal #32738
Conversation
/milestone 1.24 |
|
||
- [containerd](#containerd) | ||
- [CRI-O](#cri-o) | ||
- [Docker Engine](#docker) | ||
- [Mirantis Container Runtime](#mcr) | ||
|
||
{{< note >}} | ||
For other operating systems, look for documentation specific to your platform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original content here at least mentions other operating systems. Where this information is relocated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean OSs other than Linux or Windows?
My thinking is that, given our docs policy on 3rd party content, the container runtimes' docs should cover installs onto various operating systems. If they don't, folks can propose adding it to those docs, not to Kubernetes.
Copy-Item -Path ".\bin\" -Destination "$Env:ProgramFiles\containerd" -Recurse -Force | ||
cd $Env:ProgramFiles\containerd\ | ||
.\containerd.exe config default | Out-File config.toml -Encoding ascii | ||
|
||
# Review the configuration. Depending on setup you may want to adjust: | ||
# - the sandbox_image (Kubernetes pause image) | ||
# - cni bin_dir and conf_dir locations | ||
Get-Content config.toml | ||
|
||
# (Optional - but highly recommended) Exclude containerd from Windows Defender Scans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why drop these lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://kubernetes.io/docs/contribute/style/content-guide/#what-s-allowed basically. We should aim to link to external docs; I'm also happy to see work done on those external docs (and maybe I will do some of that myself).
content/en/docs/setup/production-environment/container-runtimes.md
Outdated
Show resolved
Hide resolved
✅ Deploy Preview for kubernetes-io-main-staging ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Co-authored-by: Qiming Teng <[email protected]>
{{< /note >}} | ||
|
||
|
||
<!-- body --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this comment doing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIG Docs uses these comments to mark where the introduction ends and the page body starts.
@@ -120,95 +134,51 @@ using the (deprecated) v1alpha2 API instead. | |||
|
|||
### containerd | |||
|
|||
This section contains the necessary steps to use containerd as CRI runtime. | |||
This section outlines the necessary steps to use containerd as CRI runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be containerd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need code style for the tool name (which is containerd).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nits, otherwise looks good from my end
@sftim, are the changes part of the Dockershim removal updates? |
Yes, these changes are related to the dockershim removal. |
/lgtm Possibly move to |
LGTM label has been added. Git tree hash: e605e45325e60394c40a8c06f58a46637290e07d
|
These changes document the story for v1.23 I expect that the existing v1.24 version of this page would “win” the merge conflict. The changes are important because people are still using v1.23 and may want to move away from dockershim already, even though it is only deprecated. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: reylejano The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This updates the Container Runtimes page of the website to include information about the dockershim deprecation, so that the users can take an informed decision. This helps with issue #28449 and potentially supersedes PR #30882.