Skip to content

Commit 2215296

Browse files
bentitobentito
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 8d46851 commit 2215296

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
@@ -791,11 +791,11 @@ func copyableCSVHash(original *v1alpha1.ClusterServiceVersion) (string, string,
791791
}
792792

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

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

872872
prototype.Namespace = existing.Namespace
873873
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
@@ -34,6 +34,76 @@ func (n *fakeCSVNamespaceLister) List(selector labels.Selector) ([]*metav1.Parti
3434
return result, nil
3535
}
3636

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

0 commit comments

Comments
 (0)