controller: sched: report DedicatedInformerActive behavior

We want to be able to see the informer mode of the scheduler as a
preparation step for having the informer default change according to the
kubelet version.
This commit enables reflecting the informer value (default or
configured) as part of the status conditions which is less direct than
reporting it in spec and is valid.

Signed-off-by: Shereen Haj <[email protected]>
(cherry picked from commit 09d76b9)
(cherry picked from commit 79be8cb)

Author: shajmakh

controllers/numaresourcesscheduler_controller.go

@@ -224,6 +224,9 @@ func (r *NUMAResourcesSchedulerReconciler) syncNUMASchedulerResources(ctx contex
 		Duration: cacheResyncPeriod,
 	}
 
+	informerCondition := buildDedicatedInformerCondition(*instance, schedSpec)
+	schedStatus.Conditions = status.GetUpdatedSchedulerConditions(schedStatus.Conditions, informerCondition)
+
 	r.SchedulerManifests.Deployment.Spec.Replicas = r.computeSchedulerReplicas(schedSpec)
 	klog.V(4).InfoS("using scheduler replicas", "replicas", *r.SchedulerManifests.Deployment.Spec.Replicas)
 	// TODO: if replicas doesn't make sense (autodetect disabled and user set impossible value) then we
@@ -289,15 +292,39 @@ func platformNormalize(spec *nropv1.NUMAResourcesSchedulerSpec, platInfo Platfor
 		klog.V(4).InfoS("SchedulerInformer default is overridden", "Platform", platInfo.Platform, "PlatformVersion", platInfo.Version.String(), "SchedulerInformer", &spec.SchedulerInformer)
 	}
 }
