Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
48 changes: 47 additions & 1 deletion internal/operator-controller/applier/boxcutter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations)

Expand Down
6 changes: 6 additions & 0 deletions internal/operator-controller/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Loading