Add a visibility API endpoint for fetching pending workloads in a Local Queue #1365

PBundyra · 2023-11-24T14:15:22Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

This PR continues the development of KEP#168-2

Which issue(s) this PR fixes:

Special notes for your reviewer:

I have manually tested RBAC.

Does this PR introduce a user-facing change?

Visibility API: Add an endpoint that allows a user to fetch information about pending workloads and their position in LocalQueue.

netlify · 2023-11-24T14:15:29Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`4f36bd3`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/65670dfbccfeff00086f6268

k8s-ci-robot · 2023-11-24T14:15:32Z

Hi @PBundyra. 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/test-infra repository.

PBundyra · 2023-11-24T14:15:39Z

/assign @mimowo

mimowo · 2023-11-24T15:31:35Z

/ok-to-test

mimowo

Generally LGTM, couple nits, yet to review the e2e test

pkg/visibility/api/install.go

pkg/visibility/api/rest/local_queue.go

pkg/visibility/api/rest/pending_workloads_LQ.go

test/e2e/e2e_test.go

PBundyra · 2023-11-24T16:29:22Z

I've manually tested RBACs, and they seem to work well

charts/kueue/templates/rbac/pending_workloads_CQ_viewer_role.yaml

PBundyra · 2023-11-27T13:54:58Z

/assign @alculquicondor
/assign @tenzen-y

apis/visibility/v1alpha1/types.go

charts/kueue/templates/rbac/pending_workloads_LQ_viewer_role.yaml

config/components/rbac/kustomization.yaml

pkg/queue/manager.go

pkg/visibility/api/rest/pending_workloads_LQ.go

config/components/manager/kustomization.yaml

apis/visibility/v1alpha1/types.go

pkg/visibility/api/rest/pending_workloads_lq_test.go

PBundyra · 2023-11-28T13:16:07Z

/assign @mwielgus

mimowo · 2023-11-28T13:46:56Z

/lgtm
/assign @alculquicondor @tenzen-y

k8s-ci-robot · 2023-11-28T13:47:03Z

LGTM label has been added.

Git tree hash: 3516af411872fefe0ecba20653eb4f979f2664d0

PBundyra · 2023-11-28T13:49:58Z

/retest

tenzen-y · 2023-11-29T03:27:44Z

/release-note-edit

Extend visibility API by adding an endpoint that allows a user to fetch information about pending workloads and their position in LocalQueue.

tenzen-y

Generally, lgtm

pkg/visibility/api/rest/pending_workloads_lq_test.go

test/e2e/e2e_test.go

tenzen-y · 2023-11-29T04:48:39Z

test/e2e/e2e_test.go

