Skip to content

kubelet: update documentation of the cgroupDriver setting#119441

Closed
marquiz wants to merge 1 commit intokubernetes:masterfrom
marquiz:devel/cgroup-driver-autoconfig
Closed

kubelet: update documentation of the cgroupDriver setting#119441
marquiz wants to merge 1 commit intokubernetes:masterfrom
marquiz:devel/cgroup-driver-autoconfig

Conversation

@marquiz
Copy link
Copy Markdown
Contributor

@marquiz marquiz commented Jul 19, 2023

What type of PR is this?

/kind documentation

What this PR does / why we need it:

Update the help text of the kubelet's cgroupDriver config setting (and the --cgroup-driver flag) to descrivbe

Which issue(s) this PR fixes:

Special notes for your reviewer:

Complements #118770

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 19, 2023
@marquiz
Copy link
Copy Markdown
Contributor Author

marquiz commented Jul 19, 2023

/cc @haircommander @SergeyKanzhelev

@k8s-ci-robot k8s-ci-robot added area/kubelet kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 19, 2023
@haircommander
Copy link
Copy Markdown
Contributor

haircommander commented Jul 19, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 19, 2023
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jul 19, 2023

LGTM label has been added.

DetailsGit tree hash: 05037cfe43191e35281d6a83b34a5dc2ba56eed4

@k8s-triage-robot
Copy link
Copy Markdown

k8s-triage-robot commented Jul 19, 2023

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@marquiz marquiz mentioned this pull request Jul 19, 2023
Closed
13 tasks
@bart0sh
Copy link
Copy Markdown
Contributor

bart0sh commented Jul 19, 2023

/triage accepted
/priority important-longterm
/retest

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 19, 2023
@marquiz marquiz mentioned this pull request Jul 24, 2023
Merged
@marquiz
Copy link
Copy Markdown
Contributor Author

marquiz commented Jul 25, 2023

/assign @mrunalp

@k8s-ci-robot k8s-ci-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jan 25, 2024
@bart0sh
Copy link
Copy Markdown
Contributor

bart0sh commented Jan 25, 2024

@marquiz please run hack/update-codegen.sh as suggested by @pacoxu above.
This should fix CI failure.

otherwise lgtm

@bart0sh
Copy link
Copy Markdown
Contributor

bart0sh commented Jan 25, 2024

/cc

@k8s-ci-robot k8s-ci-robot requested a review from bart0sh January 25, 2024 12:31
@marquiz marquiz force-pushed the devel/cgroup-driver-autoconfig branch from 1962a1a to 16c4ad4 Compare January 25, 2024 15:59
@k8s-ci-robot k8s-ci-robot added area/code-generation kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Jan 25, 2024
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jan 25, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: marquiz
Once this PR has been reviewed and has the lgtm label, please assign liggitt for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@marquiz
Copy link
Copy Markdown
Contributor Author

marquiz commented Jan 25, 2024

please run hack/update-codegen.sh as suggested by

Thanks @bart0sh for pointing this out. Fixed.

I don't know how I've managed to turn down my own belowed PR

ping also @pacoxu

bart0sh
bart0sh reviewed Jan 25, 2024
View reviewed changes
Comment thread cmd/kubelet/app/options/options.go Outdated

fs.BoolVar(&c.CgroupsPerQOS, "cgroups-per-qos", c.CgroupsPerQOS, "Enable creation of QoS cgroup hierarchy, if true top level QoS and pod cgroups are created.")
fs.StringVar(&c.CgroupDriver, "cgroup-driver", c.CgroupDriver, "Driver that the kubelet uses to manipulate cgroups on the host. Possible values: 'cgroupfs', 'systemd'")
fs.StringVar(&c.CgroupDriver, "cgroup-driver", c.CgroupDriver, "Driver that the kubelet uses to manipulate cgroups on the host. Possible values: 'cgroupfs', 'systemd'. Note: ignored if the KubeletCgroupDriverFromCRI feature gate is enabled and a sufficiently new version of a container runtime is used, in which case the correct cgroup driver is automatically detected.")
Copy link
Copy Markdown
Contributor

