Skip to content

Use Kueue config for populator namespace selector#11218

Open
MatteoFari wants to merge 1 commit into
kubernetes-sigs:mainfrom
MatteoFari:7852-kueue-populator-reuse-kueue-config
Open

Use Kueue config for populator namespace selector#11218
MatteoFari wants to merge 1 commit into
kubernetes-sigs:mainfrom
MatteoFari:7852-kueue-populator-reuse-kueue-config

Conversation

@MatteoFari
Copy link
Copy Markdown
Contributor

@MatteoFari MatteoFari commented May 15, 2026 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

kueue-populator now uses the same managedJobsNamespaceSelector as Kueue.

The Helm chart copies managedJobsNamespaceSelector from Kueue’s manager config into the existing populator config, instead of keeping a separate Helm value.

Which issue(s) this PR fixes:

Fixes #7852

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kueue-populator now uses Kueue's managedJobsNamespaceSelector from the Kueue manager configuration instead of a separate Helm value.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels May 15, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 15, 2026

@MatteoFari: The label(s) area/kueue-populator cannot be applied, because the repository doesn't have them.

Details

In response to this:

What type of PR is this?

/kind feature
/area kueue-populator

What this PR does / why we need it:

kueue-populator now reads Kueue’s managedJobsNamespaceSelector from the Kueue manager config instead of using a separate Helm value.

The old populator selector still works for manual installs when kueueConfigPath is not set.

Which issue(s) this PR fixes:

Fixes #7852

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kueue-populator now reuses Kueue's managedJobsNamespaceSelector from the Kueue manager configuration when kueueConfigPath is configured.

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.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 15, 2026 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit f28aa70
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6a0b076f1c3d3f000813f13c

@k8s-ci-robot k8s-ci-robot requested review from PBundyra and olekzabl May 15, 2026 09:12
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 15, 2026
kannon92
kannon92 reviewed May 15, 2026
View reviewed changes
Comment thread cmd/experimental/kueue-populator/charts/kueue-populator/templates/manager.yaml Outdated
{{- $kueueConfig := $defaultKueueConfig -}}
{{- with .Values.kueue }}
{{- with .managerConfig }}
{{- $kueueConfig = .controllerManagerConfigYaml | default $defaultKueueConfig -}}
Copy link
Copy Markdown
Contributor

@kannon92 kannon92 May 15, 2026

Choose a reason for hiding this comment

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

Honestly this is quite a lot of API to bring in just for one field.

Should we just expose the managedNamespaceSelector in the config and read it from the config map for Kueue?

Copy link
Copy Markdown
Contributor Author

@MatteoFari MatteoFari May 18, 2026

Choose a reason for hiding this comment

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

Yeah it's better, the chart now reads Kueue’s manager config and writes its managedJobsNamespaceSelector into the existing populator config field

@MatteoFari MatteoFari force-pushed the 7852-kueue-populator-reuse-kueue-config branch from 18d2af6 to e43356d Compare May 18, 2026 09:41
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 18, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MatteoFari
Once this PR has been reviewed and has the lgtm label, please assign mimowo for approval. For more information see the 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

mimowo
mimowo reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mimowo mimowo left a comment

Choose a reason for hiding this comment

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

cc @j-skiba as the original author of kueue-populator: #11218

j-skiba
j-skiba reviewed May 18, 2026
View reviewed changes
Comment thread cmd/experimental/kueue-populator/charts/kueue-populator/tests/manager_test.yaml Outdated
@j-skiba
Copy link
Copy Markdown
Contributor

j-skiba commented May 18, 2026

/lgtm
one nit, looks good overall

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

k8s-ci-robot commented May 18, 2026

LGTM label has been added.

DetailsGit tree hash: 50756caed3da26fbdc0bd1d600ebc3b3df27fa6e

@MatteoFari
Use Kueue config for populator namespace selector
f28aa70 
Signed-off-by: Matteo Fari <matteofari06@gmail.com>
@MatteoFari MatteoFari force-pushed the 7852-kueue-populator-reuse-kueue-config branch from e43356d to f28aa70 Compare May 18, 2026 12:34
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 18, 2026
@k8s-ci-robot k8s-ci-robot requested a review from j-skiba May 18, 2026 12:34
mimowo
mimowo reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mimowo mimowo left a comment

Choose a reason for hiding this comment

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

Thank you, reading the namespace selector from Kueue's configuration sounds great, because they should be equal most of the time. However, do we support still setting the managedJobsNamespaceSelector field explicitily in the Kueue populator config? There might still exist a use case when the Kueue-populator wants to restrict the set of namespaces it operates on. If this remains supported, do we have a test to demonstrate that?

@j-skiba
Copy link
Copy Markdown
Contributor

j-skiba commented May 18, 2026

/lgtm

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

k8s-ci-robot commented May 18, 2026

LGTM label has been added.

DetailsGit tree hash: 4e1af8c2e2958a2b0b5f996adfa558a9999de2eb

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

Reviewers

@mimowo mimowo mimowo left review comments

@olekzabl olekzabl Awaiting requested review from olekzabl

@PBundyra PBundyra Awaiting requested review from PBundyra

@j-skiba j-skiba Awaiting requested review from j-skiba

+1 more reviewer

@kannon92 kannon92 kannon92 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@j-skiba j-skiba

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Kueue-populator: re-use Kueue configuration to restrict the namespaces

5 participants

@MatteoFari @k8s-ci-robot @j-skiba @kannon92 @mimowo