From 2b2b0cb69147dc2bc286e359cd5628addb477553 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Tue, 22 Sep 2026 11:08:35 -0400 Subject: [PATCH 1/2] test: verify migration COS supersession Signed-off-by: Todd Short --- .github/workflows/migration-test.yaml | 26 ++++ hack/e2e/migration/build-fixture-catalog.sh | 4 + migration.mk | 6 + migration/pkg/migration/migration.go | 4 +- migration/pkg/migration/phase.go | 2 +- migration/pkg/migration/unit_test.go | 4 +- test/e2e/migration/e2e_test.go | 161 ++++++++++++++++++++ 7 files changed, 202 insertions(+), 5 deletions(-) diff --git a/.github/workflows/migration-test.yaml b/.github/workflows/migration-test.yaml index 34bc254..0622803 100644 --- a/.github/workflows/migration-test.yaml +++ b/.github/workflows/migration-test.yaml @@ -58,6 +58,32 @@ jobs: path: artifacts/e2e if-no-files-found: ignore + cos-supersession: + name: migration COS supersession E2E + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + - name: Set up fixture cluster + run: make migration/e2e-fixture-setup + - name: Verify migrated ClusterObjectSet supersession + run: make migration/test-e2e-cos-supersession + - name: Tear down fixture cluster + if: always() + run: E2E_CLUSTER_NAME=library-olm-fixture-e2e make migration/e2e-teardown + - name: Upload COS supersession diagnostics + if: always() + uses: actions/upload-artifact@v7 + with: + name: migration-cos-supersession-e2e-artifacts + path: artifacts/e2e + if-no-files-found: ignore + in-cluster-job: name: migration in-cluster Job E2E runs-on: ubuntu-latest diff --git a/hack/e2e/migration/build-fixture-catalog.sh b/hack/e2e/migration/build-fixture-catalog.sh index 2abe684..8210016 100755 --- a/hack/e2e/migration/build-fixture-catalog.sh +++ b/hack/e2e/migration/build-fixture-catalog.sh @@ -25,6 +25,10 @@ trap cleanup EXIT # Fixture setup is idempotent. Remove a prior CatalogSource migration result # before replaying the committed CatalogSource with the locally built image. +# The experimental operator-controller installer creates an `operatorhubio` +# ClusterCatalog that resolves packages from Quay. Fixture tests must never +# select mutable external catalog content instead of their committed snapshots. +kubectl delete clustercatalog/operatorhubio --ignore-not-found --wait=true kubectl delete clustercatalog/operatorhubio-catalog --ignore-not-found --wait=true kubectl apply -f "$root_dir/test/e2e/migration/fixtures/registry.yaml" kubectl -n "$registry_namespace" wait --for=condition=Ready certificate/fixture-registry-tls --timeout=3m diff --git a/migration.mk b/migration.mk index 69127b0..0c5fb2f 100644 --- a/migration.mk +++ b/migration.mk @@ -30,6 +30,8 @@ E2E_REAL_OPERATOR_NAMESPACE ?= migration-e2e-real E2E_REAL_OPERATOR_SUBSCRIPTION ?= ecr-secret-operator E2E_OPERATOR ?= all E2E_MIGRATION_IMAGE ?= library-olm-migration-e2e:dev +# Retain migration resources after a focused E2E run for manual inspection. +E2E_KEEP_RESOURCES ?= false ##@ Migration @@ -104,6 +106,10 @@ migration/test-e2e-in-cluster-job: migration/e2e-fixture-setup migration/build-e E2E_OPERATOR=ecr-secret-operator $(MAKE) migration/e2e-install-fixture-v0 KUBECONFIG="$(E2E_FIXTURE_KUBECONFIG)" E2E_SUITE=in-cluster-job E2E_NAMESPACE=migration-e2e-ecr-secret E2E_SUBSCRIPTION=ecr-secret-operator E2E_MIGRATION_IMAGE="$(E2E_MIGRATION_IMAGE)" E2E_ARTIFACTS="$(E2E_ARTIFACTS)/in-cluster-job" go test -count=1 -tags=e2e ./test/e2e/migration -run '^TestMigrationInClusterJob$$' -timeout "$(E2E_TIMEOUT)" +.PHONY: migration/test-e2e-cos-supersession +migration/test-e2e-cos-supersession: migration/e2e-fixture-setup migration/build ## Verify migration COS is superseded by the catalog revision + @set -euo pipefail; package=ecr-secret-operator; KUBECONFIG="$(E2E_FIXTURE_KUBECONFIG)" ./hack/e2e/migration/delete-v1.sh "$$package"; KUBECONFIG="$(E2E_FIXTURE_KUBECONFIG)" ./hack/e2e/migration/install-fixture-v0.sh "$$package"; KUBECONFIG="$(E2E_FIXTURE_KUBECONFIG)" E2E_ARTIFACTS="$(E2E_ARTIFACTS)/cos-supersession" E2E_SUITE=fixture E2E_COS_SUPERSESSION_TEST=true E2E_NAMESPACE=migration-e2e-ecr-secret E2E_SUBSCRIPTION="$$package" go test -count=1 -tags=e2e ./test/e2e/migration -run '^TestPrecreatedClusterObjectSetSupersession$$' -timeout "$(E2E_TIMEOUT)"; if [[ "$(E2E_KEEP_RESOURCES)" != true ]]; then KUBECONFIG="$(E2E_FIXTURE_KUBECONFIG)" ./hack/e2e/migration/delete-v1.sh "$$package"; else echo "Retaining migration resources for inspection"; fi + .PHONY: migration/e2e-delete-v1 migration/e2e-delete-v1: ## Delete one migration E2E operator as OLMv1, or all @if [[ "$(E2E_OPERATOR)" != all ]]; then E2E_OPERATOR="$(E2E_OPERATOR)" KUBECONFIG="$(E2E_KUBECONFIG)" bash -c 'source "$$1"; operator_fields "$$E2E_OPERATOR"' -- ./hack/e2e/migration/operators.sh; fi diff --git a/migration/pkg/migration/migration.go b/migration/pkg/migration/migration.go index c2fe00a..21bf7f8 100644 --- a/migration/pkg/migration/migration.go +++ b/migration/pkg/migration/migration.go @@ -458,7 +458,7 @@ func (m *Migrator) createClusterObjectSet(ctx context.Context, opts Options, inf stripped := stripResource(obj) cosObjects = append(cosObjects, *ocv1ac.ClusterObjectSetObject(). WithObject(stripped). - WithCollisionProtection(ocv1.CollisionProtectionIfNoController)) + WithCollisionProtection(ocv1.CollisionProtectionNone)) } phases := PhaseSort(cosObjects) @@ -527,7 +527,7 @@ func (m *Migrator) createClusterObjectSet(ctx context.Context, opts Options, inf cosSpec := ocv1ac.ClusterObjectSetSpec(). WithRevision(1). - WithCollisionProtection(ocv1.CollisionProtectionIfNoController). + WithCollisionProtection(ocv1.CollisionProtectionNone). WithLifecycleState(ocv1.ClusterObjectSetLifecycleStateActive). WithPhases(phases...) diff --git a/migration/pkg/migration/phase.go b/migration/pkg/migration/phase.go index 291e2c2..76bd869 100644 --- a/migration/pkg/migration/phase.go +++ b/migration/pkg/migration/phase.go @@ -184,7 +184,7 @@ func PhaseSort(unsortedObjs []ocv1ac.ClusterObjectSetObjectApplyConfiguration) [ objPtrs[i] = &objs[i] } - cp := ocv1.CollisionProtectionIfNoController + cp := ocv1.CollisionProtectionNone phasesSorted = append(phasesSorted, ocv1ac.ClusterObjectSetPhase(). WithName(string(phaseName)). WithCollisionProtection(cp). diff --git a/migration/pkg/migration/unit_test.go b/migration/pkg/migration/unit_test.go index a5944d1..0a89cd8 100644 --- a/migration/pkg/migration/unit_test.go +++ b/migration/pkg/migration/unit_test.go @@ -294,8 +294,8 @@ func TestPhaseSortAndResourceKeys(t *testing.T) { if len(phases) != 3 || *phases[0].Name != string(PhaseConfiguration) || *phases[1].Name != string(PhaseCRDs) || *phases[2].Name != string(PhaseDeploy) { t.Fatalf("unexpected phases %#v", phases) } - if *phases[2].CollisionProtection != ocv1.CollisionProtectionIfNoController || phases[2].Objects[0].Object.GetNamespace() != "a" { - t.Fatalf("deployment phase not sorted/adoptable: %#v", phases[2]) + if *phases[2].CollisionProtection != ocv1.CollisionProtectionNone || phases[2].Objects[0].Object.GetNamespace() != "a" { + t.Fatalf("deployment phase not sorted with migration collision protection: %#v", phases[2]) } a := unstructured.Unstructured{} a.SetGroupVersionKind(schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}) diff --git a/test/e2e/migration/e2e_test.go b/test/e2e/migration/e2e_test.go index 975722e..0c8c9b7 100644 --- a/test/e2e/migration/e2e_test.go +++ b/test/e2e/migration/e2e_test.go @@ -4,6 +4,7 @@ package e2e import ( "bytes" + "context" "encoding/json" "fmt" "os" @@ -12,8 +13,23 @@ import ( "strings" "testing" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/discovery" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "sigs.k8s.io/controller-runtime/pkg/client" + + operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + ocv1 "github.com/operator-framework/operator-controller/api/v1" + + "github.com/operator-framework/library-olm/migration/pkg/migration" ) // TestEnvironment is deliberately small: it makes both E2E make targets verify @@ -249,6 +265,151 @@ func TestFixtureNegativeGuards(t *testing.T) { run(t, "kubectl", "get", "subscription/"+subscription, "-n", namespace) } +// TestPrecreatedClusterObjectSetSupersession verifies the released controller's +// migration handoff: the migration-created COS uses collision protection None, +// then the catalog creates a controller-owned revision with Prevent protection. +// The catalog revision is newer and therefore supersedes the migration revision. +// It is intentionally a dedicated invocation because it mutates its fixture +// installation without exercising the CLI's complete cleanup path. +func TestPrecreatedClusterObjectSetSupersession(t *testing.T) { + if os.Getenv("E2E_COS_SUPERSESSION_TEST") != "true" { + t.Skip("set E2E_COS_SUPERSESSION_TEST=true to run the ClusterObjectSet supersession proof") + } + namespace, subscription := os.Getenv("E2E_NAMESPACE"), os.Getenv("E2E_SUBSCRIPTION") + if namespace == "" || subscription == "" { + t.Fatal("E2E_NAMESPACE and E2E_SUBSCRIPTION are required") + } + t.Cleanup(func() { collectArtifacts(t, namespace) }) + + ctx := context.Background() + m, kubeClient, restConfig := newMigrator(t) + opts := migration.Options{SubscriptionName: subscription, SubscriptionNamespace: namespace} + opts.ApplyDefaults() + + // C7 must be satisfied before CE construction. The fixture target installs + // a CatalogSource and the catalog CLI creates its ClusterCatalog. + run(t, binary(t, "migrate-catalogs-v0-to-v1"), "--kubeconfig", os.Getenv("KUBECONFIG")) + _, csv, installPlan, err := m.GetCSVAndInstallPlan(ctx, opts) + if err != nil { + t.Fatalf("profile OLMv0 installation: %v", err) + } + info, err := m.GetBundleInfo(ctx, opts, csv, installPlan) + if err != nil { + t.Fatalf("get bundle information: %v", err) + } + catalogName, err := m.ResolveClusterCatalog(ctx, info, restConfig) + if err != nil { + t.Fatalf("resolve ClusterCatalog: %v", err) + } + if catalogName != "operatorhubio-catalog" { + t.Fatalf("resolved ClusterCatalog = %q, want fixture catalog %q", catalogName, "operatorhubio-catalog") + } + info.ResolvedCatalogName = catalogName + info.CollectedObjects, err = m.CollectResources(ctx, opts, csv, installPlan, info.PackageName) + if err != nil { + t.Fatalf("collect migration resources: %v", err) + } + + if err := m.PrepareForMigration(ctx, opts, csv); err != nil { + t.Fatalf("remove OLMv0 management before creating COS: %v", err) + } + if err := m.CreateClusterObjectSet(ctx, opts, info); err != nil { + t.Fatalf("create pre-existing COS: %v", err) + } + + var before ocv1.ClusterObjectSetList + if err := kubeClient.List(ctx, &before, client.MatchingLabels{ + migration.LabelOwnerKind: ocv1.ClusterExtensionKind, + migration.LabelOwnerName: subscription, + }); err != nil { + t.Fatalf("list pre-existing COS: %v", err) + } + if len(before.Items) != 1 { + t.Fatalf("pre-existing COS count = %d, want 1", len(before.Items)) + } + precreatedUID := before.Items[0].UID + if precreatedUID == "" { + t.Fatal("pre-existing COS has no UID") + } + if !cosSucceeded(before.Items[0]) { + t.Fatal("pre-existing COS did not reach Succeeded=True before CE creation") + } + if before.Items[0].Spec.CollisionProtection != ocv1.CollisionProtectionNone { + t.Fatalf("migration COS collisionProtection = %q, want %q", before.Items[0].Spec.CollisionProtection, ocv1.CollisionProtectionNone) + } + + if err := m.CreateClusterExtension(ctx, opts, info); err != nil { + t.Fatalf("create CE for pre-existing COS: %v", err) + } + run(t, "kubectl", "wait", "--for=jsonpath={.status.conditions[?(@.type=='Installed')].status}=True", "clusterextension/"+subscription, "--timeout=10m") + var after ocv1.ClusterObjectSetList + if err := kubeClient.List(ctx, &after, client.MatchingLabels{ + migration.LabelOwnerKind: ocv1.ClusterExtensionKind, + migration.LabelOwnerName: subscription, + }); err != nil { + t.Fatalf("list COS after CE creation: %v", err) + } + if len(after.Items) != 2 { + t.Fatalf("COS count after CE creation = %d, want 2 (migration and catalog revisions)", len(after.Items)) + } + var migrationCOS, catalogCOS *ocv1.ClusterObjectSet + for i := range after.Items { + cos := &after.Items[i] + switch cos.Spec.Revision { + case 1: + migrationCOS = cos + case 2: + catalogCOS = cos + } + } + if migrationCOS == nil || catalogCOS == nil { + t.Fatalf("COS revisions = %#v, want migration revision 1 and catalog revision 2", after.Items) + } + if migrationCOS.UID != precreatedUID { + t.Fatalf("migration COS UID = %s, want pre-created UID %s", migrationCOS.UID, precreatedUID) + } + if migrationCOS.Spec.CollisionProtection != ocv1.CollisionProtectionNone { + t.Fatalf("migration COS collisionProtection = %q, want %q", migrationCOS.Spec.CollisionProtection, ocv1.CollisionProtectionNone) + } + if catalogCOS.Spec.CollisionProtection != ocv1.CollisionProtectionPrevent { + t.Fatalf("catalog COS collisionProtection = %q, want %q", catalogCOS.Spec.CollisionProtection, ocv1.CollisionProtectionPrevent) + } + if len(catalogCOS.OwnerReferences) != 1 || catalogCOS.OwnerReferences[0].Name != subscription { + t.Fatalf("catalog COS ownerReferences = %#v, want ClusterExtension %q", catalogCOS.OwnerReferences, subscription) + } + run(t, "kubectl", "wait", "--for=jsonpath={.status.conditions[?(@.type=='Succeeded')].status}=True", "clusterobjectset/"+catalogCOS.Name, "--timeout=10m") +} + +func cosSucceeded(cos ocv1.ClusterObjectSet) bool { + for _, condition := range cos.Status.Conditions { + if condition.Type == ocv1.ClusterObjectSetTypeSucceeded && condition.Status == metav1.ConditionTrue { + return true + } + } + return false +} + +func newMigrator(t *testing.T) (*migration.Migrator, client.Client, *rest.Config) { + t.Helper() + config, err := clientcmd.BuildConfigFromFlags("", os.Getenv("KUBECONFIG")) + if err != nil { + t.Fatalf("load kubeconfig: %v", err) + } + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(ocv1.AddToScheme(scheme)) + utilruntime.Must(appsv1.AddToScheme(scheme)) + utilruntime.Must(corev1.AddToScheme(scheme)) + utilruntime.Must(apiextensionsv1.AddToScheme(scheme)) + utilruntime.Must(operatorsv1.AddToScheme(scheme)) + utilruntime.Must(operatorsv1alpha1.AddToScheme(scheme)) + kubeClient, err := client.New(config, client.Options{Scheme: scheme}) + if err != nil { + t.Fatalf("create Kubernetes client: %v", err) + } + return migration.NewMigrator(kubeClient, config), kubeClient, config +} + // TestMigration applies the suite's complete fixture, exercises the two migration // binaries, and observes the API server rather than mocking either OLM controller. // E2E_MANIFEST must create the namespace, a CatalogSource, and the named Subscription. From a0bbb2908ccc01e2ca6a3a65dbb616f6c53cdddd Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 23 Sep 2026 10:59:21 -0400 Subject: [PATCH 2/2] fix: report migration COS collision protection Signed-off-by: Todd Short --- migration/examples/cmd/migrate-operators-v0-to-v1/convert.go | 2 +- migration/pkg/migration/migration.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migration/examples/cmd/migrate-operators-v0-to-v1/convert.go b/migration/examples/cmd/migrate-operators-v0-to-v1/convert.go index d981329..a48b128 100644 --- a/migration/examples/cmd/migrate-operators-v0-to-v1/convert.go +++ b/migration/examples/cmd/migrate-operators-v0-to-v1/convert.go @@ -334,7 +334,7 @@ func runConvertDryRun(cmd *cobra.Command, m *migration.Migrator, opts migration. detail("Version:", "(unset — automatic channel-based upgrades)") } detail("Channel:", valueOrDefault(info.Channel, "(none set)")) - detail("CollisionProtection:", "IfNoController") + detail("CollisionProtection:", "None") fmt.Printf("\n OLMv0 resources that would be deleted or changed:\n") for _, line := range dryRunCleanupPlan(opts, info) { diff --git a/migration/pkg/migration/migration.go b/migration/pkg/migration/migration.go index 21bf7f8..1e9b8d9 100644 --- a/migration/pkg/migration/migration.go +++ b/migration/pkg/migration/migration.go @@ -400,7 +400,7 @@ func (m *Migrator) ensureClusterExtensionAbsent(ctx context.Context, name string } // CreateClusterObjectSet builds and creates a COS from the collected resources. -// It uses CollisionProtection=IfNoController so OLMv1 can adopt existing resources (including CRDs). +// It uses CollisionProtection=None; the controller creates the subsequent catalog-derived revision. // The COS is annotated with the source Subscription reference. // // TODO(R2.7): when boxcutter phase 2 introduces ClusterObjectDeployment as a replacement or