Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 17, 2025
Merged

NO-ISSUE: UPSTREAM: <carry>: fix cases 81696 and 74618 for product code changes #528

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions openshift/tests-extension/test/qe/specs/olmv1_ce.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,9 +787,15 @@ var _ = g.Describe("[sig-olmv1][Jira:OLM] clusterextension", g.Label("NonHyperSh
o.Expect(err).NotTo(o.HaveOccurred())

g.By("create role for ns watch")
defer func() { _ = oc.AsAdmin().WithoutNamespace().Run("delete").Args("-f", roleNsWatchFile).Execute() }()
err = oc.AsAdmin().WithoutNamespace().Run("apply").Args("-f", roleNsWatchFile).Execute()
o.Expect(err).NotTo(o.HaveOccurred())
// Check if the watch namespace role file exists before trying to apply it
// The file may not exist if no permissions are needed for the watch namespace
if _, err := os.Stat(roleNsWatchFile); err == nil {
defer func() { _ = oc.AsAdmin().WithoutNamespace().Run("delete").Args("-f", roleNsWatchFile).Execute() }()
err = oc.AsAdmin().WithoutNamespace().Run("apply").Args("-f", roleNsWatchFile).Execute()
o.Expect(err).NotTo(o.HaveOccurred())
} else {
e2e.Logf("Watch namespace role file %s does not exist, skipping creation", roleNsWatchFile)
}

g.By("create binding")
paremeters = []string{"-n", "default", "--ignore-unknown-parameters=true", "-f", bindingTemplate, "-p",
Expand Down Expand Up @@ -918,7 +924,7 @@ var _ = g.Describe("[sig-olmv1][Jira:OLM] clusterextension", g.Label("NonHyperSh
g.By("check non all ns mode fails to be installed.")
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)
Copy link
Contributor

@camilamacedo86 camilamacedo86 Oct 17, 2025 •
edited
Loading

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@camilamacedo86 camilamacedo86 Oct 17, 2025 •
edited
Loading

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?

Copy link
Contributor Author

@kuiwang02 kuiwang02 Oct 17, 2025 •
edited
Loading

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.

Copy link
Contributor

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?

Copy link
Contributor Author

@kuiwang02 kuiwang02 Oct 17, 2025 •
edited
Loading

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.

ceNAN.CheckClusterExtensionCondition(oc, "Installed", "reason", "Failed", 10, 180, 0)
ceNAN.Delete(oc)
}
Expand Down