+
+func buildDedicatedInformerCondition(instance nropv1.NUMAResourcesScheduler, normalized nropv1.NUMAResourcesSchedulerSpec) metav1.Condition {
+	condition := metav1.Condition{
+		Type:               status.ConditionDedicatedInformerActive,
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: instance.ObjectMeta.Generation,
+		Reason:             status.ConditionDedicatedInformerActive,
+	}
+
+	if *normalized.SchedulerInformer == nropv1.SchedulerInformerShared {
+		condition.Status = metav1.ConditionFalse
+	}
+
+	return condition
+}
+
 func (r *NUMAResourcesSchedulerReconciler) updateStatus(ctx context.Context, initialStatus nropv1.NUMAResourcesSchedulerStatus, sched *nropv1.NUMAResourcesScheduler, condition string, reason string, message string) error {
-	updatedStatus := *sched.Status.DeepCopy()
+	c := metav1.Condition{
+		Type:    condition,
+		Status:  metav1.ConditionTrue,
+		Reason:  reason,
+		Message: message,
+	}
+	sched.Status.Conditions = status.GetUpdatedSchedulerConditions(sched.Status.Conditions, c)
 
-	updatedStatus.Conditions, _ = status.UpdateConditions(sched.Status.Conditions, condition, reason, message)
-	if !status.NUMAResourcesSchedulerNeedsUpdate(initialStatus, updatedStatus) {
+	if !status.NUMAResourcesSchedulerNeedsUpdate(initialStatus, sched.Status) {
 		return nil
 	}
 
-	sched.Status.Conditions = updatedStatus.Conditions
+	if status.EqualConditions(initialStatus.Conditions, sched.Status.Conditions) {
+		sched.Status.Conditions = initialStatus.Conditions
+	}
+
 	if err := r.Client.Status().Update(ctx, sched); err != nil {
 		return fmt.Errorf("could not update status for object %s: %w", client.ObjectKeyFromObject(sched), err)
 	}

controllers/numaresourcesscheduler_controller_test.go

@@ -216,6 +216,67 @@ var _ = ginkgo.Describe("Test NUMAResourcesScheduler Reconcile", func() {
 			gomega.Expect(nrs.Status.CacheResyncPeriod.Seconds()).To(gomega.Equal(resyncPeriod.Seconds()))
 		})
 
+		ginkgo.Context("should reflect DedicatedInformerActive in status conditions", func() {
+			ginkgo.It("with default values", func() {
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				gomega.Expect(err).ToNot(gomega.HaveOccurred())
+
+				gomega.Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(gomega.Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				gomega.Expect(c).ToNot(gomega.BeNil())
+				gomega.Expect(c.Status).To(gomega.Equal(metav1.ConditionTrue))
+			})
+
+			ginkgo.It("with updated values - explicitly configured to Dedicated", func() {
+				nrs := nrs.DeepCopy()
+				nrs.Spec.SchedulerInformer = ptr.To(nropv1.SchedulerInformerDedicated)
+
+				gomega.Eventually(func() bool {
+					if err := reconciler.Client.Update(context.TODO(), nrs); err != nil {
+						klog.Warningf("failed to update the scheduler object; err: %v", err)
+						return false
+					}
+					return true
+				}, 30*time.Second, 5*time.Second).Should(gomega.BeTrue())
+
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				gomega.Expect(err).ToNot(gomega.HaveOccurred())
+				gomega.Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(gomega.Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				gomega.Expect(c).ToNot(gomega.BeNil())
+				gomega.Expect(c.Status).To(gomega.Equal(metav1.ConditionTrue))
+			})
+
+			ginkgo.It("with updated values - explicitly configured to Shared", func() {
+				nrs := nrs.DeepCopy()
+				nrs.Spec.SchedulerInformer = ptr.To(nropv1.SchedulerInformerShared)
+
+				gomega.Eventually(func() bool {
+					if err := reconciler.Client.Update(context.TODO(), nrs); err != nil {
+						klog.Warningf("failed to update the scheduler object; err: %v", err)
+						return false
+					}
+					return true
+				}, 30*time.Second, 5*time.Second).Should(gomega.BeTrue())
+
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				gomega.Expect(err).ToNot(gomega.HaveOccurred())
+				gomega.Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(gomega.Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				gomega.Expect(c).ToNot(gomega.BeNil())
+				gomega.Expect(c.Status).To(gomega.Equal(metav1.ConditionFalse))
+			})
+		})
+
 		ginkgo.It("should have the correct priority class", func() {
 			key := client.ObjectKeyFromObject(nrs)
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
@@ -720,6 +781,17 @@ var _ = ginkgo.Describe("Test NUMAResourcesScheduler Reconcile", func() {
 				gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
 				expectCacheParams(reconciler.Client, depmanifests.CacheResyncAutodetect, depmanifests.CacheResyncOnlyExclusiveResources, expectedInformer)
+
+				expectedDedicatedActiveStatus := metav1.ConditionTrue
+				if expectedInformer == depmanifests.CacheInformerShared {
+					expectedDedicatedActiveStatus = metav1.ConditionFalse
+				}
+
+				gomega.Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(gomega.Succeed())
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				gomega.Expect(c).ToNot(gomega.BeNil())
+				gomega.Expect(c.Status).To(gomega.Equal(expectedDedicatedActiveStatus))
 			},
 				ginkgo.Entry("with fixed Openshift the default informer is Shared", PlatformInfo{
 					Platform: platform.OpenShift,

pkg/status/status.go

@@ -37,6 +37,10 @@ const (
 	ConditionUpgradeable = "Upgradeable"
 )
 
+const (
+	ConditionDedicatedInformerActive = "DedicatedInformerActive"
+)
+
 // TODO: are we duping these?
 const (
 	ReasonAsExpected    = "AsExpected"
@@ -70,18 +74,23 @@ func NUMAResourcesSchedulerNeedsUpdate(oldStatus, newStatus nropv1.NUMAResources
 	return !reflect.DeepEqual(os, ns)
 }
 
+func EqualConditions(current, updated []metav1.Condition) bool {
+	c := CloneConditions(current)
+	u := CloneConditions(updated)
+
+	resetIncomparableConditionFields(c)
+	resetIncomparableConditionFields(u)
+
+	return reflect.DeepEqual(c, u)
+}
+
 // UpdateConditions compute new conditions based on arguments, and then compare with given current conditions.
 // Returns the conditions to use, either current or newly computed, and a boolean flag which is `true` if conditions need
 // update - so if they are updated since the current conditions.
 func UpdateConditions(currentConditions []metav1.Condition, condition string, reason string, message string) ([]metav1.Condition, bool) {
 	conditions := NewConditions(condition, reason, message)
 
-	options := []cmp.Option{
-		cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
-		cmpopts.IgnoreFields(metav1.Condition{}, "ObservedGeneration"),
-	}
-
-	if cmp.Equal(conditions, currentConditions, options...) {
+	if EqualConditions(currentConditions, conditions) {
 		return currentConditions, false
 	}
 	return conditions, true
@@ -145,6 +154,42 @@ func newBaseConditions() []metav1.Condition {
 	}
 }
 
+func NewNUMAResourcesSchedulerBaseConditions() []metav1.Condition {
+	now := time.Now()
+	return []metav1.Condition{
+		{
+			Type:               ConditionAvailable,
+			Status:             metav1.ConditionFalse,
+			LastTransitionTime: metav1.Time{Time: now},
+			Reason:             ConditionAvailable,
+		},
+		{
+			Type:               ConditionUpgradeable,
+			Status:             metav1.ConditionFalse,
+			LastTransitionTime: metav1.Time{Time: now},
+			Reason:             ConditionUpgradeable,
+		},
+		{
+			Type:               ConditionProgressing,
+			Status:             metav1.ConditionFalse,
+			LastTransitionTime: metav1.Time{Time: now},
+			Reason:             ConditionProgressing,
+		},
+		{
+			Type:               ConditionDegraded,
+			Status:             metav1.ConditionFalse,
+			LastTransitionTime: metav1.Time{Time: now},
+			Reason:             ConditionDegraded,
+		},
+		{
+			Type:               ConditionDedicatedInformerActive,
+			Status:             metav1.ConditionUnknown,
+			LastTransitionTime: metav1.Time{Time: now},
+			Reason:             ConditionDedicatedInformerActive,
+		},
+	}
+}
+
 func ReasonFromError(err error) string {
 	if err == nil {
 		return ReasonAsExpected
@@ -162,3 +207,44 @@ func MessageFromError(err error) string {
 	}
 	return unwErr.Error()
 }
+
+func resetIncomparableConditionFields(conditions []metav1.Condition) {
+	for idx := range conditions {
+		conditions[idx].LastTransitionTime = metav1.Time{}
+		conditions[idx].ObservedGeneration = 0
+	}
+}
+
+func CloneConditions(conditions []metav1.Condition) []metav1.Condition {
+	var c = make([]metav1.Condition, len(conditions))
+	copy(c, conditions)
+	return c
+}
+
+func GetUpdatedSchedulerConditions(conditions []metav1.Condition, condition metav1.Condition) []metav1.Condition {
+	updatedConditions := NewNUMAResourcesSchedulerBaseConditions()
+	if len(conditions) != 0 {
+		updatedConditions = CloneConditions(conditions)
+	}
+
+	now := time.Now()
+	for idx, cond := range updatedConditions {
+		if cond.Type == condition.Type {
+			updatedConditions[idx] = condition
+			updatedConditions[idx].LastTransitionTime = metav1.Time{Time: now}
+			break
+		}
+	}
+
+	if condition.Type == ConditionAvailable {
+		for idx, cond := range updatedConditions {
+			if cond.Type == ConditionUpgradeable {
+				updatedConditions[idx].Status = condition.Status
+				updatedConditions[idx].LastTransitionTime = metav1.Time{Time: now}
+				break
+			}
+		}
+	}
+
+	return updatedConditions
+}