Skip to content

Conversation

@sachaos
Copy link
Contributor

@sachaos sachaos commented Mar 4, 2022

I think we are recommending the version 2 configuration file format.
docs/PLUGINS.md is formatted as version 1, which can be confusing.
So I fixed the documentation.

Signed-off-by: Takumasa Sakao [email protected]

@k8s-ci-robot
Copy link

Hi @sachaos. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sachaos sachaos force-pushed the feature/use-version-2-config-in-doc branch from dad2b70 to c58c7ce Compare March 4, 2022 00:07
@theopenlab-ci
Copy link

theopenlab-ci bot commented Mar 4, 2022

Build succeeded.

  • containerd-build-arm64 : RETRY_LIMIT in 3m 41s (non-voting)
  • containerd-test-arm64 : RETRY_LIMIT in 5m 44s (non-voting)
  • containerd-integration-test-arm64 : RETRY_LIMIT in 27m 14s (non-voting)

docs/PLUGINS.md Outdated
[plugins.cri.registry]
[plugins.cri.registry.mirrors]
[plugins.cri.registry.mirrors."docker.io"]
[plugins."io.containerd.grpc.v1.cri".registry]
Copy link
Member

Choose a reason for hiding this comment

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

this is definitely worth updating, but I wonder if we should just remove the registry section as this is using the now-deprecated pattern for registry configuration? Or simply remove the mirrors/endpoint content and replace it with the config_path setting which is the new configuration method for registry hosts/mirrors.

Copy link
Member

Choose a reason for hiding this comment

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

the example here is an old one.. and partial.. and there is no explanation about the contents...

Suggest removing the cri parts... and leave cgroups as the example

Maybe mention how to run containerd config default / dump to see the current default / combined config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@estesp @mikebrow Thank you!

Suggest removing the cri parts... and leave cgroups as the example
Maybe mention how to run containerd config default / dump to see the current default / combined config

I think your suggestion is good.
I'll remove cri part and use this snippet as a short example.
And mention about how to run containerd config default / dump to see the current default / combined config to tell how to get a full example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@estesp @mikebrow I've fixed this. 03a5e64

@sachaos sachaos force-pushed the feature/use-version-2-config-in-doc branch from c58c7ce to 03a5e64 Compare March 4, 2022 17:24
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@estesp estesp merged commit f6694f5 into containerd:main Mar 4, 2022
@thaJeztah thaJeztah added the cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch label Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch kind/docs ok-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants