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

Update docs for kubelet credentials provider GA #38139

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Update docs for kubelet credentials provider GA #38139

merged 1 commit into from
Dec 6, 2022

Conversation

ndixita
Copy link
Contributor

@ndixita ndixita commented Nov 29, 2022

Signed-off-by: Narang Dixita Sohanlal ndixita@google.com

Adding doc changes for kep-2133 for KubeletCredentialProviders GA.
kubernetes/enhancements#2133

@k8s-ci-robot k8s-ci-robot added this to the 1.26 milestone Nov 29, 2022
@netlify
Copy link

netlify bot commented Nov 29, 2022 •
edited
Loading

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 2b003da
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/638e91136551da00086ea7a8

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. language/en Issues or PRs related to English language labels Nov 29, 2022
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Nov 29, 2022
@ruiwen-zhao
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 29, 2022
sftim
sftim previously requested changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Thanks.

You need to change this text too:

* The kubelet image credential provider is introduced in v1.20 as an alpha feature. As with other alpha features,
  a feature gate `KubeletCredentialProviders` must be enabled on only the kubelet for the feature to work.

as it is out of date.

Please also update https://kubernetes.io/docs/concepts/containers/images/#using-a-private-registry to link to https://k8s.io/docs/tasks/kubelet-credential-provider/kubelet-credential-provider/ and to mark that image pull credential plugin mechanism as stable in 1.26

@sftim
Copy link
Contributor

sftim commented Nov 29, 2022

/lgtm cancel

These changes are useful, but they are not the whole set we need.

/retitle Update docs for kubelet credentials provider GA
/sig node

@k8s-ci-robot k8s-ci-robot changed the title Kep 2133 docsKep 2133: update doc references and standardize the content for KubeletCredentialsProvider Update docs for kubelet credentials provider GA Nov 29, 2022
@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Nov 29, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 29, 2022
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 29, 2022
@ndixita
Copy link
Contributor Author

ndixita commented Nov 29, 2022

@sftim I have made the recommended changes PTAL. What other changes are needed to make this as complete?

@krol3 krol3 mentioned this pull request Nov 29, 2022
Closed
@ndixita ndixita force-pushed the kep-2133-docs branch 2 times, most recently from b9ca79b to c2ce181 Compare November 29, 2022 18:32
sftim
sftim reviewed Nov 29, 2022
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

I noticed another thing. The file content/en/docs/tasks/kubelet-credential-provider/kubelet-credential-provider.md should really be
content/en/docs/tasks/kubelet-credential-provider.md and we should configure a redirect (see static/_redirects) for the old URL.

I added inline feedback.

content/en/docs/concepts/containers/images.md Outdated Show resolved Hide resolved
content/en/docs/concepts/containers/images.md Outdated Show resolved Hide resolved
content/en/docs/concepts/containers/images.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 422a5cf63872df1e7b108e30ba2adbdd96bd4f48

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@mrunalp
Copy link
Contributor

mrunalp commented Dec 5, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 5aa606e769d16b46c8b4636a26315a8ae3e9d6ae

@sftim
Copy link
Contributor

sftim commented Dec 5, 2022

@tengqm, are you happy with how this previews now?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2022
@sftim
Copy link
Contributor

sftim commented Dec 5, 2022

@ndixita: when you rebase this, would you be willing to squash it down to 1 or 2 commits as well?

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@k8s-ci-robot k8s-ci-robot requested a review from mrunalp December 5, 2022 22:36
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2022
@mickeyboxell
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 812fc1b615c0ba9037a8d313b55a236ce98eab2c

@ndixita
Updating the content and references in the docs for Credential Provider
2b003da 
GA

Signed-off-by: Dixita Narang <ndixita@google.com>
@tengqm
Copy link
Contributor

tengqm commented Dec 6, 2022

/lgtm
/approve
Thanks.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2022
@k8s-ci-robot k8s-ci-robot merged commit 2df2cb5 into kubernetes:dev-1.26 Dec 6, 2022
This was referenced Dec 14, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewsykim andrewsykim andrewsykim left review comments

@tengqm tengqm tengqm left review comments

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@ruiwen-zhao ruiwen-zhao Awaiting requested review from ruiwen-zhao

@sftim sftim Awaiting requested review from sftim

@mrunalp mrunalp Awaiting requested review from mrunalp

Assignees

@mrunalp mrunalp

@tengqm tengqm

@ruiwen-zhao ruiwen-zhao

@derekwaynecarr derekwaynecarr

@mickeyboxell mickeyboxell

@sftim sftim

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
1.26
Development

Successfully merging this pull request may close these issues.