Merge pull request openshift#406 from awgreene/fix-api-spam

OCPBUGS-2556: Order an operator CR's status.Component.Refs array (#2880)

Author: openshift-merge-robot

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go

@@ -2,6 +2,7 @@ package decorators
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/itchyny/gojq"
@@ -331,6 +332,23 @@ func (o *Operator) AddComponents(components ...runtime.Object) error {
 
 	o.Status.Components.Refs = append(o.Status.Components.Refs, refs...)
 
+	// Sort the component refs to so subsequent reconciles of the object do not change
+	// the status and result in an update to the object.
+	sort.SliceStable(o.Status.Components.Refs, func(i, j int) bool {
+		if o.Status.Components.Refs[i].Kind != o.Status.Components.Refs[j].Kind {
+			return o.Status.Components.Refs[i].Kind < o.Status.Components.Refs[j].Kind
+		}
+
+		if o.Status.Components.Refs[i].APIVersion != o.Status.Components.Refs[j].APIVersion {
+			return o.Status.Components.Refs[i].APIVersion < o.Status.Components.Refs[j].APIVersion
+		}
+
+		if o.Status.Components.Refs[i].Namespace != o.Status.Components.Refs[j].Namespace {
+			return o.Status.Components.Refs[i].Namespace < o.Status.Components.Refs[j].Namespace
+		}
+		return o.Status.Components.Refs[i].Name < o.Status.Components.Refs[j].Name
+	})
+
 	return nil
 }
 

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go

@@ -174,6 +174,22 @@ func TestAddComponents(t *testing.T) {
 						},
 					}
 					operator.Status.Components.Refs = []operatorsv1.RichReference{
+						{
+							ObjectReference: &corev1.ObjectReference{
+								APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
+								Kind:       operatorsv1alpha1.ClusterServiceVersionKind,
+								Namespace:  "atlantic",
+								Name:       "puffin",
+							},
+							Conditions: []operatorsv1.Condition{
+								{
+									Type:    operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
+									Status:  corev1.ConditionTrue,
+									Reason:  string(operatorsv1alpha1.CSVReasonInstallSuccessful),
+									Message: "this puffin is happy",
+								},
+							},
+						},
 						{
 							ObjectReference: &corev1.ObjectReference{
 								APIVersion: "v1",
@@ -195,22 +211,6 @@ func TestAddComponents(t *testing.T) {
 								},
 							},
 						},
-						{
-							ObjectReference: &corev1.ObjectReference{
-								APIVersion: operatorsv1alpha1.SchemeGroupVersion.String(),
-								Kind:       operatorsv1alpha1.ClusterServiceVersionKind,
-								Namespace:  "atlantic",
-								Name:       "puffin",
-							},
-							Conditions: []operatorsv1.Condition{
-								{
-									Type:    operatorsv1.ConditionType(operatorsv1alpha1.CSVPhaseSucceeded),
-									Status:  corev1.ConditionTrue,
-									Reason:  string(operatorsv1alpha1.CSVReasonInstallSuccessful),
-									Message: "this puffin is happy",
-								},
-							},
-						},
 					}
 
 					return operator

staging/operator-lifecycle-manager/pkg/controller/operators/operator_controller.go

@@ -9,6 +9,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
@@ -152,9 +153,11 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			return ctrl.Result{Requeue: true}, nil
 		}
 	} else {
-		if err := r.Status().Update(ctx, operator.Operator); err != nil {
-			log.Error(err, "Could not update Operator status")
-			return ctrl.Result{Requeue: true}, nil
+		if !equality.Semantic.DeepEqual(in.Status, operator.Operator.Status) {
+			if err := r.Status().Update(ctx, operator.Operator); err != nil {
+				log.Error(err, "Could not update Operator status")
+				return ctrl.Result{Requeue: true}, nil
+			}
 		}
 	}
 

staging/operator-lifecycle-manager/pkg/controller/operators/operator_controller_test.go

@@ -2,6 +2,7 @@ package operators
 
 import (
 	"context"
+	"fmt"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -204,6 +205,44 @@ var _ = Describe("Operator Controller", func() {
 				})
 			})
 
+			Context("when multiple types of a gvk are labeled", func() {
+				BeforeEach(func() {
+					newObjs := make([]runtime.Object, 9)
+
+					// Create objects in reverse order to ensure they are eventually ordered alphabetically in the status of the operator.
+					for i := 8; i >= 0; i-- {
+						newObjs[8-i] = testobj.WithLabels(map[string]string{expectedKey: ""}, testobj.WithNamespacedName(
+							&types.NamespacedName{Namespace: namespace, Name: fmt.Sprintf("sa-%d", i)}, &corev1.ServiceAccount{},
+						))
+					}
+
+					for _, obj := range newObjs {
+						Expect(k8sClient.Create(ctx, obj.(client.Object))).To(Succeed())
+					}
+
+					objs = append(objs, newObjs...)
+					expectedRefs = append(expectedRefs, toRefs(scheme, newObjs...)...)
+				})
+
+				It("should list each of the component references in alphabetical order by namespace and name", func() {
+					Eventually(func() ([]operatorsv1.RichReference, error) {
+						err := k8sClient.Get(ctx, name, operator)
+						return operator.Status.Components.Refs, err
+					}, timeout, interval).Should(ConsistOf(expectedRefs))
+
+					serviceAccountCount := 0
+					for _, ref := range operator.Status.Components.Refs {
+						if ref.Kind != rbacv1.ServiceAccountKind {
+							continue
+						}
+
+						Expect(ref.Name).Should(Equal(fmt.Sprintf("sa-%d", serviceAccountCount)))
+						serviceAccountCount++
+					}
+					Expect(serviceAccountCount).To(Equal(9))
+				})
+			})
+
 			Context("when component labels are removed", func() {
 
 				BeforeEach(func() {