Skip to content

OPRUN-4459,OPRUN-4460: Remove dependency in OCP catalog content #642

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

Open
camilamacedo86 wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −7
Open

OPRUN-4459,OPRUN-4460: Remove dependency in OCP catalog content #642

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
29 changes: 22 additions & 7 deletions openshift/tests-extension/test/olmv1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,17 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM] OLMv1 CRDs", func() {
})
})

// Keeping this test as skip:disconnected, so we can attempt to install a "real" operator, rather than a generated one
// There is an equivalent in-cluster positive test below
Copy link
Member

@joelanford joelanford Feb 24, 2026

Choose a reason for hiding this comment

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

This seems to indicate that we already have an "in-cluster positive test"?

Does that mean we would simply delete this test? If not, what is the difference between the updated version of this test and the "equivalent in-cluster positive test below"?

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Feb 24, 2026

Choose a reason for hiding this comment

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

Most of our tests need to install a solution. But we don’t have another test that only checks the install step.

Right now, I would not in favor of remove this test because it affects our component readiness signal. We can think about removing it in the next cut branch, but not in this branch (we are too close).

So IMO we have these options:

  • A) Merge this PR. This removes the same pain we have every OCP release, and we keep a basic smoke test that can help when we need to isolate issues.

  • B) Close this PR now and reopen it after the new branch. Then we can propose removing the test 100% and handle any readiness-signal impact over time.

  • C) Do nothing. Keep everything as it is and coordinate this content for every OCP release cut.

Copy link
Member

@joelanford joelanford Feb 24, 2026

Choose a reason for hiding this comment

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

But we don’t have another test that only checks the install step.

To me this isn't that compelling of an argument on its own.

I would not in favor of remove this test because it affects our component readiness signal.

That is a compelling argument. And I think doing it now means one fewer release branch where we are exposed to the possibility that future catalog change can break this test.

I think my vote would be to merge this now, and then revisit after branch cut to see if it makes sense to reduce our test footprint if this test provides no extra coverage.

// Uses the in-cluster built bundle (catalogdata + operatordata). Standalone; does not rely on Red Hat index.
var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation", func() {
Copy link
Member

@joelanford joelanford Feb 24, 2026

Choose a reason for hiding this comment

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

Are we able to drop [Skipped:Disconnected]?

Or does that affect the test name matching that component readiness does?

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Feb 24, 2026

Choose a reason for hiding this comment

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

We could remove it.

However, if we remove it now, we will change the test name/ID, and that will affect the component readiness signal.

So I suggest we review this after the branch cut.

After the cut:

  • If we want to remove the test, we can remove it fully and we won’t need to create a new ID just to remove the skip.
  • If we want to keep the test, then we remove the skip and follow the steps to deprecate this name and have the new ID test/signal

var (
namespace string
k8sClient client.Client
unique string
ccName string
opName string
)

BeforeEach(func() {
BeforeEach(func(ctx SpecContext) {
helpers.RequireOLMv1CapabilityOnOpenshift()
k8sClient = env.Get().K8sClient
namespace = "install-test-ns-" + rand.String(4)
Expand All @@ -96,6 +98,19 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
By(fmt.Sprintf("deleting namespace %s", namespace))
_ = k8sClient.Delete(context.Background(), ns)
})

By("building in-cluster catalog and bundle")
testVersion := env.Get().OpenShiftVersion
replacements := map[string]string{
"{{ TEST-BUNDLE }}": "", // Auto-filled
"{{ NAMESPACE }}": "", // Auto-filled
"{{ VERSION }}": testVersion,
"{{ TEST-CONTROLLER }}": image.ShellImage(),
}
unique, _, ccName, opName = helpers.NewCatalogAndClusterBundles(ctx, replacements,
catalogdata.AssetNames, catalogdata.Asset,
operatordata.AssetNames, operatordata.Asset,
)
})

AfterEach(func(ctx SpecContext) {
Expand All @@ -111,14 +126,14 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
Skip("Requires OCP Catalogs: not OpenShift")
}

By("ensuring no ClusterExtension and CRD for quay-operator")
helpers.EnsureCleanupClusterExtension(context.Background(), "quay-operator", "quayregistries.quay.redhat.com")
By("ensuring no ClusterExtension and CRD for the operator")
helpers.EnsureCleanupClusterExtension(context.Background(), opName, "")

By("applying the ClusterExtension resource")
name, cleanup := helpers.CreateClusterExtension("quay-operator", "3.13.0", namespace, "")
name, cleanup := helpers.CreateClusterExtension(opName, "", namespace, unique, helpers.WithCatalogNameSelector(ccName))
DeferCleanup(cleanup)

By("waiting for the quay-operator ClusterExtension to be installed")
By("waiting for the ClusterExtension to be installed")
helpers.ExpectClusterExtensionToBeInstalled(ctx, name)
})
})
Expand Down