Skip to content

Commit 622df64

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 622df64

File tree

3 files changed

+65
-3
lines changed

3 files changed

+65
-3
lines changed

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

pkg/controller/operators/operator_controller.go

+8-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"
@@ -134,6 +135,8 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
134135
}
135136
}
136137

138+
log.V(1).Info("operator", "comp", in.Status)
139+
137140
// Wrap with convenience decorator
138141
operator, err := r.factory.NewOperator(in)
139142
if err != nil {
@@ -152,9 +155,11 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
152155
return ctrl.Result{Requeue: true}, nil
153156
}
154157
} 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
158+
if !equality.Semantic.DeepEqual(in.Status, operator.Operator.Status) {
159+
if err := r.Status().Update(ctx, operator.Operator); err != nil {
160+
log.Error(err, "Could not update Operator status")
161+
return ctrl.Result{Requeue: true}, nil
162+
}
158163
}
159164
}
160165

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)