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.

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

Promote Local storage capacity isolation feature to GA #111513

Merged
merged 1 commit into from Aug 3, 2022
Merged

Promote Local storage capacity isolation feature to GA #111513

merged 1 commit into from Aug 3, 2022

Conversation

jingxu97
Copy link
Contributor

This change is to promote local storage capacity isolation feature to GA

At the same time, to allow rootless system disable this feature due to
unable to get root fs, this change introduced a new kubelet config
"localStorageCapacityIsolation". By default it is set to true. For
rootless systems, they can set this configuration to false to disable
the feature. Once it is set, user cannot set ephemeral-storage
request/limit because capacity and allocatable will not be set.

Change-Id: I48a52e737c6a09e9131454db6ad31247b56c000a

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Local Storage Capacity Isolation feature is GA in 1.25 release. For systems (rootless) that cannot check root file system, please use kubelet config --local-storage-capacity-isolation=false to disable this feature. Once disabled, pod cannot set local ephemeral storage request/limit, and emptyDir sizeLimit niether.

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

- [KEP]: https://github.com/kubernetes/enhancements/pull/3422

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. 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 28, 2022
@k8s-ci-robot k8s-ci-robot added area/code-generation area/kubelet 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. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 28, 2022
@jingxu97
Copy link
Contributor Author

cc @msau42

@leilajal
Copy link
Contributor

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jul 28, 2022
@k8s-triage-robot
Copy link

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.

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jul 28, 2022
@k8s-ci-robot k8s-ci-robot added the sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. label Jul 28, 2022
@jingxu97 jingxu97 force-pushed the july/localstorage branch 2 times, most recently from 14c7a41 to 0cc6e56 Compare July 29, 2022 17:43
@msau42
Copy link
Member

msau42 commented Aug 3, 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 Aug 3, 2022
@BenTheElder
Copy link
Member

/retest

@aojea
Copy link
Member

aojea commented Aug 3, 2022

the failure in pull-kubernetes-verify is legit, it needs to regenerate some files

@BenTheElder
Copy link
Member

Yes, but for example capz-windows is pure noise apparently https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/111513/pull-kubernetes-e2e-capz-windows-containerd/1554678217585987584

ERROR: AADSTS7000222: The provided client secret keys for app 'a46d7624-de73-47f7-addb-4d2993a9b10c' are expired. Visit the Azure portal to create new keys for your app: https://aka.ms/NewClientSecret, or consider using certificate credentials for added security: https://aka.ms/certCreds.

@aojea
Copy link
Member

aojea commented Aug 3, 2022

Yes, but for example capz-windows is pure noise apparently https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/111513/pull-kubernetes-e2e-capz-windows-containerd/1554678217585987584

ERROR: AADSTS7000222: The provided client secret keys for app 'a46d7624-de73-47f7-addb-4d2993a9b10c' are expired. Visit the Azure portal to create new keys for your app: https://aka.ms/NewClientSecret, or consider using certificate credentials for added security: https://aka.ms/certCreds.

that job is not in my list of "worth to check failures" ;) see the job history https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-e2e-capz-windows-containerd

@jingxu97
Promote Local storage capacity isolation feature to GA
0064010 
This change is to promote local storage capacity isolation feature to GA

At the same time, to allow rootless system disable this feature due to
unable to get root fs, this change introduced a new kubelet config
"localStorageCapacityIsolation". By default it is set to true. For
rootless systems, they can set this configuration to false to disable
the feature. Once it is set, user cannot set ephemeral-storage
request/limit because capacity and allocatable will not be set.

Change-Id: I48a52e737c6a09e9131454db6ad31247b56c000a
@k8s-triage-robot
Copy link

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

1 similar comment
@k8s-triage-robot
Copy link

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

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

jingxu97 commented Aug 3, 2022

Yes, but for example capz-windows is pure noise apparently https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/111513/pull-kubernetes-e2e-capz-windows-containerd/1554678217585987584

ERROR: AADSTS7000222: The provided client secret keys for app 'a46d7624-de73-47f7-addb-4d2993a9b10c' are expired. Visit the Azure portal to create new keys for your app: https://aka.ms/NewClientSecret, or consider using certificate credentials for added security: https://aka.ms/certCreds.

that job is not in my list of "worth to check failures" ;) see the job history https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-e2e-capz-windows-containerd

Yes, I updated the file, hope it works this time. Thanks!

@thockin
Copy link
Member

thockin commented Aug 3, 2022

Confirmed only regenerated files.

/lgtm

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

jingxu97 commented Aug 3, 2022

/retest

@liggitt liggitt removed their assignment Aug 3, 2022
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Aug 3, 2022 •
edited

@jingxu97: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-local-e2e 0064010 link false /test pull-kubernetes-local-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@BenTheElder
Copy link
Member

/skip

@k8s-ci-robot k8s-ci-robot merged commit 442574f into kubernetes:master Aug 3, 2022
This was referenced Aug 3, 2022
Closed
Closed
Open
Closed
Open
Closed
@cici37
Copy link
Contributor

cici37 commented Aug 4, 2022

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 4, 2022
@SataQiu SataQiu mentioned this pull request Dec 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtaufen mtaufen mtaufen left review comments

@BenTheElder BenTheElder BenTheElder left review comments

@liggitt liggitt liggitt left review comments

@alculquicondor alculquicondor alculquicondor left review comments

@Huang-Wei Huang-Wei Huang-Wei left review comments

@thockin thockin thockin left review comments

@msau42 msau42 msau42 left review comments

@denkensk denkensk Awaiting requested review from denkensk

@feiskyer feiskyer Awaiting requested review from feiskyer

Assignees

@BenTheElder BenTheElder

@alculquicondor alculquicondor

@Huang-Wei Huang-Wei

@thockin thockin

@derekwaynecarr derekwaynecarr

@dchen1107 dchen1107

@msau42 msau42

@mattcary mattcary

Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. 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 lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
API Reviews
Status: API review completed, 1.25
Milestone
v1.25
Development

Successfully merging this pull request may close these issues.

None yet