Skip to content

Conversation

@anahas-redhat
Copy link
Contributor

@anahas-redhat anahas-redhat commented Nov 3, 2025

This commit adds comprehensive e2e tests for the pending workloads visibility API
functionality, including RBAC verification and priority ordering validation.

Key changes:

Add e2e tests for ClusterQueue pending workloads visibility API

Verify admin users can access ClusterQueue pending workloads
Test priority ordering of pending workloads (high, medium, low)
Validate pending workloads list becomes empty after execution
Enhance test utilities

Add helper functions for creating test resources with builder pattern
Add ClusterQueue, LocalQueue, and ResourceFlavor wrapper builders
Add CreateWithObject methods that return created objects and cleanup functions
Add GenerateName support for test resource builders

Jira link:
https://issues.redhat.com/browse/OCPKUEUE-319

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2025
@openshift-ci
Copy link

openshift-ci bot commented Nov 3, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@anahas-redhat anahas-redhat marked this pull request as ready for review November 3, 2025 20:31
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2025
@anahas-redhat
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2025
@anahas-redhat
Copy link
Contributor Author

/retest

@anahas-redhat
Copy link
Contributor Author

@rphillips @cpmeadors can you please take a look at this PR when you have chance? Thank you.

@anahas-redhat
Copy link
Contributor Author

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2025
@anahas-redhat anahas-redhat changed the title OCPKUEUE-318 - Pending Workloads for ClusterQueue OCPKUEUE-319 - Pending Workloads for ClusterQueue Nov 4, 2025
@anahas-redhat
Copy link
Contributor Author

/retest

@kannon92
Copy link
Contributor

kannon92 commented Nov 5, 2025

Kueue Metrics Metrics Infrastructure should have ServiceMonitor created [metrics]
/go/src/github.com/openshift/kueue-operator/test/e2e/e2e_metrics_test.go:177
[FAILED] in [It] - /go/src/github.com/openshift/kueue-operator/test/e2e/e2e_metrics_test.go:186 @ 11/05/25 15:16:48.713
• [FAILED] [0.040 seconds]
Kueue Metrics Metrics Infrastructure [It] should have ServiceMonitor created [metrics]
/go/src/github.com/openshift/kueue-operator/test/e2e/e2e_metrics_test.go:177
[FAILED] Expected
: metrics
to equal
: https
In [It] at: /go/src/github.com/openshift/kueue-operator/test/e2e/e2e_metrics_test.go:186 @ 11/05/25 15:16:48.713


I think you should rebase your PR if you haven't tried that yet.

@anahas-redhat anahas-redhat force-pushed the workloads_clusterqueue branch from 02f442b to dbfc853 Compare November 5, 2025 19:39
@anahas-redhat
Copy link
Contributor Author

/retest

@anahas-redhat
Copy link
Contributor Author

Executions for 4.19 are failing for different reasons, the last one I'll print below. However, all tests are passing.
`error: invalid reference format
make: *** [Makefile:245: run-must] Error 1
{"component":"entrypoint","error":"wrapped process failed: exit status 2","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:84","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"2025-11-06T18:11:11Z"}
error: failed to execute wrapped command: exit status 2
INFO[2025-11-06T18:11:12Z] Step test-e2e-4-19-kueue-must-gather failed after 53s.
INFO[2025-11-06T18:11:12Z] Running step test-e2e-4-19-gather-must-gather.
INFO[2025-11-06T18:13:05Z] Step test-e2e-4-19-gather-must-gather succeeded after 1m53s.
INFO[2025-11-06T18:13:05Z] Step phase post failed after 2m47s.
INFO[2025-11-06T18:13:05Z] Releasing cluster claims for test test-e2e-4-19
INFO[2025-11-06T18:13:05Z] Ran for 1h4m13s
ERRO[2025-11-06T18:13:05Z] Some steps failed:
ERRO[2025-11-06T18:13:05Z]

  • could not run steps: step test-e2e-4-19 failed: "test-e2e-4-19" post steps failed: "test-e2e-4-19" pod "test-e2e-4-19-kueue-must-gather" failed: could not watch pod: the pod ci-op-8q53mys4/test-e2e-4-19-kueue-must-gather failed after 51s (failed containers: test): ContainerFailed one or more containers exited`

