ctrl: sched: add topologySpreadConstraints to deployment

To evenly spread the pods across the nodes in a a balanced way taking
into account the new replicaset pods while rollout.

Signed-off-by: Shereen Haj <shajmakh@redhat.com>

Author: shajmakh

internal/controller/numaresourcesscheduler_controller.go

@@ -364,6 +364,10 @@ func (r *NUMAResourcesSchedulerReconciler) syncNUMASchedulerResources(ctx contex
 		return nropv1.NUMAResourcesSchedulerStatus{}, err
 	}
 
+	if err := schedupdate.DeploymentTopologySpreadConstraints(r.SchedulerManifests.Deployment); err != nil {
+		return nropv1.NUMAResourcesSchedulerStatus{}, err
+	}
+
 	k8swgrbacupdate.RoleForLeaderElection(r.SchedulerManifests.Role, r.Namespace, nrosched.LeaderElectionResourceName)
 	k8swgrbacupdate.RoleBinding(r.SchedulerManifests.RoleBinding, r.SchedulerManifests.ServiceAccount.Name, r.Namespace)
 

internal/controller/numaresourcesscheduler_controller_test.go

@@ -1030,6 +1030,28 @@ var _ = Describe("Test NUMAResourcesScheduler Reconcile", func() {
 			Expect(dp.Spec.Template.Spec.Containers[0].Args).To(ContainElement("--tls-min-version=" + updatedSettings.MinVersion))
 			Expect(dp.Spec.Template.Spec.Containers[0].Args).To(ContainElement("--tls-cipher-suites=" + updatedSettings.CipherSuites))
 		})
+
+		It("should set the TopologySpreadConstraints in the deployment by default", func() {
+			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: nrsKey})
+			Expect(err).ToNot(HaveOccurred())
+
+			Expect(reconciler.Client.Get(context.TODO(), nrsKey, nrs)).To(Succeed())
+			dpKey := client.ObjectKey{Namespace: nrs.Status.Deployment.Namespace, Name: nrs.Status.Deployment.Name}
+			dp := &appsv1.Deployment{}
+			Expect(reconciler.Client.Get(context.TODO(), dpKey, dp)).To(Succeed())
+			Expect(dp.Spec.Template.Spec.TopologySpreadConstraints).To(HaveLen(1))
+
+			expectedConstraint := corev1.TopologySpreadConstraint{
+				MaxSkew:           1,
+				TopologyKey:       "kubernetes.io/hostname",
+				WhenUnsatisfiable: corev1.DoNotSchedule,
+				MatchLabelKeys:    []string{"pod-template-hash"},
+				LabelSelector: &metav1.LabelSelector{
+					MatchLabels: dp.Spec.Template.Labels,
+				},
+			}
+			Expect(dp.Spec.Template.Spec.TopologySpreadConstraints[0]).To(Equal(expectedConstraint))
+		})
 	})
 })
 

pkg/objectupdate/sched/sched.go

@@ -22,6 +22,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 
 	"github.com/k8stopologyawareschedwg/deployer/pkg/flagcodec"
@@ -102,6 +103,28 @@ func DeploymentTLSSettings(dp *appsv1.Deployment, tlsSettings objtls.Settings) e
 	return nil
 }
 
+func DeploymentTopologySpreadConstraints(dp *appsv1.Deployment) error {
+	labels := dp.Spec.Template.Labels
+	if len(labels) == 0 {
+		return fmt.Errorf("no labels found in deployment template")
+	}
+
+	schedConstr := corev1.TopologySpreadConstraint{
+		LabelSelector: &metav1.LabelSelector{
+			MatchLabels: labels,
+		},
+		MaxSkew:           1,
+		TopologyKey:       "kubernetes.io/hostname",
+		WhenUnsatisfiable: corev1.DoNotSchedule,
+		// this is needed for safe rollouts to allow ignoring the old replicaset and calculates the spread
+		// purely based on the new replicaset, ensuring the final state is perfectly balanced without stalling.
+		MatchLabelKeys: []string{"pod-template-hash"},
+	}
+
+	dp.Spec.Template.Spec.TopologySpreadConstraints = []corev1.TopologySpreadConstraint{schedConstr}
+	klog.V(3).InfoS("scheduler deployment topology spread constraints", "constraints", schedConstr.String())
+	return nil
+}
 func SchedulerConfig(cm *corev1.ConfigMap, name string, params *k8swgmanifests.ConfigParams) error {
 	if cm.Data == nil {
 		return fmt.Errorf("no data found in ConfigMap: %s/%s", cm.Namespace, cm.Name)

pkg/objectupdate/sched/sched_test.go

@@ -24,6 +24,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -631,6 +633,43 @@ func TestDeploymentTLSSettingsRepeated(t *testing.T) {
 	}
 }
 
+func TestDeploymentTopologySpreadConstraintsNoLabels(t *testing.T) {
+	dp := dpMinimal.DeepCopy()
+	dp.Spec.Template.Labels = nil
+	if err := DeploymentTopologySpreadConstraints(dp); err == nil {
+		t.Fatalf("expected error but got nil")
+	}
+
+	dp.Spec.Template.Labels = map[string]string{}
+	if err := DeploymentTopologySpreadConstraints(dp); err == nil {
+		t.Fatalf("expected error but got nil")
+	}
+}
+
+func TestDeploymentTopologySpreadConstraints(t *testing.T) {
+	dp := dpMinimal.DeepCopy()
+	dp.Spec.Template.Labels = map[string]string{
+		"app": "numaresources-scheduler",
+	}
+	if err := DeploymentTopologySpreadConstraints(dp); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	expectedConstraints := []corev1.TopologySpreadConstraint{
+		{
+			LabelSelector: &metav1.LabelSelector{
+				MatchLabels: dp.Spec.Template.Labels,
+			},
+			MaxSkew:           1,
+			TopologyKey:       "kubernetes.io/hostname",
+			WhenUnsatisfiable: corev1.DoNotSchedule,
+			MatchLabelKeys:    []string{"pod-template-hash"},
+		},
+	}
+	if cmp.Diff(dp.Spec.Template.Spec.TopologySpreadConstraints, expectedConstraints) != "" {
+		t.Errorf("constraints mismatch\ngot:      %v\nexpected: %v", dp.Spec.Template.Spec.TopologySpreadConstraints, expectedConstraints)
+	}
+}
+
 func mustParseResource(t *testing.T, v string) resource.Quantity {
 	t.Helper()
 	qty, err := resource.ParseQuantity(v)