support namespace indexer for namespaced resources like pods #121906

ahutsunshine · 2023-11-15T13:38:01Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

In our cluster, the number of some resources like pods are very large(10w+). When listing a namespace resources like pods from apiserver cache, the apiserver process will firstly get all the resources in all the namespaces and then filter the resources by namespace predicate even it's a very small request. It's heavy for filtering namespaced resource even the resource number in the namespace is very small with many client requests. It consumes lots of cpu & memory that's not expected.

Which issue(s) this PR fixes:

feature support and discussion: #120778

NONE

linux-foundation-easycla · 2023-11-15T13:38:06Z

The committers listed above are authorized under a signed CLA.

✅ login: ahutsunshine / name: ahutsunshine (d8bd150)

k8s-ci-robot · 2023-11-15T13:38:10Z

Welcome @ahutsunshine!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-11-15T13:38:11Z

Hi @ahutsunshine. Thanks for your PR.

I'm waiting for a kubernetes 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.

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.

HirazawaUi · 2023-11-15T14:59:04Z

If we need to add a feature gate, kep is a good place to start, but before we do that, I want to know if we really need it

ahutsunshine · 2023-11-17T03:38:17Z

@HirazawaUi We do a benchmark and see perfect performance to improve latency&cpu&memory. I proposal this because listing the pods across various namespaces significantly impacts CPU and memory resources and causes the incidents in our production. You can check more #120778 and we can discuss the feature/enhancements.

bart0sh · 2023-11-24T16:21:35Z

Thank you for your PR. Please sign the CLA to proceed further, thanks.

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go

halfcrazy · 2023-11-26T02:15:29Z

The namespace indexer only adds to some in-tree resources. Could this work for custom resources as well?

jiahuif · 2023-11-28T21:28:22Z

Also, pinging for CLA.
/triage accepted

ahutsunshine · 2023-11-30T06:07:46Z

@wojtek-t @halfcrazy pls go ahead to review

wojtek-t

Just one minor nit - other than that LGTM.

staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go

wojtek-t · 2023-11-30T10:14:32Z

LGTM - please squash the commits and I will approve.

fix comments optimize code small optimization for the namespace scope check

ahutsunshine · 2023-11-30T10:51:58Z

LGTM - please squash the commits and I will approve.

Really appreciate your review and recommendations. Squash the commits and commit.

wojtek-t · 2023-11-30T11:07:49Z

/lgtm
/approve

k8s-ci-robot · 2023-11-30T11:07:57Z

LGTM label has been added.

Git tree hash: 11fb4d30b8ef149f79fa967d27afae67d3d92284

k8s-ci-robot · 2023-11-30T11:08:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahutsunshine, wojtek-t

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:

