Skip to content

Commit 46c7962

Browse files
test: Verify migrated revision status is set correctly
Add explicit test assertions to verify that the BoxcutterStorageMigrator sets the Succeeded=True status condition with the Migrated reason when creating revisions from Helm releases. This ensures the migration fix works correctly by: - Verifying Status().Update() is called during migration - Asserting the Succeeded condition is set to True - Confirming the reason is ClusterExtensionRevisionReasonMigrated - Validating the message and ObservedGeneration are correct Also adds missing k8s.io/apimachinery/pkg/api/meta import to boxcutter.go that was needed for the meta.SetStatusCondition call. The test now captures the status update in a persistent statusWriterMock instance, allowing verification of the status condition that prevents the timing gap issue during OLM upgrades. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 0f9ea71 commit 46c7962

2 files changed

Lines changed: 28 additions & 3 deletions

File tree

internal/operator-controller/applier/boxcutter.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"helm.sh/helm/v3/pkg/release"
1616
"helm.sh/helm/v3/pkg/storage/driver"
1717
apierrors "k8s.io/apimachinery/pkg/api/errors"
18+
"k8s.io/apimachinery/pkg/api/meta"
1819
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1920
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2021
"k8s.io/apimachinery/pkg/runtime"

internal/operator-controller/applier/boxcutter_test.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
corev1 "k8s.io/api/core/v1"
2020
rbacv1 "k8s.io/api/rbac/v1"
2121
apierrors "k8s.io/apimachinery/pkg/api/errors"
22+
apimeta "k8s.io/apimachinery/pkg/api/meta"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2324
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2425
"k8s.io/apimachinery/pkg/runtime"
@@ -1251,6 +1252,21 @@ func TestBoxcutterStorageMigrator(t *testing.T) {
12511252
require.NoError(t, err)
12521253

12531254
client.AssertExpectations(t)
1255+
1256+
// Verify the migrated revision has Succeeded=True status with Migrated reason
1257+
statusWriter := client.Status().(*statusWriterMock)
1258+
require.True(t, statusWriter.updateCalled, "Status().Update() should be called during migration")
1259+
require.NotNil(t, statusWriter.updatedObj, "Updated object should not be nil")
1260+
1261+
rev, ok := statusWriter.updatedObj.(*ocv1.ClusterExtensionRevision)
1262+
require.True(t, ok, "Updated object should be a ClusterExtensionRevision")
1263+
1264+
succeededCond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeSucceeded)
1265+
require.NotNil(t, succeededCond, "Succeeded condition should be set")
1266+
assert.Equal(t, metav1.ConditionTrue, succeededCond.Status, "Succeeded condition should be True")
1267+
assert.Equal(t, ocv1.ClusterExtensionRevisionReasonMigrated, succeededCond.Reason, "Reason should be Migrated")
1268+
assert.Equal(t, "Migrated from Helm release", succeededCond.Message, "Message should indicate Helm migration")
1269+
assert.Equal(t, int64(1), succeededCond.ObservedGeneration, "ObservedGeneration should match revision generation")
12541270
})
12551271

12561272
t.Run("does not create revision when revisions exist", func(t *testing.T) {
@@ -1345,6 +1361,7 @@ func (m *mockBundleRevisionBuilder) GenerateRevisionFromHelmRelease(
13451361

13461362
type clientMock struct {
13471363
mock.Mock
1364+
statusWriter *statusWriterMock
13481365
}
13491366

13501367
func (m *clientMock) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
@@ -1363,15 +1380,22 @@ func (m *clientMock) Create(ctx context.Context, obj client.Object, opts ...clie
13631380
}
13641381

13651382
func (m *clientMock) Status() client.StatusWriter {
1366-
return &statusWriterMock{mock: &m.Mock}
1383+
if m.statusWriter == nil {
1384+
m.statusWriter = &statusWriterMock{mock: &m.Mock}
1385+
}
1386+
return m.statusWriter
13671387
}
13681388

13691389
type statusWriterMock struct {
1370-
mock *mock.Mock
1390+
mock *mock.Mock
1391+
updatedObj client.Object
1392+
updateCalled bool
13711393
}
13721394

13731395
func (s *statusWriterMock) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error {
1374-
// Status updates are expected during migration - return success by default
1396+
// Capture the status update for test verification
1397+
s.updatedObj = obj
1398+
s.updateCalled = true
13751399
return nil
13761400
}
13771401

0 commit comments

Comments
 (0)