-
Notifications
You must be signed in to change notification settings - Fork 33
NO-ISSUE: UPSTREAM: <carry>: fix cases 81696 and 74618 for product code changes #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@kuiwang02: This pull request explicitly references no jira issue. In 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. |
defer ceNAN.Delete(oc) | ||
_ = ceNAN.CreateWithoutCheck(oc) | ||
ceNAN.CheckClusterExtensionCondition(oc, "Progressing", "message", "do not support targeting all namespaces", 10, 180, 0) | ||
ceNAN.CheckClusterExtensionCondition(oc, "Progressing", "message", "bundle does not support AllNamespaces install mode", 10, 180, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is no longer a valid scenario since now we support single/own namespace as well.
And we are promoting in this sprint those feature flags to GA
Should we not review those checks accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but now it is valid case. after it is GA, I will change it again.
if we do not enhance it, the case is always failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tech-preview clusters, that is not valid.
Will it not fail in Component Readiness or block us from moving forward with GA right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not impact TP cluster because it do not run into this logic if it is TP cluster.
Firstly it will not impact the GA because the case pass now. (actually if we do not fix it, it is failing)
secondly, even it is failing, it will not impact GA because it is informing and not in component readiness now.
so, do not worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the changes made on those tested in the pre-merge/ci?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About pre-merge:
if the qe case is labeled with ReleaseGate, it will be in pre-merged job.
if the qe case is not labeled with ReleaseGate, it will not be in pre-merged job. we ever talked with @chengzhang1016 @jianzhangbjz ,the conclusion is that we do not use it currently, and will consider it from qe's view.
the two cases are not labeled with ReleaseGate. but as the PR description, and we required, we run them and post the case execution log in PR.
About CI:
The QE case which is not labeled with ReleaseGate only run in the qe custom prow job. and we do not put them into readiness (it ever is shown in readiness, but it is fixed now). about Not Shown in readiness, it is conclusion by QE.
by the way, as I said before, the QE migrated cases are informing if it is not labeled with ReleaseGate. so, it does not impact readiness and block your GA. When we design QE migrated, we ever consider this point, so make such design.
/retest-required |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jianzhangbjz, kuiwang02 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 |
/verified by @kuiwang02 |
@kuiwang02: This PR has been marked as verified by In 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. |
@kuiwang02: The following test failed, say
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. |
bf97f12
into
openshift:main
fix OLM v1 test cases 81696 and 74618 for product code changes
Why / Problem Statement
Two OLM v1 test cases (81696 and 74618) were failing due to misalignment between test expectations and updated product code behavior:
Test Case 81696 - Preflight permission check on single namespace mode was failing with:
The test assumed that RBAC role files would always be generated for both install and watch namespaces, but the
GenerateRBACFromMissingRules
function correctly only generates role files when there are actual permissions needed for that namespace.Test Case 74618 - Bundle validation test was failing with error message mismatch:
"do not support targeting all namespaces"
"bundle does not support AllNamespaces install mode"
The operator-controller implementation had updated its error message format to be more clear and accurate, but the test was still checking for the old message.
What / Solution
Updated two test cases in the OLM v1 ClusterExtension test suite to align with current product code behavior.
Changes Made
File:
test/qe/specs/olmv1_ce.go
Lines 789-798 (Test Case 81696 - PolarionID:81696):
GenerateRBACFromMissingRules
doesn't create a role file for the watch namespace (because no permissions are needed), the test now gracefully handles this instead of failingLine 927 (Test Case 74618 - PolarionID:74618):
"do not support targeting all namespaces"
"bundle does not support AllNamespaces install mode"
Testing
Assisted-by: Claude Code