Skip to content

Commit 5738d68

Browse files
bentitotmshort
bentito
authored and
tmshort
committed
Tests for metadata guard
Verifies that exactly three updates (spec, status, guard) are issued when the observedGeneration doesn’t match. Signed-off-by: Brett Tofel <[email protected]>
1 parent da1e61f commit 5738d68

File tree

2 files changed

+112
-42
lines changed

2 files changed

+112
-42
lines changed

‎pkg/controller/operators/olm/operatorgroup.go‎

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -792,11 +792,11 @@ func copyableCSVHash(original *v1alpha1.ClusterServiceVersion) (string, string,
792792
}
793793

794794
const (
795-
nonStatusCopyHashAnnotation = "olm.operatorframework.io/nonStatusCopyHash"
796-
statusCopyHashAnnotation = "olm.operatorframework.io/statusCopyHash"
797-
// annotations for metadata drift guard
798-
observedGenerationAnnotation = "olm.operatorframework.io/observedGeneration"
799-
observedResourceVersionAnnotation = "olm.operatorframework.io/observedResourceVersion"
795+
nonStatusCopyHashAnnotation = "olm.operatorframework.io/nonStatusCopyHash"
796+
statusCopyHashAnnotation = "olm.operatorframework.io/statusCopyHash"
797+
// annotations for metadata drift guard
798+
observedGenerationAnnotation = "olm.operatorframework.io/observedGeneration"
799+
observedResourceVersionAnnotation = "olm.operatorframework.io/observedResourceVersion"
800800
)
801801

802802
// If returned error is not nil, the returned ClusterServiceVersion
@@ -832,43 +832,43 @@ func (a *Operator) copyToNamespace(prototype *v1alpha1.ClusterServiceVersion, ns
832832
UID: created.UID,
833833
},
834834
}, nil
835-
} else if err != nil {
836-
return nil, err
837-
}
838-
// metadata drift guard: detect manual modifications to spec or status
839-
if og, orv := existing.Annotations[observedGenerationAnnotation], existing.Annotations[observedResourceVersionAnnotation]; (og != "" && og != fmt.Sprint(existing.GetGeneration())) || (orv != "" && orv != existing.ResourceVersion) {
840-
// full resync for metadata drift
841-
// prepare prototype for update
842-
prototype.Namespace = existing.Namespace
843-
prototype.ResourceVersion = existing.ResourceVersion
844-
prototype.UID = existing.UID
845-
// sync hash annotations
846-
prototype.Annotations[nonStatusCopyHashAnnotation] = nonstatus
847-
prototype.Annotations[statusCopyHashAnnotation] = status
848-
// update spec and annotations
849-
updated, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), prototype, metav1.UpdateOptions{})
850-
if err != nil {
851-
return nil, fmt.Errorf("failed to resync spec for metadata drift guard: %w", err)
852-
}
853-
// update status subresource
854-
updated.Status = prototype.Status
855-
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).UpdateStatus(context.TODO(), updated, metav1.UpdateOptions{}); err != nil {
856-
return nil, fmt.Errorf("failed to resync status for metadata drift guard: %w", err)
857-
}
858-
// record observed generation and resourceVersion
859-
updated.Annotations[observedGenerationAnnotation] = fmt.Sprint(updated.GetGeneration())
860-
updated.Annotations[observedResourceVersionAnnotation] = updated.ResourceVersion
861-
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), updated, metav1.UpdateOptions{}); err != nil {
862-
return nil, fmt.Errorf("failed to update metadata guard annotations: %w", err)
863-
}
864-
return &v1alpha1.ClusterServiceVersion{
865-
ObjectMeta: metav1.ObjectMeta{
866-
Name: updated.Name,
867-
Namespace: updated.Namespace,
868-
UID: updated.UID,
869-
},
870-
}, nil
871-
}
835+
} else if err != nil {
836+
return nil, err
837+
}
838+
// metadata drift guard: detect manual modifications to spec or status
839+
if og, orv := existing.Annotations[observedGenerationAnnotation], existing.Annotations[observedResourceVersionAnnotation]; (og != "" && og != fmt.Sprint(existing.GetGeneration())) || (orv != "" && orv != existing.ResourceVersion) {
840+
// full resync for metadata drift
841+
// prepare prototype for update
842+
prototype.Namespace = existing.Namespace
843+
prototype.ResourceVersion = existing.ResourceVersion
844+
prototype.UID = existing.UID
845+
// sync hash annotations
846+
prototype.Annotations[nonStatusCopyHashAnnotation] = nonstatus
847+
prototype.Annotations[statusCopyHashAnnotation] = status
848+
// update spec and annotations
849+
updated, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), prototype, metav1.UpdateOptions{})
850+
if err != nil {
851+
return nil, fmt.Errorf("failed to resync spec for metadata drift guard: %w", err)
852+
}
853+
// update status subresource
854+
updated.Status = prototype.Status
855+
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).UpdateStatus(context.TODO(), updated, metav1.UpdateOptions{}); err != nil {
856+
return nil, fmt.Errorf("failed to resync status for metadata drift guard: %w", err)
857+
}
858+
// record observed generation and resourceVersion
859+
updated.Annotations[observedGenerationAnnotation] = fmt.Sprint(updated.GetGeneration())
860+
updated.Annotations[observedResourceVersionAnnotation] = updated.ResourceVersion
861+
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(nsTo).Update(context.TODO(), updated, metav1.UpdateOptions{}); err != nil {
862+
return nil, fmt.Errorf("failed to update metadata guard annotations: %w", err)
863+
}
864+
return &v1alpha1.ClusterServiceVersion{
865+
ObjectMeta: metav1.ObjectMeta{
866+
Name: updated.Name,
867+
Namespace: updated.Namespace,
868+
UID: updated.UID,
869+
},
870+
}, nil
871+
}
872872