~~pkg/features/OWNERS~~ [wojtek-t]
~~pkg/registry/core/pod/OWNERS~~ [wojtek-t]
~~staging/src/k8s.io/apiserver/pkg/storage/OWNERS~~ [wojtek-t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ahutsunshine · 2023-11-30T12:59:21Z

@halfcrazy @caesarxuchao @deads2k Feel free to review the PR if necessary. @wojtek-t gives LGTM and approval.

halfcrazy · 2023-11-30T13:01:38Z

/lgtm

k8s-ci-robot · 2023-11-30T13:01:44Z

@halfcrazy: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

ahutsunshine · 2023-11-30T14:08:30Z

/milestone v1.29

k8s-ci-robot · 2023-11-30T14:08:33Z

@ahutsunshine: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v1.29

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.

wojtek-t · 2023-11-30T14:43:23Z

No - we're very deep in code freeze (right before release) and this shouldn't go to 1.29.
This will merge once the branch opens after release.

bart0sh · 2023-12-04T12:43:13Z

/priority important-longterm

MadhavJivrajani · 2024-01-09T09:40:41Z

Considering this adds in a feature gate to control the behaviour, its probably worth adding a release note since we aren't going down the KEP route.

MadhavJivrajani · 2024-01-09T09:46:53Z

pkg/features/kube_features.go

@@ -975,6 +975,12 @@ const (
 	// will not graduate or be enabled by default in future Kubernetes
 	// releases.
 	UserNamespacesPodSecurityStandards featuregate.Feature = "UserNamespacesPodSecurityStandards"
+
+	// owner: @ahutsunshine
+	// beta: v1.29


This is probably a mistake since the PR ended up going in after code freeze for 1.29 was lifted (i.e. in the 1.30 release)?

@MadhavJivrajani Thanks for your reminding. Yeah, it's a mistake. Shall we file a PR to fix it?

Yes please do! Thank you!

PR: #122732 to fix version typo

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not 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. labels Nov 15, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 15, 2023

k8s-ci-robot requested review from caesarxuchao and deads2k November 15, 2023 13:39

SergeyKanzhelev added this to Triage in SIG Node PR Triage Nov 15, 2023

bart0sh moved this from Triage to Waiting on Author in SIG Node PR Triage Nov 24, 2023

halfcrazy reviewed Nov 26, 2023

View reviewed changes

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go Outdated Show resolved Hide resolved

wojtek-t self-assigned this Nov 28, 2023

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 Nov 28, 2023

ahutsunshine force-pushed the namespace-indexer branch 3 times, most recently from 0eacdd6 to 421519d Compare November 30, 2023 09:00

wojtek-t reviewed Nov 30, 2023

View reviewed changes

staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go Outdated Show resolved Hide resolved

support pod namespace indexer

d8bd150

fix comments optimize code small optimization for the namespace scope check

ahutsunshine force-pushed the namespace-indexer branch from 4f6ac82 to d8bd150 Compare November 30, 2023 10:39

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2023

k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 4, 2023

bart0sh moved this from Triage to Needs Approver in SIG Node PR Triage Dec 4, 2023

k8s-ci-robot merged commit 2eb2a62 into kubernetes:master Dec 13, 2023
15 checks passed

SIG Node PR Triage automation moved this from Needs Approver to Done Dec 13, 2023

k8s-ci-robot added this to the v1.30 milestone Dec 13, 2023

MadhavJivrajani reviewed Jan 9, 2024

View reviewed changes

ahutsunshine mentioned this pull request Jan 12, 2024

fix version typo of pod namespace indexer #122732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support namespace indexer for namespaced resources like pods #121906

support namespace indexer for namespaced resources like pods #121906

ahutsunshine commented Nov 15, 2023 •

edited by wojtek-t

linux-foundation-easycla bot commented Nov 15, 2023 •

edited

k8s-ci-robot commented Nov 15, 2023

k8s-ci-robot commented Nov 15, 2023

HirazawaUi commented Nov 15, 2023

ahutsunshine commented Nov 17, 2023

bart0sh commented Nov 24, 2023

halfcrazy commented Nov 26, 2023

jiahuif commented Nov 28, 2023

ahutsunshine commented Nov 30, 2023

wojtek-t left a comment

wojtek-t commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023 •

edited

wojtek-t commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023

halfcrazy commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

wojtek-t commented Nov 30, 2023

bart0sh commented Dec 4, 2023

MadhavJivrajani commented Jan 9, 2024 •

edited

MadhavJivrajani Jan 9, 2024

ahutsunshine Jan 11, 2024

MadhavJivrajani Jan 11, 2024

ahutsunshine Jan 12, 2024 •

edited

support namespace indexer for namespaced resources like pods #121906

support namespace indexer for namespaced resources like pods #121906

Conversation

ahutsunshine commented Nov 15, 2023 • edited by wojtek-t

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

linux-foundation-easycla bot commented Nov 15, 2023 • edited

k8s-ci-robot commented Nov 15, 2023

k8s-ci-robot commented Nov 15, 2023

HirazawaUi commented Nov 15, 2023

ahutsunshine commented Nov 17, 2023

bart0sh commented Nov 24, 2023

halfcrazy commented Nov 26, 2023

jiahuif commented Nov 28, 2023

ahutsunshine commented Nov 30, 2023

wojtek-t left a comment

Choose a reason for hiding this comment

wojtek-t commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023 • edited

wojtek-t commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023

halfcrazy commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

ahutsunshine commented Nov 30, 2023

k8s-ci-robot commented Nov 30, 2023

wojtek-t commented Nov 30, 2023

bart0sh commented Dec 4, 2023

MadhavJivrajani commented Jan 9, 2024 • edited

MadhavJivrajani Jan 9, 2024

Choose a reason for hiding this comment

ahutsunshine Jan 11, 2024

Choose a reason for hiding this comment

MadhavJivrajani Jan 11, 2024

Choose a reason for hiding this comment

ahutsunshine Jan 12, 2024 • edited

Choose a reason for hiding this comment

ahutsunshine commented Nov 15, 2023 •

edited by wojtek-t

linux-foundation-easycla bot commented Nov 15, 2023 •

edited

ahutsunshine commented Nov 30, 2023 •

edited

MadhavJivrajani commented Jan 9, 2024 •

edited

ahutsunshine Jan 12, 2024 •

edited