+			})
+		})
+
+		ginkgo.It("Should allow fetching information about position of pending workloads from different LocalQueues", func() {


Can we move this and "Should allow fetching information about position of pending workloads from different LocalQueues from different Namespaces" to integration tests?

I'd like to make e2e lightweight as much as possible.

Currently, integration tests for the visibility server are under development. I definitely agree with the idea of keeping e2e as lightweight as possible, but I suggest keeping them as they are until we have integration tests ready.

Yea, @PBundyra already had one attempt to add integration tests, but it proved to be challenging, because it requires setting up kubeconfig / certificates for communication between the main API server and the visibility extension. Also, there is no good example, because the metrics-server only uses e2e tests.

So, we went with e2e tests until the challenges with integration tests are figured out.

Also, we put an effort into running the tests as quickly as possible, in particular, we don't use the 5s sleep.

Thanks for the clarification. Introducing integration tests seems to be hard work. So we can work on these follow-ups.

mimowo · 2023-11-29T10:07:47Z

Still LGTM, please squash.

tenzen-y

@PBundyra Thanks for this great and valuable contribution :)
/lgtm
/approve

k8s-ci-robot · 2023-11-29T10:08:29Z

LGTM label has been added.

Git tree hash: 181eb85d3e351b4528b865f032bf231df15ef572

k8s-ci-robot · 2023-11-29T10:08:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: PBundyra, tenzen-y

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~~ [tenzen-y]

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

tenzen-y · 2023-11-29T10:09:19Z

Still LGTM, please squash.

I'm ok without squash. The prow automatically will squash commits.

…alQueue

tenzen-y · 2023-11-29T10:10:45Z

/lgtm

alculquicondor · 2024-02-13T19:39:17Z

/release-note-edit

Visibility API: Add an endpoint that allows a user to fetch information about pending workloads and their position in LocalQueue.

k8s-ci-robot requested review from denkensk and trasc November 24, 2023 14:15

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 24, 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 24, 2023

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 24, 2023

k8s-ci-robot assigned mimowo Nov 24, 2023

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

PBundyra force-pushed the lq branch from ed71fff to 68c3449 Compare November 24, 2023 15:38

mimowo reviewed Nov 24, 2023

View reviewed changes

pkg/visibility/api/install.go Outdated Show resolved Hide resolved

pkg/visibility/api/rest/local_queue.go Outdated Show resolved Hide resolved

pkg/visibility/api/rest/pending_workloads_LQ.go Outdated Show resolved Hide resolved

mimowo reviewed Nov 24, 2023

View reviewed changes

test/e2e/e2e_test.go Show resolved Hide resolved

mimowo reviewed Nov 24, 2023

View reviewed changes

charts/kueue/templates/rbac/pending_workloads_CQ_viewer_role.yaml Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2023

PBundyra force-pushed the lq branch from b8b1fdd to be44ef7 Compare November 27, 2023 13:18

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 27, 2023

PBundyra force-pushed the lq branch 3 times, most recently from 9ee157f to 11db7fa Compare November 27, 2023 13:48

k8s-ci-robot assigned alculquicondor and tenzen-y Nov 27, 2023

mimowo reviewed Nov 27, 2023

View reviewed changes

mimowo reviewed Nov 28, 2023

View reviewed changes

PBundyra force-pushed the lq branch from edb6118 to ec11f85 Compare November 28, 2023 13:05

k8s-ci-robot assigned mwielgus Nov 28, 2023

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

tenzen-y reviewed Nov 29, 2023

View reviewed changes

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

k8s-ci-robot requested review from alculquicondor, mimowo, mwielgus and tenzen-y November 29, 2023 09:52

tenzen-y reviewed Nov 29, 2023

View reviewed changes

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

PBundyra mentioned this pull request Nov 29, 2023

Add integration test for visibility server #1377

Open

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

Add a visibility API endpoint for fetching pending workloads in a Loc…

4f36bd3

…alQueue

PBundyra force-pushed the lq branch from 86d8e7c to 4f36bd3 Compare November 29, 2023 10:10

k8s-ci-robot merged commit 808319d into kubernetes-sigs:main Nov 29, 2023
14 checks passed

k8s-ci-robot added this to the v0.6 milestone Nov 29, 2023

PBundyra mentioned this pull request Feb 28, 2024

REQUEST: New membership for PBundyra kubernetes/org#4795

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a visibility API endpoint for fetching pending workloads in a Local Queue #1365

Add a visibility API endpoint for fetching pending workloads in a Local Queue #1365

PBundyra commented Nov 24, 2023 •

edited by k8s-ci-robot

netlify bot commented Nov 24, 2023 •

edited

k8s-ci-robot commented Nov 24, 2023

PBundyra commented Nov 24, 2023

mimowo commented Nov 24, 2023

mimowo left a comment •

edited

PBundyra commented Nov 24, 2023

PBundyra commented Nov 27, 2023

PBundyra commented Nov 28, 2023

mimowo commented Nov 28, 2023

k8s-ci-robot commented Nov 28, 2023

PBundyra commented Nov 28, 2023

tenzen-y commented Nov 29, 2023

tenzen-y left a comment

tenzen-y Nov 29, 2023 •

edited

PBundyra Nov 29, 2023

mimowo Nov 29, 2023 •

edited

tenzen-y Nov 29, 2023

mimowo commented Nov 29, 2023

tenzen-y left a comment

k8s-ci-robot commented Nov 29, 2023

k8s-ci-robot commented Nov 29, 2023

tenzen-y commented Nov 29, 2023

tenzen-y commented Nov 29, 2023

alculquicondor commented Feb 13, 2024

Add a visibility API endpoint for fetching pending workloads in a Local Queue #1365

Add a visibility API endpoint for fetching pending workloads in a Local Queue #1365

Conversation

PBundyra commented Nov 24, 2023 • edited by k8s-ci-robot

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?

netlify bot commented Nov 24, 2023 • edited

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

k8s-ci-robot commented Nov 24, 2023

PBundyra commented Nov 24, 2023

mimowo commented Nov 24, 2023

mimowo left a comment • edited

Choose a reason for hiding this comment

PBundyra commented Nov 24, 2023

PBundyra commented Nov 27, 2023

PBundyra commented Nov 28, 2023

mimowo commented Nov 28, 2023

k8s-ci-robot commented Nov 28, 2023

PBundyra commented Nov 28, 2023

tenzen-y commented Nov 29, 2023

tenzen-y left a comment

Choose a reason for hiding this comment

tenzen-y Nov 29, 2023 • edited

Choose a reason for hiding this comment

PBundyra Nov 29, 2023

Choose a reason for hiding this comment

mimowo Nov 29, 2023 • edited

Choose a reason for hiding this comment

tenzen-y Nov 29, 2023

Choose a reason for hiding this comment

mimowo commented Nov 29, 2023

tenzen-y left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 29, 2023

k8s-ci-robot commented Nov 29, 2023

tenzen-y commented Nov 29, 2023

tenzen-y commented Nov 29, 2023

alculquicondor commented Feb 13, 2024

PBundyra commented Nov 24, 2023 •

edited by k8s-ci-robot

netlify bot commented Nov 24, 2023 •

edited

mimowo left a comment •

edited

tenzen-y Nov 29, 2023 •

edited

mimowo Nov 29, 2023 •

edited