@anahas-redhat
Copy link
Contributor Author

/retest

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2025
return createdPriorityClass, cleanup, nil
}

// createServiceAccount creates a ServiceAccount in the specified namespaceand returns the created object
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: namespaceand

@MaysaMacedo
Copy link
Contributor

/lgtm

@kannon92
Copy link
Contributor

kannon92 commented Nov 7, 2025

/cherry-pick release-1.2

@openshift-cherrypick-robot
Copy link
Contributor

@kannon92: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.2

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.

Expect(err).NotTo(HaveOccurred())

cleanupClusterQueue, err = testutils.CreateClusterQueue(kueueClient)
cleanupClusterQueue, err = testutils.CreateClusterQueue(context.TODO(), kueueClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use one context at the top of the function.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2025
… visibility API

This commit adds comprehensive e2e tests for the pending workloads visibility API
functionality, including RBAC verification and priority ordering validation.

Key changes:
- Add e2e tests for ClusterQueue pending workloads visibility API
  * Verify admin users can access ClusterQueue pending workloads
  * Test priority ordering of pending workloads (high, medium, low)
  * Validate pending workloads list becomes empty after execution

- Enhance test utilities
  * Add helper functions for creating test resources with builder pattern
  * Add ClusterQueue, LocalQueue, and ResourceFlavor wrapper builders
  * Add CreateWithObject methods that return created objects and cleanup functions
  * Add GenerateName support for test resource builders
… visibility API

-Add e2e tests for ClusterQueue pending workloads visibility API

Verify admin users can access ClusterQueue pending workloads
Test priority ordering of pending workloads (high, medium, low)
Validate pending workloads list becomes empty after execution
Enhance test utilities

Add helper functions for creating test resources with builder pattern
Add ClusterQueue, LocalQueue, and ResourceFlavor wrapper builders
Add CreateWithObject methods that return created objects and cleanup functions
Add GenerateName support for test resource builders
- Replace hardcoded 'kueue.openshift.io/managed' with testutils.OpenShiftManagedLabel
- Fix context usage in namespace creation (use ctx instead of context.TODO())
- Improves code maintainability and consistency across test suite
…r_test.go

- Removed duplicate import of k8s.io/apimachinery/pkg/api/errors
- Updated reference from 'errors' to 'apierrors' alias for consistency
- Replaced context.TODO() with ctx parameter
- No functional changes to e2e_visibility_on_demand_test.go
Copy link
Contributor

@MaysaMacedo MaysaMacedo left a comment

Choose a reason for hiding this comment

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

/lgtm

DeferCleanup(cleanupLowPriorityClass)

By("Creating RBAC kueue-batch-admin-role for Visibility API")
kueueTestSA, err := createServiceAccount(ctx, namespaceA.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor suggestion:

When a namespace is created 3 service accounts already exists by default there, so you could re-use one of them instead of creating a new one:

$ kubectl get serviceaccount -n test
NAME       SECRETS   AGE
builder    1         2m4s
default    1         2m4s
deployer   1         2m4s

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2025
@rphillips
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Nov 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anahas-redhat, rphillips

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2025
@kannon92
Copy link
Contributor

/cherry-pick release-1.2

@openshift-cherrypick-robot
Copy link
Contributor

@kannon92: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.2

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.

@openshift-ci
Copy link

openshift-ci bot commented Nov 10, 2025

@anahas-redhat: all tests passed!

Full PR test history. Your PR dashboard.

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

@openshift-merge-bot openshift-merge-bot bot merged commit 8d21fe7 into openshift:main Nov 10, 2025
17 checks passed
@openshift-cherrypick-robot
Copy link
Contributor

@kannon92: new pull request created: #960

In response to this:

/cherry-pick release-1.2

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.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants