Skip to content

Make CRI registry docs more clear#5254

Merged
mikebrow merged 1 commit intocontainerd:masterfrom
voltbit:clarify-cri-registry-doc
Mar 23, 2021
Merged

Make CRI registry docs more clear#5254
mikebrow merged 1 commit intocontainerd:masterfrom
voltbit:clarify-cri-registry-doc

Conversation

@voltbit
Copy link
Copy Markdown
Contributor

@voltbit voltbit commented Mar 23, 2021

Added reference to previous config syntax.

Signed-off-by: Andrei Dobre [email protected]

Fixes #5151

@k8s-ci-robot
Copy link
Copy Markdown

Hi @voltbit. 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.

Copy link
Copy Markdown
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

see comments..

Note: commits have to be signed with a real name and email. Suggest using 'git commit -s --amend' after setting up your git config for name/email.

Cheers, Mike

Comment thread docs/cri/registry.md Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be ok to have a link to an archived v1 description for pre 1.3 users. We don't support moving a v2 config to the old syntax.

@voltbit voltbit force-pushed the clarify-cri-registry-doc branch from bf1af93 to cb68cf3 Compare March 23, 2021 16:23
@voltbit
Copy link
Copy Markdown
Contributor Author

voltbit commented Mar 23, 2021

Hey Mike, thanks for the comment, my commit is already signed with my personal email. You mean I should also add the 'Signed-off' message to my commit message?

L.E. I've just read the CONTRIBUTING.md, applogies for not checking it first. I've added the text to my commit message.

@voltbit voltbit requested a review from mikebrow March 23, 2021 16:27
@voltbit voltbit force-pushed the clarify-cri-registry-doc branch 2 times, most recently from 99363c6 to e49effd Compare March 23, 2021 16:48
Comment thread docs/cri/registry.md Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

believe it or not for this registry doc.. the 1.2 version was in continerd/cri .. over here:
https://github.com/containerd/cri/blob/release/1.2/docs/registry.md

@mikebrow
Copy link
Copy Markdown
Member

Hey Mike, thanks for the comment, my commit is already signed with my personal email. You mean I should also add the 'Signed-off' message to my commit message?

L.E. I've just read the CONTRIBUTING.md, applogies for not checking it first. I've added the text to my commit message.

sry about the DCO hassle :-) The reason I mentioned using -s for signing is it's hard to get that cut and paste just right.

Here's an example:

update for cni config

Signed-off-by: Mike Brown <[email protected]>

# Please enter the commit message for your changes. Lines starting

note description is one line and fairly short; note blank line before and after signature... I didn't pull your commit header but it's usually one of those details...

@voltbit voltbit force-pushed the clarify-cri-registry-doc branch 3 times, most recently from ddf1800 to 6cae3b6 Compare March 23, 2021 19:30
Added reference to previous config syntax.

Signed-off-by: Andrei Dobre <[email protected]>
@voltbit voltbit force-pushed the clarify-cri-registry-doc branch from 6cae3b6 to e4b9b10 Compare March 23, 2021 20:13
@voltbit voltbit requested a review from mikebrow March 23, 2021 20:23
Copy link
Copy Markdown
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@mikebrow mikebrow merged commit 664088d into containerd:master Mar 23, 2021
@mikebrow
Copy link
Copy Markdown
Member

Thanks!

@voltbit voltbit deleted the clarify-cri-registry-doc branch March 24, 2021 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Containerd 1.5.0-beta generated config still used : "io.containerd.grpc.v1.cri" instead of cri

3 participants