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]>

Author: shajmakh

internal/controller/numaresourcesscheduler_controller.go

@@ -311,6 +311,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 = schedSpec.Replicas
 	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
@@ -377,15 +380,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)
 	}

internal/controller/numaresourcesscheduler_controller_test.go

@@ -222,6 +222,67 @@ var _ = Describe("Test NUMAResourcesScheduler Reconcile", func() {
 			Expect(nrs.Status.CacheResyncPeriod.Seconds()).To(Equal(resyncPeriod.Seconds()))
 		})
 
+		Context("should reflect DedicatedInformerActive in status conditions", func() {
+			It("with default values", func() {
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				Expect(c).ToNot(BeNil())
+				Expect(c.Status).To(Equal(metav1.ConditionTrue))
+			})
+
+			It("with updated values - explicitly configured to Dedicated", func() {
+				nrs := nrs.DeepCopy()
+				nrs.Spec.SchedulerInformer = ptr.To(nropv1.SchedulerInformerDedicated)
+
+				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(BeTrue())
+
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				Expect(c).ToNot(BeNil())
+				Expect(c.Status).To(Equal(metav1.ConditionTrue))
+			})
+
+			It("with updated values - explicitly configured to Shared", func() {
+				nrs := nrs.DeepCopy()
+				nrs.Spec.SchedulerInformer = ptr.To(nropv1.SchedulerInformerShared)
+
+				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(BeTrue())
+
+				key := client.ObjectKeyFromObject(nrs)
+				_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(Succeed())
+
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				Expect(c).ToNot(BeNil())
+				Expect(c.Status).To(Equal(metav1.ConditionFalse))
+			})
+		})
+
 		It("should have the correct priority class", func() {
 			key := client.ObjectKeyFromObject(nrs)
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key})
@@ -749,6 +810,17 @@ var _ = Describe("Test NUMAResourcesScheduler Reconcile", func() {
 				Expect(err).ToNot(HaveOccurred())
 
 				expectCacheParams(reconciler.Client, depmanifests.CacheResyncAutodetect, depmanifests.CacheResyncOnlyExclusiveResources, expectedInformer)
+
+				expectedDedicatedActiveStatus := metav1.ConditionTrue
+				if expectedInformer == depmanifests.CacheInformerShared {
+					expectedDedicatedActiveStatus = metav1.ConditionFalse
+				}
+
+				Expect(reconciler.Client.Get(context.TODO(), key, nrs)).To(Succeed())
+				c := getConditionByType(nrs.Status.Conditions, status.ConditionDedicatedInformerActive)
+
+				Expect(c).ToNot(BeNil())
+				Expect(c.Status).To(Equal(expectedDedicatedActiveStatus))
 			},
 				Entry("with fixed Openshift the default informer is Shared", PlatformInfo{
 					Platform: platform.OpenShift,

pkg/status/status.go

@@ -34,6 +34,10 @@ const (
 	ConditionUpgradeable = "Upgradeable"
 )
 
+const (
+	ConditionDedicatedInformerActive = "DedicatedInformerActive"
+)
+
 // TODO: are we duping these?
 const (
 	ReasonAsExpected    = "AsExpected"
@@ -68,14 +72,24 @@ 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)
 
-	cond := clone(conditions)
-	curCond := clone(currentConditions)
+	cond := CloneConditions(conditions)
+	curCond := CloneConditions(currentConditions)
 
 	resetIncomparableConditionFields(cond)
 	resetIncomparableConditionFields(curCond)
@@ -144,6 +158,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
@@ -169,8 +219,37 @@ func resetIncomparableConditionFields(conditions []metav1.Condition) {
 	}
 }
 
-func clone(conditions []metav1.Condition) []metav1.Condition {
+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
+}