Skip to content

OCPBUGS-63347: Fix flake for single/own ns tests by ensuring uniquess and waiting for k8s cleanups #536

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 24, 2025
+49 −26
Merged

OCPBUGS-63347: Fix flake for single/own ns tests by ensuring uniquess and waiting for k8s cleanups #536

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
75 changes: 49 additions & 26 deletions openshift/tests-extension/test/olmv1-singleownnamespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
Expand Down Expand Up @@ -277,9 +278,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
var (
k8sClient client.Client
activeNamespaces map[string]struct{}
catalogName string
packageName string
crdSuffix string
singleownImage string
)

BeforeEach(func(ctx SpecContext) {
Expand All @@ -291,29 +290,8 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
helpers.RequireImageRegistry(ctx)
k8sClient = env.Get().K8sClient
activeNamespaces = map[string]struct{}{}

// Generate unique CRD suffix for parallel execution
crdSuffix = rand.String(4)

// Build in-cluster bundle and catalog
singleownImage := image.LocationFor("quay.io/olmtest/webhook-operator:v0.0.5")
packageName = fmt.Sprintf("singleown-operator-both-%s", crdSuffix)
By(fmt.Sprintf("using singleown operator image: %s, CRD suffix: %s, package: %s", singleownImage, crdSuffix, packageName))

replacements := map[string]string{
"{{ TEST-BUNDLE }}": "",
"{{ NAMESPACE }}": "",
"{{ TEST-CONTROLLER }}": singleownImage,
"{{ CRD-SUFFIX }}": crdSuffix,
"{{ PACKAGE-NAME }}": packageName,
}

var nsName, opName string
_, nsName, catalogName, opName = helpers.NewCatalogAndClusterBundles(ctx, replacements,
singleownindex.AssetNames, singleownindex.Asset,
singleownbundle.AssetNames, singleownbundle.Asset,
)
By(fmt.Sprintf("singleown bundle %q and catalog %q built successfully in namespace %q", opName, catalogName, nsName))
singleownImage = image.LocationFor("quay.io/olmtest/webhook-operator:v0.0.5")
By(fmt.Sprintf("using singleown operator image: %s", singleownImage))
})

