diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index a52fa21c7e..412b8e4e52 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -478,6 +478,17 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return true, "", nil } + // The migration tool creates a successful revision 1 before it creates the + // ClusterExtension. Do not render that same bundle into a second revision: + // claim the pre-existing revision and use it as the initial installed state. + // Comparing the resolved bundle metadata keeps normal upgrades intact. + if revision := migratedSubscriptionRevision(existingRevisions, revisionAnnotations); revision != nil { + if err := bc.adoptRevision(ctx, ext, revision); err != nil { + return false, "", fmt.Errorf("adopting migrated revision %s: %w", revision.Name, err) + } + return true, "", nil + } + // Generate desired revision desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations) if err != nil { @@ -574,6 +585,50 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return true, "", nil } +// migratedSubscriptionRevision finds a successful migration-created initial +// revision whose resolved bundle metadata matches the desired revision. +func migratedSubscriptionRevision(revisions []ocv1.ClusterObjectSet, desiredAnnotations map[string]string) *ocv1.ClusterObjectSet { + for i := range revisions { + revision := &revisions[i] + if revision.Spec.Revision != 1 || + revision.Annotations[labels.MigratedFromSubscriptionKey] == "" || + !meta.IsStatusConditionTrue(revision.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) { + continue + } + + matchesBundle := true + for _, key := range []string{labels.PackageNameKey, labels.BundleNameKey, labels.BundleVersionKey, labels.BundleReferenceKey, labels.BundleReleaseKey} { + if revision.Annotations[key] != desiredAnnotations[key] { + matchesBundle = false + break + } + } + if matchesBundle { + return revision + } + } + return nil +} + +// adoptRevision gives the ClusterExtension controller ownership of a matching +// migration-created revision without rendering a duplicate revision. +func (bc *Boxcutter) adoptRevision(ctx context.Context, ext *ocv1.ClusterExtension, revision *ocv1.ClusterObjectSet) error { + for _, ref := range revision.OwnerReferences { + if ref.Controller != nil && *ref.Controller && ref.UID != ext.UID { + return fmt.Errorf("revision is already controlled by %s %q", ref.Kind, ref.Name) + } + } + + for _, ref := range revision.OwnerReferences { + if ref.Controller != nil && *ref.Controller && ref.UID == ext.UID { + return nil + } + } + + revision.OwnerReferences = append(revision.OwnerReferences, *metav1.NewControllerRef(ext, ocv1.SchemeGroupVersion.WithKind(ocv1.ClusterExtensionKind))) + return bc.Client.Update(ctx, revision) +} + // createExternalizedRevision creates a new COS with all objects externalized to Secrets. // It follows a crash-safe three-step sequence: create Secrets, create COS, patch ownerRefs. func (bc *Boxcutter) createExternalizedRevision(ctx context.Context, ext *ocv1.ClusterExtension, desiredRevision *ocv1ac.ClusterObjectSetApplyConfiguration, existingRevisions []ocv1.ClusterObjectSet) error { diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 25963c9a01..f46cd6b41f 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -666,6 +666,43 @@ func TestBoxcutter_Apply(t *testing.T) { assert.Equal(t, "test-ext-1", revList.Items[0].Name) }, }, + { + name: "adopts matching migration revision without creating a duplicate", + mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { + ctrl := gomock.NewController(t) + return mockapplier.NewMockClusterObjectSetGenerator(ctrl) + }, + existingObjs: []client.Object{ + &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext-1", + Labels: map[string]string{labels.OwnerNameKey: ext.Name}, + Annotations: map[string]string{ + labels.MigratedFromSubscriptionKey: "test-namespace/test-subscription", + labels.PackageNameKey: "test-package", + labels.BundleNameKey: "test-package.v1.0.0", + labels.BundleVersionKey: "1.0.0", + labels.BundleReferenceKey: "registry.example/test-package@sha256:123", + labels.BundleReleaseKey: "42", + }, + }, + Spec: ocv1.ClusterObjectSetSpec{Revision: 1}, + Status: ocv1.ClusterObjectSetStatus{Conditions: []metav1.Condition{{ + Type: ocv1.ClusterObjectSetTypeSucceeded, + Status: metav1.ConditionTrue, + }}}, + }, + }, + validate: func(t *testing.T, c client.Client) { + revList := &ocv1.ClusterObjectSetList{} + require.NoError(t, c.List(t.Context(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})) + require.Len(t, revList.Items, 1) + assert.Equal(t, "test-ext-1", revList.Items[0].Name) + require.Len(t, revList.Items[0].OwnerReferences, 1) + assert.Equal(t, ext.Name, revList.Items[0].OwnerReferences[0].Name) + assert.Equal(t, ext.UID, revList.Items[0].OwnerReferences[0].UID) + }, + }, { name: "new revision created when objects in new revision are different", mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { @@ -1081,12 +1118,21 @@ func TestBoxcutter_Apply(t *testing.T) { // Execute revisionAnnotations := map[string]string{} - if tc.name == "annotation-only update (same phases, different annotations)" { + switch tc.name { + case "annotation-only update (same phases, different annotations)": // For annotation-only update test, pass NEW annotations revisionAnnotations = map[string]string{ labels.BundleVersionKey: "1.0.1", labels.PackageNameKey: "test-package", } + case "adopts matching migration revision without creating a duplicate": + revisionAnnotations = map[string]string{ + labels.PackageNameKey: "test-package", + labels.BundleNameKey: "test-package.v1.0.0", + labels.BundleVersionKey: "1.0.0", + labels.BundleReferenceKey: "registry.example/test-package@sha256:123", + labels.BundleReleaseKey: "42", + } } completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations) diff --git a/internal/operator-controller/labels/labels.go b/internal/operator-controller/labels/labels.go index 3a0cdaf46a..e2dfd37aca 100644 --- a/internal/operator-controller/labels/labels.go +++ b/internal/operator-controller/labels/labels.go @@ -51,4 +51,10 @@ const ( // that were created during migration from Helm releases. This label is used // to distinguish migrated revisions from those created by normal Boxcutter operation. MigratedFromHelmKey = "olm.operatorframework.io/migrated-from-helm" + + // MigratedFromSubscriptionKey is the annotation placed on a revision created + // by the OLM v0-to-v1 migration tool. A matching revision 1 is adopted by a + // subsequently-created ClusterExtension instead of being replaced by its + // initial Boxcutter reconciliation. + MigratedFromSubscriptionKey = "olm.operatorframework.io/migrated-from-subscription" )