Repository: rancher/fleet

Issue:
Fleet Controller Helm Charts Values for Anti-/Affinity
When running the  upstream controllers with multiple replicas, users want to make sure the replicas run on different nodes.

https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity

This could be helm values.


Discussion:
@kkaempf: @manno with #4145 merged, I'd assume this should be in `QA review` or `done` ?! 🤔 @sbulage: ## System Version
| Rancher Version | Fleet Version |
|--------|--------|
| v2.13.0-alpha2 | 108.0.0+up0.14.0-alpha.3 | 

## Steps performed

- Below `values.yaml` file used to update `affinity` rules to `Fleet`.

    <details><summary>values.yaml</summary>
    
    ```yaml
    affinity:
      nodeAffinity:
        requiredDuringSchedulingIgnoredDuringExecution:
          nodeSelectorTerms:
            - matchExpressions:
                - key: disktype
                  operator: In
                  values:
                    - ssd
    ```
    
    </details> 

- Upgraded Fleet by using above `values.yaml`.
- Wait for a sometime for Fleet deployments to adopt new affinity values.
- Add label to the `Nodes` (on which Fleet's pod will be created with matched expression).
- Fleet Controller, GitJob and HelmOps pod will be created on the Node with matched expression label.

- For more detailed steps, see below video.

  <details><summary>Fleet Affinity Video</summary>
  
  https://github.com/user-attachments/assets/0810c1e4-0848-450f-9d9a-e148acc68313
  </details> 
- For `PodAffinity`, we have to update the Fleet-controller deployment's pod section with label.
- After that pods will be scheduled for creation.

_**Note: Please see video in 1.5x speed**_

Solution patch:
diff --git a/README.md b/README.md
index 8855414a..371385f6 100644
--- a/README.md
+++ b/README.md
@@ -37,9 +37,9 @@ Install the Fleet Helm charts (there's two because we separate out CRDs for ulti
 
 ```shell
 helm -n cattle-fleet-system install --create-namespace --wait \
-    fleet-crd https://github.com/rancher/fleet/releases/download/v0.13.1/fleet-crd-0.13.1.tgz
+    fleet-crd https://github.com/rancher/fleet/releases/download/v0.13.2/fleet-crd-0.13.2.tgz
 helm -n cattle-fleet-system install --create-namespace --wait \
-    fleet https://github.com/rancher/fleet/releases/download/v0.13.1/fleet-0.13.1.tgz
+    fleet https://github.com/rancher/fleet/releases/download/v0.13.2/fleet-0.13.2.tgz
 ```
 
 ## Add a Git Repo to watch
diff --git a/charts/fleet-agent/Chart.yaml b/charts/fleet-agent/Chart.yaml
index 215427c4..a84826c6 100644
--- a/charts/fleet-agent/Chart.yaml
+++ b/charts/fleet-agent/Chart.yaml
@@ -1,11 +1,11 @@
 annotations:
   catalog.cattle.io/certified: rancher
   catalog.cattle.io/hidden: "true"
-  catalog.cattle.io/kube-version: '>= 1.28.0-0 < 1.34.0-0'
+  catalog.cattle.io/kube-version: '>= 1.32.0-0 < 1.35.0-0'
   catalog.cattle.io/namespace: cattle-fleet-system
   catalog.cattle.io/os: linux
   catalog.cattle.io/permits-os: linux,windows
-  catalog.cattle.io/rancher-version: '>= 2.12.0-0 < 2.13.0-0'
+  catalog.cattle.io/rancher-version: '>= 2.13.0-0 < 2.14.0-0'
   catalog.cattle.io/release-name: fleet-agent
 apiVersion: v2
 appVersion: 0.0.0
diff --git a/charts/fleet/Chart.yaml b/charts/fleet/Chart.yaml
index 71f31f92..ad77e6f9 100644
--- a/charts/fleet/Chart.yaml
+++ b/charts/fleet/Chart.yaml
@@ -3,12 +3,12 @@ annotations:
   catalog.cattle.io/certified: rancher
   catalog.cattle.io/experimental: "true"
   catalog.cattle.io/hidden: "true"
-  catalog.cattle.io/kube-version: '>= 1.28.0-0 < 1.34.0-0'
+  catalog.cattle.io/kube-version: '>= 1.32.0-0 < 1.35.0-0'
   catalog.cattle.io/namespace: cattle-fleet-system
   catalog.cattle.io/os: linux
   catalog.cattle.io/permits-os: linux,windows
   catalog.cattle.io/provides-gvr: clusters.fleet.cattle.io/v1alpha1
-  catalog.cattle.io/rancher-version: '>= 2.12.0-0 < 2.13.0-0'
+  catalog.cattle.io/rancher-version: '>= 2.13.0-0 < 2.14.0-0'
   catalog.cattle.io/release-name: fleet
 apiVersion: v2
 appVersion: 0.0.0
diff --git a/charts/fleet/templates/deployment.yaml b/charts/fleet/templates/deployment.yaml
index bdcdf376..35918e3e 100644
--- a/charts/fleet/templates/deployment.yaml
+++ b/charts/fleet/templates/deployment.yaml
@@ -251,6 +251,10 @@ spec:
 {{- if $.Values.tolerations }}
 {{ toYaml $.Values.tolerations | indent 8 }}
 {{- end }}
+      {{- with $.Values.affinity }}
+      affinity:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
       {{- if $.Values.priorityClassName }}
       priorityClassName: "{{$.Values.priorityClassName}}"
       {{- end }}
diff --git a/charts/fleet/templates/deployment_gitjob.yaml b/charts/fleet/templates/deployment_gitjob.yaml
index b73d3148..2b90a45e 100644
--- a/charts/fleet/templates/deployment_gitjob.yaml
+++ b/charts/fleet/templates/deployment_gitjob.yaml
@@ -134,6 +134,10 @@ spec:
 {{- if $.Values.tolerations }}
 {{ toYaml $.Values.tolerations | indent 8 }}
 {{- end }}
+      {{- with $.Values.affinity }}
+      affinity:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
       {{- if $.Values.priorityClassName }}
       priorityClassName: "{{$.Values.priorityClassName}}"
       {{- end }}
diff --git a/charts/fleet/templates/deployment_helmops.yaml b/charts/fleet/templates/deployment_helmops.yaml
index 998653f1..1b4dc45d 100644
--- a/charts/fleet/templates/deployment_helmops.yaml
+++ b/charts/fleet/templates/deployment_helmops.yaml
@@ -121,6 +121,10 @@ spec:
 {{- if $.Values.tolerations }}
 {{ toYaml $.Values.tolerations | indent 8 }}
 {{- end }}
+      {{- with $.Values.affinity }}
+      affinity:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
       {{- if $.Values.priorityClassName }}
       priorityClassName: "{{$.Values.priorityClassName}}"
       {{- end }}
diff --git a/charts/fleet/values.yaml b/charts/fleet/values.yaml
index ed704e40..c1efaf44 100644
--- a/charts/fleet/values.yaml
+++ b/charts/fleet/values.yaml
@@ -69,9 +69,13 @@ global:
 ## Ref: https://kubernetes.io/docs/user-guide/node-selection/
 ##
 nodeSelector: {}
+
 ## List of node taints to tolerate (requires Kubernetes >= 1.6)
 tolerations: []
 
+## Pod affinity for the controllers.
+affinity: {}
+
 ## PriorityClassName assigned to deployment.
 priorityClassName: ""
 
diff --git a/internal/cmd/controller/agentmanagement/scheduling/scheduling.go b/internal/cmd/controller/agentmanagement/scheduling/scheduling.go
index 846cda96..d85cfeb4 100644
--- a/internal/cmd/controller/agentmanagement/scheduling/scheduling.go
+++ b/internal/cmd/controller/agentmanagement/scheduling/scheduling.go
@@ -29,25 +29,25 @@ func PriorityClass(priorityClass *fleet.PriorityClassSpec) *schedulingv1.Priorit
 	}
 }
 