AfterEach(func(ctx SpecContext) {
Expand Down Expand Up @@ -359,6 +337,29 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
activeNamespaces[watchNamespace] = struct{}{}
}

// Ensure unique names per scenario
crdSuffix := rand.String(4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I realize this is the original name of the variable, but it's used for more than just the CRD. So perhaps unique (used in other place), might be better.

packageName := fmt.Sprintf("singleown-operator-both-%s", crdSuffix)
By(fmt.Sprintf("building singleown operator assets for %s scenario: image=%s, CRD suffix=%s, package=%s", sc.label, singleownImage, crdSuffix, packageName))

replacements := map[string]string{
"{{ TEST-BUNDLE }}": "",
"{{ NAMESPACE }}": "",
"{{ TEST-CONTROLLER }}": singleownImage,
"{{ CRD-SUFFIX }}": crdSuffix, // Unique CRD suffix per scenario
"{{ PACKAGE-NAME }}": packageName,
"webhook-operator-webhooktest-admin-role": fmt.Sprintf("webhook-operator-webhooktest-admin-role-%s", crdSuffix),
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI

					"webhook-operator-webhooktest-admin-role":  fmt.Sprintf("webhook-operator-webhooktest-admin-role-%s", crdSuffix),
					"webhook-operator-webhooktest-editor-role": fmt.Sprintf("webhook-operator-webhooktest-editor-role-%s", crdSuffix),
					"webhook-operator-webhooktest-viewer-role": fmt.Sprintf("webhook-operator-webhooktest-viewer-role-%s", crdSuffix),
					"webhook-operator-metrics-reader":          fmt.Sprintf("webhook-operator-metrics-reader-%s", crdSuffix),

although I do not think it is necessary (actually it is not used), it will not impact the case execution.

"webhook-operator-webhooktest-editor-role": fmt.Sprintf("webhook-operator-webhooktest-editor-role-%s", crdSuffix),
"webhook-operator-webhooktest-viewer-role": fmt.Sprintf("webhook-operator-webhooktest-viewer-role-%s", crdSuffix),
"webhook-operator-metrics-reader": fmt.Sprintf("webhook-operator-metrics-reader-%s", crdSuffix),
}

_, nsName, catalogName, opName := helpers.NewCatalogAndClusterBundles(ctx, replacements,
singleownindex.AssetNames, singleownindex.Asset,
singleownbundle.AssetNames, singleownbundle.Asset,
)
By(fmt.Sprintf("singleown bundle %q and catalog %q built successfully in namespace %q for %s scenario", opName, catalogName, nsName, sc.label))

By(fmt.Sprintf("ensuring no ClusterExtension for %s before %s scenario", packageName, sc.label))
crdName := fmt.Sprintf("webhooktests-%s.webhook.operators.coreos.io", crdSuffix)
helpers.EnsureCleanupClusterExtension(context.Background(), packageName, crdName)
Expand Down Expand Up @@ -432,6 +433,13 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
DeferCleanup(func() {
By(fmt.Sprintf("cleanup: deleting ClusterExtension %s", ce.Name))
_ = k8sClient.Delete(context.Background(), ce, client.PropagationPolicy(metav1.DeletePropagationForeground))
if crdName != "" {
crd := &apiextensionsv1.CustomResourceDefinition{}
if err := k8sClient.Get(context.Background(), client.ObjectKey{Name: crdName}, crd); err == nil {
By(fmt.Sprintf("cleanup: deleting CRD %s", crdName))
_ = k8sClient.Delete(context.Background(), crd)
}
}
Comment on lines +436 to +442
Copy link
Contributor

Choose a reason for hiding this comment

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

We're waiting on the namepsace to be deleted, do we need to wait on the CRD to be deleted too? I see that the tests are passing, so this might just be something we need to keep an eye on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We call helpers.EnsureCleanupClusterExtension(context.Background(), ceName, namespace) at the cleanup. But since it is only be called after all we can use it here as well

})

By(fmt.Sprintf("waiting for the ClusterExtension %s to be installed for %s scenario", ceName, sc.label))
Expand Down Expand Up @@ -459,6 +467,8 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
g.Expect(found).To(BeTrue(), "failed to find deployment with olm.targetNamespaces annotation")
}).WithTimeout(helpers.DefaultTimeout).WithPolling(helpers.DefaultPolling).Should(Succeed())

// Ginkgo never invokes those deferred cleanups until we exit the whole spec, so the first scenario’s
// cluster resources survive long enough to collide with the second scenario.
By(fmt.Sprintf("cleaning up resources created for %s scenario to allow next scenario", sc.label))
deletePolicy := metav1.DeletePropagationForeground
Expect(k8sClient.Delete(ctx, ce, client.PropagationPolicy(deletePolicy))).To(Succeed(), "failed to delete ClusterExtension %q", ceName)
Expand All @@ -471,6 +481,19 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMOwnSingleNamespace] OLMv1 ope
Expect(k8sClient.Delete(ctx, watchNSObj, client.PropagationPolicy(deletePolicy))).To(Succeed(), "failed to delete watch namespace %q", watchNamespace)
}
Expect(k8sClient.Delete(ctx, installNS, client.PropagationPolicy(deletePolicy))).To(Succeed(), "failed to delete install namespace %q", installNamespace)

By(fmt.Sprintf("waiting for namespace %s to be fully deleted before next scenario", installNamespace))
Eventually(func() bool {
err := k8sClient.Get(ctx, client.ObjectKey{Name: installNamespace}, &corev1.Namespace{})
return errors.IsNotFound(err)
}).WithTimeout(helpers.DefaultTimeout).WithPolling(helpers.DefaultPolling).Should(BeTrue(), "expected namespace %s to be deleted", installNamespace)
if watchNSObj != nil {
By(fmt.Sprintf("waiting for namespace %s to be fully deleted before next scenario", watchNamespace))
Eventually(func() bool {
err := k8sClient.Get(ctx, client.ObjectKey{Name: watchNamespace}, &corev1.Namespace{})
return errors.IsNotFound(err)
}).WithTimeout(helpers.DefaultTimeout).WithPolling(helpers.DefaultPolling).Should(BeTrue(), "expected namespace %s to be deleted", watchNamespace)
}
}
})
})
Expand Down