Add default TopologySpreadContraints to distributor (#9383)

Signed-off-by: Joao Marcal <jmarcal@redhat.com>

operator/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Main
 
+- [9383](https://github.com/grafana/loki/pull/9383) **JoaoBraveCoding**: Add default TopologySpreadContaints to Distributor
 - [9406](https://github.com/grafana/loki/pull/9406) **aminesnow**: Add label selector to zone awareness TopologySpreadConstraints
 - [9366](https://github.com/grafana/loki/pull/9366) **periklis**: Add support for custom tenant topology in rules
 - [9315](https://github.com/grafana/loki/pull/9315) **aminesnow**: Add zone awareness spec to LokiStack

operator/internal/manifests/distributor.go

@@ -59,7 +59,8 @@ func NewDistributorDeployment(opts Options) *appsv1.Deployment {
 	l := ComponentLabels(LabelDistributorComponent, opts.Name)
 	a := commonAnnotations(opts.ConfigSHA1, opts.CertRotationRequiredAt)
 	podSpec := corev1.PodSpec{
-		Affinity: configureAffinity(l, opts.Gates.DefaultNodeAffinity),
+		Affinity:                  configureAffinity(l, opts.Gates.DefaultNodeAffinity),
+		TopologySpreadConstraints: defaultTopologySpreadConstraints(l),
 		Volumes: []corev1.Volume{
 			{
 				Name: configVolumeName,
@@ -128,7 +129,7 @@ func NewDistributorDeployment(opts Options) *appsv1.Deployment {
 	}
 
 	if opts.Stack.Replication != nil {
-		podSpec.TopologySpreadConstraints = topologySpreadConstraints(*opts.Stack.Replication, LabelDistributorComponent, opts.Name)
+		podSpec.TopologySpreadConstraints = append(podSpec.TopologySpreadConstraints, topologySpreadConstraints(*opts.Stack.Replication, LabelDistributorComponent, opts.Name)...)
 	}
 
 	return &appsv1.Deployment{

operator/internal/manifests/distributor_test.go

@@ -101,16 +101,30 @@ func TestBuildDistributor_PodDisruptionBudget(t *testing.T) {
 	require.EqualValues(t, manifests.ComponentLabels(manifests.LabelDistributorComponent, opts.Name), pdb.Spec.Selector.MatchLabels)
 }
 
-func TestNewDistributorDeployment_TopologySpreadConstraints(t *testing.T) {
+func TestNewDistributoDeployment_TopologySpreadConstraints(t *testing.T) {
-	depl := manifests.NewDistributorDeployment(manifests.Options{
+	for _, tc := range []struct {
-		Name:      "abcd",
+		Name                            string
-		Namespace: "efgh",
+		Replication                     *lokiv1.ReplicationSpec
-		Stack: lokiv1.LokiStackSpec{
+		ExpectedTopologySpreadContraint []corev1.TopologySpreadConstraint
-			Template: &lokiv1.LokiTemplateSpec{
+	}{
-				Distributor: &lokiv1.LokiComponentSpec{
+		{
-					Replicas: 1,
+			Name: "default",
+			ExpectedTopologySpreadContraint: []corev1.TopologySpreadConstraint{
+				{
+					MaxSkew:     1,
+					TopologyKey: "kubernetes.io/hostname",
+					LabelSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/component": "distributor",
+							"app.kubernetes.io/instance":  "abcd",
+						},
+					},
+					WhenUnsatisfiable: corev1.ScheduleAnyway,
 				},
 			},
+		},
+		{
+			Name: "replication_defined",
 			Replication: &lokiv1.ReplicationSpec{
 				Zones: []lokiv1.ZoneSpec{
 					{
@@ -124,31 +138,58 @@ func TestNewDistributorDeployment_TopologySpreadConstraints(t *testing.T) {
 				},
 				Factor: 1,
 			},
-		},
+			ExpectedTopologySpreadContraint: []corev1.TopologySpreadConstraint{
-	})
+				{
-
+					MaxSkew:     1,
-	require.Equal(t, []corev1.TopologySpreadConstraint{
+					TopologyKey: "kubernetes.io/hostname",
-		{
+					LabelSelector: &metav1.LabelSelector{
-			MaxSkew:           3,
+						MatchLabels: map[string]string{
-			TopologyKey:       "zone",
+							"app.kubernetes.io/component": "distributor",
-			WhenUnsatisfiable: "DoNotSchedule",
+							"app.kubernetes.io/instance":  "abcd",
-			LabelSelector: &metav1.LabelSelector{
+						},
-				MatchLabels: map[string]string{
+					},
-					"app.kubernetes.io/component": "distributor",
+					WhenUnsatisfiable: corev1.ScheduleAnyway,
-					"app.kubernetes.io/instance":  "abcd",
 				},
-			},
+				{
-		},
+					MaxSkew:           3,
-		{
+					TopologyKey:       "zone",
-			MaxSkew:           2,
+					WhenUnsatisfiable: "DoNotSchedule",
-			TopologyKey:       "region",
+					LabelSelector: &metav1.LabelSelector{
-			WhenUnsatisfiable: "DoNotSchedule",
+						MatchLabels: map[string]string{
-			LabelSelector: &metav1.LabelSelector{
+							"app.kubernetes.io/component": "distributor",
-				MatchLabels: map[string]string{
+							"app.kubernetes.io/instance":  "abcd",
-					"app.kubernetes.io/component": "distributor",
+						},
-					"app.kubernetes.io/instance":  "abcd",
+					},
+				},
+				{
+					MaxSkew:           2,
+					TopologyKey:       "region",
+					WhenUnsatisfiable: "DoNotSchedule",
+					LabelSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/component": "distributor",
+							"app.kubernetes.io/instance":  "abcd",
+						},
+					},
 				},
 			},
 		},
-	}, depl.Spec.Template.Spec.TopologySpreadConstraints)
+	} {
+		t.Run(tc.Name, func(t *testing.T) {
+			depl := manifests.NewDistributorDeployment(manifests.Options{
+				Name:      "abcd",
+				Namespace: "efgh",
+				Stack: lokiv1.LokiStackSpec{
+					Template: &lokiv1.LokiTemplateSpec{
+						Distributor: &lokiv1.LokiComponentSpec{
+							Replicas: 1,
+						},
+					},
+					Replication: tc.Replication,
+				},
+			})
+
+			require.Equal(t, tc.ExpectedTopologySpreadContraint, depl.Spec.Template.Spec.TopologySpreadConstraints)
+		})
+	}
 }

operator/internal/manifests/var.go

@@ -105,7 +105,7 @@ const (
 	kubernetesNodeOSLabel       = "kubernetes.io/os"
 	kubernetesNodeOSLinux       = "linux"
 	kubernetesNodeHostnameLabel = "kubernetes.io/hostname"
-	kubernetesCompomentLabel    = "app.kubernetes.io/component"
+	kubernetesComponentLabel    = "app.kubernetes.io/component"
 	kubernetesInstanceLabel     = "app.kubernetes.io/instance"
 )
 
@@ -135,6 +135,13 @@ func commonLabels(stackName string) map[string]string {
 	}
 }
 
+func componentInstaceLabels(component string, stackName string) map[string]string {
+	return map[string]string{
+		kubernetesInstanceLabel:  stackName,
+		kubernetesComponentLabel: component,
+	}
+}
+
 func serviceAnnotations(serviceName string, enableSigningService bool) map[string]string {
 	annotations := map[string]string{}
 	if enableSigningService {
@@ -154,7 +161,7 @@ func topologySpreadConstraints(spec lokiv1.ReplicationSpec, component string, st
 				WhenUnsatisfiable: corev1.DoNotSchedule,
 				LabelSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{
-						kubernetesCompomentLabel: component,
+						kubernetesComponentLabel: component,
 						kubernetesInstanceLabel:  stackName,
 					},
 				},
@@ -168,7 +175,7 @@ func topologySpreadConstraints(spec lokiv1.ReplicationSpec, component string, st
 // ComponentLabels is a list of all commonLabels including the app.kubernetes.io/component:<component> label
 func ComponentLabels(component, stackName string) labels.Set {
 	return labels.Merge(commonLabels(stackName), map[string]string{
-		kubernetesCompomentLabel: component,
+		kubernetesComponentLabel: component,
 	})
 }
 
@@ -488,6 +495,19 @@ func gatewayServiceMonitorEndpoint(gatewayName, portName, serviceName, namespace
 	}
 }
 
+// defaultTopologySpreadConstraints returns a topology spread contraint that will
+// instruct the scheduler to try and schedule pods from the same component in different nodes
+func defaultTopologySpreadConstraints(labels labels.Set) []corev1.TopologySpreadConstraint {
+	return []corev1.TopologySpreadConstraint{{
+		MaxSkew:     1,
+		TopologyKey: kubernetesNodeHostnameLabel,
+		LabelSelector: &metav1.LabelSelector{
+			MatchLabels: componentInstaceLabels(labels[kubernetesComponentLabel], labels[kubernetesInstanceLabel]),
+		},
+		WhenUnsatisfiable: corev1.ScheduleAnyway,
+	}}
+}
+
 // configureAffinity returns an Affinity struture that can be used directly
 // in a Deployment/StatefulSet. Parameters will affected configuration of the
 // different fields in Affinity (NodeAffinity, PodAffinity, PodAntiAffinity).
@@ -534,9 +554,7 @@ func defaultPodAntiAffinity(labels labels.Set) *corev1.PodAntiAffinity {
 	// This code assumes that this function will never be called with a set of labels
 	// that don't have the "component" and "instance" labels since we enforce those on
 	// all the components of the LokiStack
-	componentLabel := labels[kubernetesCompomentLabel]
+	componentLabel := labels[kubernetesComponentLabel]
-	stackName := labels[kubernetesInstanceLabel]
-
 	_, enablePodAntiAffinity := podAntiAffinityComponents[componentLabel]
 	if !enablePodAntiAffinity {
 		return nil
@@ -548,10 +566,7 @@ func defaultPodAntiAffinity(labels labels.Set) *corev1.PodAntiAffinity {
 				Weight: 100,
 				PodAffinityTerm: corev1.PodAffinityTerm{
 					LabelSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
+						MatchLabels: componentInstaceLabels(labels[kubernetesComponentLabel], labels[kubernetesInstanceLabel]),
-							"app.kubernetes.io/component": componentLabel,
-							"app.kubernetes.io/instance":  stackName,
-						},
 					},
 					TopologyKey: kubernetesNodeHostnameLabel,
 				},