Filter resource prefixes #2267

pajakd · 2024-05-23T13:51:21Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a functionality to Kueue to ignore resources with certain (configurable) prefixes.

Which issue(s) this PR fixes:

Fixes #2266

Special notes for your reviewer:

Does this PR introduce a user-facing change?

You can configure Kueue to ignore container resources that match specified prefixes.

k8s-ci-robot · 2024-05-23T13:51:31Z

Hi @pajakd. Thanks for your PR.

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

netlify · 2024-05-23T13:51:40Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`bfc6b05`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/665071220a9b0200082827af

alculquicondor · 2024-05-23T18:23:46Z

/release-note-edit

You can configure Kueue to ignore container resources that match specified prefixes.

alculquicondor · 2024-05-23T18:23:56Z

/ok-to-test

alculquicondor

Let's add

apis/config/v1beta1/configuration_types.go

pkg/workload/workload.go

pkg/workload/workload_test.go

alculquicondor · 2024-05-23T18:40:36Z

test/e2e/singlecluster/pod_test.go

@@ -96,6 +96,10 @@ var _ = ginkgo.Describe("Pod groups", func() {
 				Image("gcr.io/k8s-staging-perf-tests/sleep:v0.1.0", []string{"1ms"}).
 				Queue(lq.Name).
 				Request(corev1.ResourceCPU, "1").
+				// Adding a resource that is *not* in Cluster Queue but is marked
+				// as excluded in the configuration. Testing that it passes without error.
+				Request("networking.networks/vpc1", "1").


This Pod will pass Kueue admission, but it will not run, because this is an E2E test and kube-scheduler wouldn't schedule it.

Better test at integration level.

Oh, thank you for this -- I could not figure out why e2e was failing.

Added integration testing.

alculquicondor · 2024-05-23T18:41:21Z

LGTM overall.

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

mimowo

LGTM, just nits

pkg/workload/workload.go

test/integration/controller/jobs/pod/pod_controller_test.go

pkg/queue/manager.go

mimowo

I would suggest to squash the commits.
/lgtm
/assign @tenzen-y @alculquicondor

k8s-ci-robot · 2024-05-24T09:20:54Z

LGTM label has been added.

Git tree hash: 0641aa3b9a3035ab2133649fd38c1ab11837996e

mimowo · 2024-05-24T10:42:13Z

pkg/workload/workload.go

@@ -626,5 +665,6 @@ func AdmissionChecksForWorkload(log logr.Logger, wl *kueue.Workload, admissionCh
 			}
 		}
 	}
+


non-blocking nit: revert

mimowo · 2024-05-24T11:05:12Z

/lgtm

k8s-ci-robot · 2024-05-24T11:05:17Z

LGTM label has been added.

Git tree hash: ae69eba684dc0eec92c3f550ac5bdf40b3fa7c3f

alculquicondor

/approve

k8s-ci-robot · 2024-05-24T17:10:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, pajakd

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:

~~OWNERS~~ [alculquicondor]

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

Filter resource prefixes

108d7c1

k8s-ci-robot requested review from kerthcet and mimowo May 23, 2024 13:51

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 May 23, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 23, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 23, 2024

alculquicondor reviewed May 23, 2024

View reviewed changes

pajakd and others added 7 commits May 24, 2024 06:27

Update apis/config/v1beta1/configuration_types.go

f39b8eb

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

Update apis/config/v1beta1/configuration_types.go

a0a799f

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

Update pkg/workload/workload.go

3a8fa7c

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

Update pkg/workload/workload.go

0e33dff

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

Update pkg/workload/workload_test.go

1a0cebd

Co-authored-by: Aldo Culquicondor <1299064+alculquicondor@users.noreply.github.com>

Fix unit tests, remove changes in e2e

89494d5

Integration test

3e6a326

pajakd marked this pull request as ready for review May 24, 2024 08:09

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 24, 2024

Update the generated file

4ba1ccd

mimowo reviewed May 24, 2024

View reviewed changes

pkg/workload/workload.go Outdated Show resolved Hide resolved

test/integration/controller/jobs/pod/pod_controller_test.go Outdated Show resolved Hide resolved

pkg/queue/manager.go Outdated Show resolved Hide resolved

Merge branch 'kubernetes-sigs:main' into prefixes_ignore

e7be6f1

mimowo reviewed May 24, 2024

View reviewed changes

k8s-ci-robot assigned alculquicondor, tenzen-y and mimowo May 24, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2024

Address comments, update documentation

c311271

pajakd force-pushed the prefixes_ignore branch from 8a0cf9c to c311271 Compare May 24, 2024 09:46

Update controller_manager_config.yaml

32db853

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2024

k8s-ci-robot requested review from alculquicondor, mimowo and tenzen-y May 24, 2024 09:53

mimowo reviewed May 24, 2024

View reviewed changes

Revert blank line addition

bfc6b05

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2024

alculquicondor reviewed May 24, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2024

k8s-ci-robot merged commit 3ed768e into kubernetes-sigs:main May 24, 2024
15 checks passed

k8s-ci-robot added this to the v0.7 milestone May 24, 2024

pajakd deleted the prefixes_ignore branch May 27, 2024 06:37

tenzen-y mentioned this pull request May 30, 2024

Add documentation for the excludeResourcePrefixes #2328

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter resource prefixes #2267

Filter resource prefixes #2267

pajakd commented May 23, 2024 •

edited by k8s-ci-robot

Loading

k8s-ci-robot commented May 23, 2024

netlify bot commented May 23, 2024 •

edited

Loading

alculquicondor commented May 23, 2024

alculquicondor commented May 23, 2024

alculquicondor left a comment

alculquicondor May 23, 2024

pajakd May 24, 2024

alculquicondor commented May 23, 2024

mimowo left a comment

mimowo left a comment

k8s-ci-robot commented May 24, 2024

mimowo May 24, 2024

pajakd May 24, 2024

mimowo commented May 24, 2024

k8s-ci-robot commented May 24, 2024

alculquicondor left a comment

k8s-ci-robot commented May 24, 2024

Filter resource prefixes #2267

Filter resource prefixes #2267

Conversation

pajakd commented May 23, 2024 • edited by k8s-ci-robot Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented May 23, 2024

netlify bot commented May 23, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

alculquicondor commented May 23, 2024

alculquicondor commented May 23, 2024

alculquicondor left a comment

Choose a reason for hiding this comment

alculquicondor May 23, 2024

Choose a reason for hiding this comment

pajakd May 24, 2024

Choose a reason for hiding this comment

alculquicondor commented May 23, 2024

mimowo left a comment

Choose a reason for hiding this comment

mimowo left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 24, 2024

mimowo May 24, 2024

Choose a reason for hiding this comment

pajakd May 24, 2024

Choose a reason for hiding this comment

mimowo commented May 24, 2024

k8s-ci-robot commented May 24, 2024

alculquicondor left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 24, 2024

pajakd commented May 23, 2024 •

edited by k8s-ci-robot

Loading

netlify bot commented May 23, 2024 •

edited

Loading