OPRUN-4571: add OLMv1 topology-based deployment scaling e2e test#737
OPRUN-4571: add OLMv1 topology-based deployment scaling e2e test#737tmshort wants to merge 1 commit into
Conversation
|
@tmshort: This pull request references OPRUN-4571 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughImports Kubernetes policy/v1 into the test environment scheme, adds an OLMv1 topology-aware Ginkgo test that validates operand Deployment replicas and PodDisruptionBudgets by controlPlaneTopology, registers the new test in payload JSON, and updates generated bindata embedded modTime timestamps. ChangesOLMv1 Topology-based Scaling Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openshift/tests-extension/test/olmv1-topology.go`:
- Around line 79-87: The current check lists all PodDisruptionBudgets (pdbList
:= &policyv1.PodDisruptionBudgetList{}; k8sClient.List(...,
client.InNamespace(c.namespace))) and asserts emptiness based on isHA, which is
too broad; instead scope PDB assertions to only those PDBs that target the
current deployment’s pods by using the deployment’s label set (e.g., the labels
used by the Deployment referenced by this test) as a selector when listing or by
filtering pdbList.Items by comparing each pdb.Spec.Selector
(MatchLabels/MatchExpressions) against the deployment labels; update the
k8sClient.List call or add a filter step so the Expect checks (the isHA branch)
only consider PDBs that select the deployment’s pods (referencing pdbList,
policyv1.PodDisruptionBudgetList, k8sClient.List, c.namespace, isHA, and the
test’s deployment labels).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 395a5019-d7ab-42d6-b359-fc1f67a85f1f
📒 Files selected for processing (2)
openshift/tests-extension/pkg/env/cluster.goopenshift/tests-extension/test/olmv1-topology.go
|
It seems fine /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm cancel I think we need to run the tests manually first to ensure that has no flakes With /payload-aggregate |
Adds a new test that verifies cluster-olm-operator correctly configures operator-controller and catalogd deployments based on the cluster's control plane topology: - HA topologies (HighlyAvailable, HighlyAvailableArbiter, DualReplica): replicas=2 with a PodDisruptionBudget present - Non-HA topologies (SingleReplica/SNO, External): replicas=1, no PDB Also registers policyv1 in the test scheme to support PDB list queries. Assisted-by: claude Signed-off-by: Todd Short <tshort@redhat.com>
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.23-e2e-gcp-ovn-techpreview 5 |
|
@tmshort: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command |
|
/payload-aggregate periodic-ci-openshift-release-main-ci-4.23-e2e-gcp-ovn-techpreview 5 |
|
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6a85cb70-59d7-11f1-9a81-4ef5e34f3614-0 |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
@tmshort: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/verified by @Leo6Leo Manual cluster verification (HA cluster, OCP 5.0 nightly)
Built and ran the new e2e test
Per @tmshort 's guidance, this verify is conditional on the payload-aggregate: if payload-aggregate job succeeds link to payload, then it should be good. |
openshift-ci-robot
added
the
verified
|
@Leo6Leo: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/payload-aggregate periodic-ci-openshift-release-main-ci-4.23-e2e-gcp-ovn-techpreview 5 On the first attempt, it appears that the aggregator failed (the jobs rand and finished, but results were not aggregated). I'll take a look at the individual jobs to verify it. But also, will run it again (just in case). |
|
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/810c4620-5a1e-11f1-8d7c-22ccbcc910ea-0 |
5 participants
Adds a new test that verifies cluster-olm-operator correctly configures operator-controller and catalogd deployments based on the cluster's control plane topology:
Also registers policyv1 in the test scheme to support PDB list queries.
Summary by CodeRabbit
Tests
Chores