Skip to content

Commit 793a7cc

Browse files
authored
Order an operator CR's status.Component.Refs array (#2880)
Problem: The operator CR's status includes a list of componenets owned by the operator. The list of components are ordered by GVK, but the order of objects with the same GVK may change. If an operator owns many components, there is a high likelyhood that OLM will continuously update the status of the operator CR because the order of its components have changed, Solution: Order an operators component references so OLM does not attempt to update the status of the operator CR. Signed-off-by: Alexander Greene <[email protected]> Signed-off-by: Alexander Greene <[email protected]>
1 parent 91e5d90 commit 793a7cc

File tree

4 files changed

+79
-19
lines changed

4 files changed

+79
-19
lines changed

Diff for: pkg/controller/operators/decorators/operator.go

+18
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package decorators
22

33
import (
44
"fmt"
5+
"sort"
56
"strings"
67

78
"github.com/itchyny/gojq"
@@ -331,6 +332,23 @@ func (o *Operator) AddComponents(components ...runtime.Object) error {
331332

332333
o.Status.Components.Refs = append(o.Status.Components.Refs, refs...)
333334

335+
// Sort the component refs to so subsequent reconciles of the object do not change
336+
// the status and result in an update to the object.
337+
sort.SliceStable(o.Status.Components.Refs, func(i, j int) bool {
338+
if o.Status.Components.Refs[i].Kind != o.Status.Components.Refs[j].Kind {
339+
return o.Status.Components.Refs[i].Kind < o.Status.Components.Refs[j].Kind
340+
}
341+
342+
if o.Status.Components.Refs[i].APIVersion != o.Status.Components.Refs[j].APIVersion {
343+
return o.Status.Components.Refs[i].APIVersion < o.Status.Components.Refs[j].APIVersion
344+
}
345+
346+
if o.Status.Components.Refs[i].Namespace != o.Status.Components.Refs[j].Namespace {
347+
return o.Status.Components.Refs[i].Namespace < o.Status.Components.Refs[j].Namespace
348+
}
349+
return o.Status.Components.Refs[i].Name < o.Status.Components.Refs[j].Name
350+
})
351+
334352
return nil
335353
}
336354

Diff for: pkg/controller/operators/decorators/operator_test.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,22 @@ func TestAddComponents(t *testing.T) {
174174
},
175175
}
176176
operator.Status.Components.Refs = []operatorsv1.RichReference{
177+
{
178+
ObjectReference: &corev1.ObjectReference{
179+
APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
180+
Kind: operatorsv1alpha1.ClusterServiceVersionKind,
181+
Namespace: "atlantic",
182+
Name: "puffin",
183+
},
184+
Conditions: []operatorsv1.Condition{
185+
{
186+
Type: operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
187+
Status: corev1.ConditionTrue,
188+
Reason: string(operatorsv1alpha1.CSVReasonInstallSuccessful),
189+
Message: "this puffin is happy",
190+
},
191+
},
192+
},
177193
{
178194
ObjectReference: &corev1.ObjectReference{
179195
APIVersion: "v1",
@@ -195,22 +211,6 @@ func TestAddComponents(t *testing.T) {
195211
},
196212
},
197213
},
198-
{
199-
ObjectReference: &corev1.ObjectReference{
200-
APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
201-
Kind: operatorsv1alpha1.ClusterServiceVersionKind,
202-
Namespace: "atlantic",
203-
Name: "puffin",
204-
},
205-
Conditions: []operatorsv1.Condition{
206-
{
207-
Type: operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
208-
Status: corev1.ConditionTrue,
209-
Reason: string(operatorsv1alpha1.CSVReasonInstallSuccessful),
210-
Message: "this puffin is happy",
211-
},
212-
},
213-
},
214214
}
215215

216216
return operator

Diff for: pkg/controller/operators/operator_controller.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
corev1 "k8s.io/api/core/v1"
1010
rbacv1 "k8s.io/api/rbac/v1"
1111
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
12+
"k8s.io/apimachinery/pkg/api/equality"
1213
apierrors "k8s.io/apimachinery/pkg/api/errors"
1314
"k8s.io/apimachinery/pkg/api/meta"
1415
"k8s.io/apimachinery/pkg/labels"
@@ -152,9 +153,11 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
152153
return ctrl.Result{Requeue: true}, nil
153154
}
154155
} else {
155-
if err := r.Status().Update(ctx, operator.Operator); err != nil {
156-
log.Error(err, "Could not update Operator status")
157-
return ctrl.Result{Requeue: true}, nil
156+
if !equality.Semantic.DeepEqual(in.Status, operator.Operator.Status) {
157+
if err := r.Status().Update(ctx, operator.Operator); err != nil {
158+
log.Error(err, "Could not update Operator status")
159+
return ctrl.Result{Requeue: true}, nil
160+
}
158161
}
159162
}
160163

Diff for: pkg/controller/operators/operator_controller_test.go

+39
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package operators
22

33
import (
44
"context"
5+
"fmt"
56

67
. "github.com/onsi/ginkgo/v2"
78
. "github.com/onsi/gomega"
@@ -204,6 +205,44 @@ var _ = Describe("Operator Controller", func() {
204205
})
205206
})
206207

208+
Context("when multiple types of a gvk are labeled", func() {
209+
BeforeEach(func() {
210+
newObjs := make([]runtime.Object, 9)
211+
212+
// Create objects in reverse order to ensure they are eventually ordered alphabetically in the status of the operator.
213+
for i := 8; i >= 0; i-- {
214+
newObjs[8-i] = testobj.WithLabels(map[string]string{expectedKey: ""}, testobj.WithNamespacedName(
215+
&types.NamespacedName{Namespace: namespace, Name: fmt.Sprintf("sa-%d", i)}, &corev1.ServiceAccount{},
216+
))
217+
}
218+
219+
for _, obj := range newObjs {
220+
Expect(k8sClient.Create(ctx, obj.(client.Object))).To(Succeed())
221+
}
222+
223+
objs = append(objs, newObjs...)
224+
expectedRefs = append(expectedRefs, toRefs(scheme, newObjs...)...)
225+
})
226+
227+
It("should list each of the component references in alphabetical order by namespace and name", func() {
228+
Eventually(func() ([]operatorsv1.RichReference, error) {
229+
err := k8sClient.Get(ctx, name, operator)
230+
return operator.Status.Components.Refs, err
231+
}, timeout, interval).Should(ConsistOf(expectedRefs))
232+
233+
serviceAccountCount := 0
234+
for _, ref := range operator.Status.Components.Refs {
235+
if ref.Kind != rbacv1.ServiceAccountKind {
236+
continue
237+
}
238+
239+
Expect(ref.Name).Should(Equal(fmt.Sprintf("sa-%d", serviceAccountCount)))
240+
serviceAccountCount++
241+
}
242+
Expect(serviceAccountCount).To(Equal(9))
243+
})
244+
})
245+
207246
Context("when component labels are removed", func() {
208247

209248
BeforeEach(func() {

0 commit comments

Comments
 (0)