@bart0sh bart0sh Jan 25, 2024

Choose a reason for hiding this comment

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

Can you be more specific about a container runtime version or re-frase this somehow? Do you mean the version that supports certain CRI API?

Copy link
Copy Markdown
Contributor Author

@marquiz marquiz Jan 26, 2024

Choose a reason for hiding this comment

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

Yes, this is about a version of the container runtime that supports the RuntimeConfig CRI rpc. In practice this means CRI-O v1.28+ and containerd 2.0+ (2.0 is not released yet).

Sure, I can reword it. I feel that talking about RuntimeConfig rpc is not useful information to the end-user. Do we want to pintpoint specific versions of container runtimes here? WDYT?

Examples:

Note: ignored if the KubeletCgroupDriverFromCRI feature gate is enabled and a container runtime supporting RuntimeConfig rpm is used, in which case the correct cgroup driver is automatically detected.")

OR

Note: ignored if the KubeletCgroupDriverFromCRI feature gate is enabled and a sufficiently new version of a container runtime is used (containerd v2.0 or later, cri-o v1.28 or later), in which case the correct cgroup driver is automatically detected.")

Copy link
Copy Markdown
Contributor

@bart0sh bart0sh Jan 26, 2024

Choose a reason for hiding this comment

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

Thank you. I like the former better, except rpm thing. I don't understand it. Is it a typo and you meant rpc?
Can we use CRI API or something similar instead?

Copy link
Copy Markdown
Contributor Author

@marquiz marquiz Jan 26, 2024

Choose a reason for hiding this comment

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

Argh, my fat fingers and brain, rpm vs rpc 🙈

Changed to

Note: ignored if the KubeletCgroupDriverFromCRI feature gate is enabled and a container runtime supporting the RuntimeConfig CRI rpc is used, in which case the correct cgroup driver is automatically detected.

ping @sftim
you have educated opinions on these
/cc sftim

Copy link
Copy Markdown
Contributor

@bart0sh bart0sh Jan 26, 2024

Choose a reason for hiding this comment

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

Your last edition looks good to me. Please, update the PR and I'll lgtm it.

@marquiz marquiz force-pushed the devel/cgroup-driver-autoconfig branch from 16c4ad4 to d9bba2f Compare January 26, 2024 11:03
@k8s-ci-robot k8s-ci-robot requested a review from sftim January 26, 2024 11:04
@bart0sh
Copy link
Copy Markdown
Contributor

bart0sh commented Jan 26, 2024

/lgtm

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

k8s-ci-robot commented Jan 26, 2024

LGTM label has been added.

DetailsGit tree hash: 3c41a8aab24dd038ca53cce19a10a46e0e34c45a

@bart0sh
Copy link
Copy Markdown
Contributor

bart0sh commented Jan 26, 2024

/assign @mrunalp @derekwaynecarr @dchen1107
for a final approval

@k8s-triage-robot
Copy link
Copy Markdown

k8s-triage-robot commented Apr 25, 2024

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 25, 2024
@k8s-triage-robot
Copy link
Copy Markdown

k8s-triage-robot commented May 25, 2024

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle rotten
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 25, 2024
@dims
Copy link
Copy Markdown
Member

dims commented May 26, 2024

@marquiz @SergeyKanzhelev i see the note to bump this feature to beta in kubernetes/enhancements#4033 (comment)

is that in another PR or will this be updated?

@k8s-triage-robot
Copy link
Copy Markdown

k8s-triage-robot commented Jun 25, 2024

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jun 25, 2024

@k8s-triage-robot: Closed this PR.

Details

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haircommander haircommander Awaiting requested review from haircommander

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@odinuge odinuge Awaiting requested review from odinuge

@mrunalp mrunalp Awaiting requested review from mrunalp

@sftim sftim Awaiting requested review from sftim

1 more reviewer

@bart0sh bart0sh bart0sh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mrunalp mrunalp

@derekwaynecarr derekwaynecarr

@dchen1107 dchen1107

@haircommander haircommander

@bart0sh bart0sh

Labels

area/code-generation area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

SIG Node: code and documentation PRs
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.