Skip to content

Commit b0a6ee7

Browse files
committed
Order an operator CR's status.Component.Refs array
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]>
1 parent 91e5d90 commit b0a6ee7

File tree

3 files changed

+62
-3
lines changed

3 files changed

+62
-3
lines changed

pkg/controller/operators/decorators/operator.go

+17
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,22 @@ func (o *Operator) AddComponents(components ...runtime.Object) error {
331332

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

335+
// Sort to ensure ordering
336+
sort.SliceStable(o.Status.Components.Refs, func(i, j int) bool {
337+
if o.Status.Components.Refs[i].Kind != o.Status.Components.Refs[j].Kind {
338+
return o.Status.Components.Refs[i].Kind < o.Status.Components.Refs[j].Kind
339+
}
340+
341+
if o.Status.Components.Refs[i].APIVersion != o.Status.Components.Refs[j].APIVersion {
342+
return o.Status.Components.Refs[i].APIVersion < o.Status.Components.Refs[j].APIVersion
343+
}
344+
345+
if o.Status.Components.Refs[i].Namespace != o.Status.Components.Refs[j].Namespace {
346+
return o.Status.Components.Refs[i].Namespace < o.Status.Components.Refs[j].Namespace
347+
}
348+
return o.Status.Components.Refs[i].Name < o.Status.Components.Refs[j].Name
349+
})
350+
334351
return nil
335352
}
336353

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

pkg/controller/operators/operator_controller_test.go

+39
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,45 @@ var _ = Describe("Operator Controller", func() {
204204
})
205205
})
206206

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

209248
BeforeEach(func() {

0 commit comments

Comments
 (0)