873873
prototype.Namespace = existing.Namespace
874874
prototype.ResourceVersion = existing.ResourceVersion

‎pkg/controller/operators/olm/operatorgroup_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,76 @@ func (n *fakeCSVNamespaceLister) List(selector labels.Selector) ([]*metav1.Parti
3838
return result, nil
3939
}
4040

41+
// Test full resync via metadata drift guard when observedGeneration mismatches
42+
func TestCopyToNamespace_MetadataDriftGuard(t *testing.T) {
43+
// Prepare prototype CSV and compute hashes
44+
prototype := v1alpha1.ClusterServiceVersion{
45+
ObjectMeta: metav1.ObjectMeta{
46+
Name: "name",
47+
Annotations: map[string]string{},
48+
},
49+
Spec: v1alpha1.ClusterServiceVersionSpec{Replaces: "replacee"},
50+
Status: v1alpha1.ClusterServiceVersionStatus{Phase: "waxing gibbous"},
51+
}
52+
nonstatus, status, err := copyableCSVHash(&prototype)
53+
require.NoError(t, err)
54+
55+
// Existing partial copy with observedGeneration mismatched
56+
existingCopy := &metav1.PartialObjectMetadata{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "name",
59+
Namespace: "to",
60+
UID: "uid",
61+
ResourceVersion: "42",
62+
Generation: 2,
63+
Annotations: map[string]string{
64+
nonStatusCopyHashAnnotation: nonstatus,
65+
statusCopyHashAnnotation: status,
66+
observedGenerationAnnotation: "1",
67+
observedResourceVersionAnnotation: "42",
68+
},
69+
},
70+
}
71+
// Full CSV for fake client
72+
full := &v1alpha1.ClusterServiceVersion{
73+
ObjectMeta: metav1.ObjectMeta{
74+
Name: existingCopy.Name,
75+
Namespace: existingCopy.Namespace,
76+
UID: existingCopy.UID,
77+
ResourceVersion: existingCopy.ResourceVersion,
78+
Generation: existingCopy.Generation,
79+
Annotations: existingCopy.Annotations,
80+
},
81+
Spec: prototype.Spec,
82+
Status: prototype.Status,
83+
}
84+
85+
client := fake.NewSimpleClientset(full)
86+
lister := &fakeCSVLister{items: []*metav1.PartialObjectMetadata{existingCopy}}
87+
logger, _ := test.NewNullLogger()
88+
o := &Operator{copiedCSVLister: lister, client: client, logger: logger}
89+
90+
protoCopy := prototype.DeepCopy()
91+
result, err := o.copyToNamespace(protoCopy, "from", "to", nonstatus, status)
92+
require.NoError(t, err)
93+
require.Equal(t, "name", result.GetName())
94+
require.Equal(t, "to", result.GetNamespace())
95+
require.Equal(t, "uid", string(result.GetUID()))
96+
97+
actions := client.Actions()
98+
// Expect: update(spec), updateStatus, update(guard annotations)
99+
require.Len(t, actions, 3)
100+
// First action: spec update
101+
ua0 := actions[0].(ktesting.UpdateAction)
102+
require.Equal(t, "", ua0.GetSubresource())
103+
// Second action: status subresource
104+
ua1 := actions[1].(ktesting.UpdateAction)
105+
require.Equal(t, "status", ua1.GetSubresource())
106+
// Third action: metadata guard update
107+
ua2 := actions[2].(ktesting.UpdateAction)
108+
require.Equal(t, "", ua2.GetSubresource())
109+
}
110+
41111
func (n *fakeCSVNamespaceLister) Get(name string) (*metav1.PartialObjectMetadata, error) {
42112
for _, item := range n.items {
43113
if item != nil && item.Namespace == n.namespace && item.Name == name {

0 commit comments

Comments
 (0)