OCPBUGS-100143: e2e: skip CRD Compatibility Checker tests on External topology clusters - #638
OCPBUGS-100143: e2e: skip CRD Compatibility Checker tests on External topology clusters#638simkam wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@simkam: This pull request references Jira Issue OCPBUGS-100143, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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
Bot
added
the
needs-rebase
WalkthroughCRD compatibility tests now declare and enforce exclusion from external-topology clusters. The test extension translates platform and topology labels into environment selectors, and the README documents these labels and the E2E fallback. ChangesCRD topology selection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@simkam: This pull request references Jira Issue OCPBUGS-100143, which is valid. 3 validation(s) were run on this bug
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. |
2b7a0ea to
32b0987
Compare
openshift-ci
Bot
removed
the
needs-rebase
| specs.Select(et.NameContains("[OCPFeatureGate:CRDCompatibilityRequirementOperator]")). | ||
| Exclude(et.TopologyEquals("External")). | ||
| AddLabel("[Skipped:ExternalTopology]") | ||
|
|
There was a problem hiding this comment.
@pmeida since you added the platforms label above, can you please check this? Feel free to suggest an alternative approach for handling "Skipped:ExternalTopology". Thanks!
There was a problem hiding this comment.
hmm maybe generic exclude based on
- tag in test name
specs.Select(et.NameContains("[Skipped:ExternalTopology]")).
Exclude(et.TopologyEquals("External"))
- or the same based on ginkgo label
There was a problem hiding this comment.
I would go with option 2 and extend the existing specs.Walk to handle it. Something like:
specs.Walk(func(spec *et.ExtensionTestSpec) {
for label := range spec.Labels {
if platform, ok := strings.CutPrefix(label, "platform:"); ok {
spec.Include(et.PlatformEquals(platform))
}
if topology, ok := strings.CutPrefix(label, "skip-topology:"); ok {
spec.Exclude(et.TopologyEquals(topology))
spec.AddLabel(fmt.Sprintf("[Skipped:%sTopology]", topology)) //optional
}
}
})and then add the ginko label in the Describe (e.g Label("skip-topology:External")).
This also allows us to skip other topologies if we need it (e.g. Label("skip-topology:SingleReplica"))
what do you think?
There was a problem hiding this comment.
I'm not sure if we need to add a label since the base label is already descriptive. I wouldnt.
There was a problem hiding this comment.
Another thing I would ask is if you could document the labels folks can add to tests and what they do in openshift-tests-extension/README.md
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
… topology clusters https://issues.redhat.com/browse/OCPBUGS-100143 The compatibility-requirements-controllers Deployment is not deployed on HyperShift (External topology) clusters because the control plane runs externally, causing all CRD Compatibility Checker tests to fail. Two skip mechanisms cover both execution paths: - OTE: a generic `Label("skip-topology:<mode>")` convention is introduced. `specs.Walk()` translates it into an exclude selector so the harness drops matching tests before selecting them for execution. The test `Describe` block carries `Label("skip-topology:External")`. - make e2e (standard Ginkgo): a runtime `BeforeAll Skip()` guards the suite since OTE selectors are not evaluated by the Ginkgo runner.
32b0987 to
4edd134
Compare
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/pipeline required |
|
Scheduling tests matching the |
|
/lgtm thanks! |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
@simkam: The following tests failed, say
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. |
3 participants
https://issues.redhat.com/browse/OCPBUGS-100143
The compatibility-requirements-controllers Deployment is not deployed on HyperShift (External topology) clusters because the control plane runs externally, causing all CRD Compatibility Checker tests to fail.
Two skip mechanisms cover both execution paths:
Label("skip-topology:<mode>")convention is introduced.specs.Walk()translates it into an exclude selector so the harness drops matching tests before selecting them for execution. The testDescribeblock carriesLabel("skip-topology:External").BeforeAll Skip()guards the suite since OTE selectors are not evaluated by the Ginkgo runner.Summary by CodeRabbit
platform:<name>andskip-topology:<mode>) to control which tests run.