-func PodDisruptionBudget(agentNamespace string, podDisruptionBudgetSpec *fleet.PodDisruptionBudgetSpec) (*policyv1.PodDisruptionBudget, error) {
+func PodDisruptionBudget(agentNamespace string, pdbs *fleet.PodDisruptionBudgetSpec) (*policyv1.PodDisruptionBudget, error) {
 	pdbSpec := policyv1.PodDisruptionBudgetSpec{
 		Selector: &metav1.LabelSelector{
 			MatchLabels: map[string]string{"app": "fleet-agent"},
 		},
 	}
 
-	if podDisruptionBudgetSpec.MaxUnavailable == "" && podDisruptionBudgetSpec.MinAvailable == "" {
+	if pdbs.MaxUnavailable == "" && pdbs.MinAvailable == "" {
 		logrus.Warnf("Neither MaxUnavailable nor MinAvailable is set, defaulting to 0 for MaxUnavailable")
 		pdbSpec.MaxUnavailable = &intstr.IntOrString{IntVal: 0}
-	} else if podDisruptionBudgetSpec.MaxUnavailable != "" && podDisruptionBudgetSpec.MinAvailable != "" {
-		return &policyv1.PodDisruptionBudget{},
-			fmt.Errorf("both MaxUnavailable (%s) and MinAvailable (%s) are set, not creating PDB", podDisruptionBudgetSpec.MaxUnavailable, podDisruptionBudgetSpec.MinAvailable)
-	} else if podDisruptionBudgetSpec.MaxUnavailable != "" {
-		mu := intstr.Parse(podDisruptionBudgetSpec.MaxUnavailable)
+	} else if pdbs.MaxUnavailable != "" && (pdbs.MinAvailable == "" || pdbs.MinAvailable == "0") {
+		mu := intstr.Parse(pdbs.MaxUnavailable)
 		pdbSpec.MaxUnavailable = &mu
-	} else if podDisruptionBudgetSpec.MinAvailable != "" {
-		mu := intstr.Parse(podDisruptionBudgetSpec.MinAvailable)
-		pdbSpec.MinAvailable = &mu
+	} else if pdbs.MinAvailable != "" && (pdbs.MaxUnavailable == "" || pdbs.MaxUnavailable == "0") {
+		ma := intstr.Parse(pdbs.MinAvailable)
+		pdbSpec.MinAvailable = &ma
+	} else if pdbs.MaxUnavailable != "" && pdbs.MinAvailable != "" {
+		return &policyv1.PodDisruptionBudget{},
+			fmt.Errorf("both MaxUnavailable (%s) and MinAvailable (%s) are set, not creating PDB", pdbs.MaxUnavailable, pdbs.MinAvailable)
 	}
 
 	return &policyv1.PodDisruptionBudget{



Test patch:
diff --git a/charts/fleet/tests/affinity_test.yaml b/charts/fleet/tests/affinity_test.yaml
new file mode 100644
index 00000000..c1cb2da9
--- /dev/null
+++ b/charts/fleet/tests/affinity_test.yaml
@@ -0,0 +1,52 @@
+suite: test affinity rules
+templates:
+  - deployment.yaml
+  - deployment_gitjob.yaml
+  - deployment_helmops.yaml
+tests:
+  - it: should set affinity rules
+    set:
+      affinity:
+        nodeAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            nodeSelectorTerms:
+            - matchExpressions:
+                - key: disktype
+                  operator: In
+                  values:
+                    - ssd
+        podAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            - labelSelector:
+                matchExpressions:
+                  - key: security
+                    operator: In
+                    values:
+                      - S1
+              topologyKey: "kubernetes.io/hostname"
+        podAntiAffinity:
+          requiredDuringSchedulingIgnoredDuringExecution:
+            - labelSelector:
+                matchExpressions:
+                  - key: app
+                    operator: In
+                    values:
+                      - fleet
+              topologyKey: "kubernetes.io/hostname"
+    asserts:
+      - equal:
+          path: spec.template.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key
+          value: disktype
+      - equal:
+          path: spec.template.spec.affinity.podAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].labelSelector.matchExpressions[0].key
+          value: security
+      - equal:
+          path: spec.template.spec.affinity.podAntiAffinity.requiredDuringSchedulingIgnoredDuringExecution[0].labelSelector.matchExpressions[0].key
+          value: app
+
+  - it: should work without affinity rules
+    set:
+      affinity: {}
+    asserts:
+      - notExists:
+          path: spec.template.spec.affinity
diff --git a/internal/cmd/controller/agentmanagement/scheduling/scheduling_test.go b/internal/cmd/controller/agentmanagement/scheduling/scheduling_test.go
index c4607150..5fe1814f 100644
--- a/internal/cmd/controller/agentmanagement/scheduling/scheduling_test.go
+++ b/internal/cmd/controller/agentmanagement/scheduling/scheduling_test.go
@@ -50,6 +50,31 @@ func TestPodDisruptionBudget(t *testing.T) {
 			spec:    &fleet.PodDisruptionBudgetSpec{MinAvailable: "1", MaxUnavailable: "1"},
 			wantErr: true,
 		},
+		{
+			name:   "having both values, MinAvailable as zero, should disable the zero value",
+			spec:   &fleet.PodDisruptionBudgetSpec{MinAvailable: "0", MaxUnavailable: "1"},
+			wantMU: ptr(intstr.FromInt(1)),
+		},
+		{
+			name:   "having both values, MaxUnavailable as zero, should disable the zero value",
+			spec:   &fleet.PodDisruptionBudgetSpec{MinAvailable: "1", MaxUnavailable: "0"},
+			wantMA: ptr(intstr.FromInt(1)),
+		},
+		{
+			name:   "having a percent value while MinAvailable is zero should disable the zero value",
+			spec:   &fleet.PodDisruptionBudgetSpec{MinAvailable: "0", MaxUnavailable: "10%"},
+			wantMU: ptr(intstr.FromString("10%")),
+		},
+		{
+			name:   "having a percent value while MaxUnavailable is zero should disable the zero value",
+			spec:   &fleet.PodDisruptionBudgetSpec{MinAvailable: "10%", MaxUnavailable: "0"},
+			wantMA: ptr(intstr.FromString("10%")),
+		},
+		{
+			name:    "two non-zero values shouldn't work",
+			spec:    &fleet.PodDisruptionBudgetSpec{MinAvailable: "1", MaxUnavailable: "1"},
+			wantErr: true,
+		},
 	}
 
 	for _, test := range tests {