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

outline files kubelet uses #47425

Merged
merged 2 commits into from
Oct 8, 2024
Merged

outline files kubelet uses #47425

merged 2 commits into from
Oct 8, 2024

Conversation

SergeyKanzhelev
Copy link
Member

Description

Refrence page on files being used and written by the kubelet.

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 10, 2024
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Aug 10, 2024
@k8s-ci-robot k8s-ci-robot requested a review from tengqm August 10, 2024 21:05
@SergeyKanzhelev SergeyKanzhelev mentioned this pull request Aug 10, 2024
Open
@netlify Netlify
Copy link

netlify bot commented Aug 10, 2024 •
edited
Loading

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 598a418
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/66e9d30d625443000850c7de
😎 Deploy Preview https://deploy-preview-47425--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

tengqm
tengqm reviewed Aug 11, 2024
View reviewed changes
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot requested a review from ffromani August 11, 2024 04:09
sftim
sftim reviewed Aug 11, 2024
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.

One bit of feedback to bear in mind.

content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
@johnnyboystar22 johnnyboystar22 mentioned this pull request Aug 12, 2024
Closed
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 12, 2024
@sftim
Copy link
Contributor

sftim commented Aug 13, 2024

Hello Tide. This may now be mergeable.

Hello Sergey. Would you be willing to rebase against main anyway? We've had a minor release since you opened this and there is scope to update.

@SergeyKanzhelev
Copy link
Member Author

rebased

sftim
sftim reviewed Aug 13, 2024
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.

I've suggested some more updates.

We should get SIG Windows to provide a tech review, as Windows doesn't use POSIX conventions for paths.

content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
@SergeyKanzhelev
Copy link
Member Author

SergeyKanzhelev commented Aug 13, 2024 •
edited
Loading

We should get SIG Windows to provide a tech review, as Windows doesn't use POSIX conventions for paths.

Do we have any precedence of saying that this is Linux documentation and Windows is coming up? So somebody from SIG Windows can send the PR later?

@sftim
Copy link
Contributor

sftim commented Aug 13, 2024

Do we have any precedence of saying that this is Linux documentation and Windows is coming up? So somebody from SIG Windows can send the PR later?

No; [as a Kubernetes docs reviewer] I've always aimed to be neutral about documenting details and not picked one OS to favor over another.

@SergeyKanzhelev
Copy link
Member Author

@bart0sh can you re-review/lgtm?

@SergeyKanzhelev
Copy link
Member Author

In the preview, the note shortcodes are not rendering properly. See my suggestions Preview: https://deploy-preview-47425--kubernetes-io-main-staging.netlify.app/docs/reference/node/kubelet-files/

this is fixed now. Thank you for noticing!

@SergeyKanzhelev SergeyKanzhelev force-pushed the kubelet-files branch 2 times, most recently from de43b29 to 2c5df8a Compare September 17, 2024 06:38
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 17, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: c94388726fa56b77f29d84b9b7add58f0757d09e

@sftim sftim mentioned this pull request Sep 17, 2024
Merged
@sftim
Copy link
Contributor

sftim commented Sep 17, 2024

PR #47967 also mentions a path used for local (Localhost) seccomp profiles

@SergeyKanzhelev
Copy link
Member Author

SergeyKanzhelev commented Sep 17, 2024 •
edited
Loading

PR #47967 also mentions a path used for local (Localhost) seccomp profiles

I view it mostly as something that Pod reference, not what kubelet depends on. Similar how one can mount some host path

@sftim
Copy link
Contributor

sftim commented Sep 17, 2024

I view it mostly as something that Pod reference, not what kubelet depends on. Similar how one can mount some host path

References should be complete, so if there is a path on a node that people might need to know about, we should mention it. Especially if that path is associated with the kubelet rather than a particular container runtime.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 17, 2024
@SergeyKanzhelev
Copy link
Member Author

I view it mostly as something that Pod reference, not what kubelet depends on. Similar how one can mount some host path

References should be complete, so if there is a path on a node that people might need to know about, we should mention it. Especially if that path is associated with the kubelet rather than a particular container runtime.

Yes, makes sense to mention the folder. I also added a small note about AppArmor

sftim
sftim reviewed Sep 17, 2024
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.

This is good!

We need to
/hold
it until #47967 merges, though.

(OK to LGTM and approve it in the meantime).

content/en/docs/reference/node/kubelet-files.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 17, 2024
@sftim
Copy link
Contributor

sftim commented Sep 17, 2024 •
edited
Loading

LGTM from #47425 (comment)

/lgtm
/approve

🛑 Do not unhold until PR #47967 has merged

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

LGTM label has been added.

Git tree hash: 104f57f09903a10013ccf5246ee85eae42b2cfed

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knabben, sftim

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 Sep 17, 2024
@SergeyKanzhelev
Copy link
Member Author

/unhold

#47967 is merged

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2024
@k8s-ci-robot k8s-ci-robot merged commit 7fb8441 into kubernetes:main Oct 8, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reylejano reylejano reylejano left review comments

@sftim sftim sftim left review comments

@tengqm tengqm tengqm left review comments

@BenTheElder BenTheElder BenTheElder left review comments

@knabben knabben knabben approved these changes

@divya-mohan0209 divya-mohan0209 Awaiting requested review from divya-mohan0209

@ffromani ffromani Awaiting requested review from ffromani

@jsturtevant jsturtevant Awaiting requested review from jsturtevant

@marosset marosset Awaiting requested review from marosset

@bart0sh bart0sh Awaiting requested review from bart0sh

Assignees

@jsturtevant jsturtevant

@bart0sh bart0sh

@marosset marosset

@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/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
SIG Node: code and documentation PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@SergeyKanzhelev @sftim @marosset @k8s-ci-robot @reylejano @jsturtevant @BenTheElder @knabben @tengqm @bart0sh