Merge pull request #1341 from simonpasquier/bz1933847

Bug 1933847: enable hard affinity + PodDisruptionBudget for Prometheus and Thanos Ruler pods

Author: openshift-merge-robot

assets/prometheus-k8s/pod-disruption-budget.yaml

@@ -0,0 +1,18 @@
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  labels:
+    app.kubernetes.io/component: prometheus
+    app.kubernetes.io/name: prometheus
+    app.kubernetes.io/part-of: openshift-monitoring
+    app.kubernetes.io/version: 2.30.3
+  name: prometheus-k8s
+  namespace: openshift-monitoring
+spec:
+  minAvailable: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/component: prometheus
+      app.kubernetes.io/name: prometheus
+      app.kubernetes.io/part-of: openshift-monitoring
+      prometheus: k8s

assets/prometheus-k8s/prometheus.yaml

@@ -12,18 +12,16 @@ metadata:
 spec:
   affinity:
     podAntiAffinity:
-      preferredDuringSchedulingIgnoredDuringExecution:
-      - podAffinityTerm:
-          labelSelector:
-            matchLabels:
-              app.kubernetes.io/component: prometheus
-              app.kubernetes.io/name: prometheus
-              app.kubernetes.io/part-of: openshift-monitoring
-              prometheus: k8s
-          namespaces:
-          - openshift-monitoring
-          topologyKey: kubernetes.io/hostname
-        weight: 100
+      requiredDuringSchedulingIgnoredDuringExecution:
+      - labelSelector:
+          matchLabels:
+            app.kubernetes.io/component: prometheus
+            app.kubernetes.io/name: prometheus
+            app.kubernetes.io/part-of: openshift-monitoring
+            prometheus: k8s
+        namespaces:
+        - openshift-monitoring
+        topologyKey: kubernetes.io/hostname
   alerting:
     alertmanagers:
     - apiVersion: v2

assets/prometheus-user-workload/pod-disruption-budget.yaml

@@ -0,0 +1,18 @@
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  labels:
+    app.kubernetes.io/component: prometheus
+    app.kubernetes.io/name: prometheus
+    app.kubernetes.io/part-of: openshift-monitoring
+    app.kubernetes.io/version: 2.30.3
+  name: prometheus-user-workload
+  namespace: openshift-user-workload-monitoring
+spec:
+  minAvailable: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/component: prometheus
+      app.kubernetes.io/name: prometheus
+      app.kubernetes.io/part-of: openshift-monitoring
+      prometheus: user-workload

assets/prometheus-user-workload/prometheus.yaml

@@ -12,18 +12,16 @@ metadata:
 spec:
   affinity:
     podAntiAffinity:
-      preferredDuringSchedulingIgnoredDuringExecution:
-      - podAffinityTerm:
-          labelSelector:
-            matchLabels:
-              app.kubernetes.io/component: prometheus
-              app.kubernetes.io/name: prometheus
-              app.kubernetes.io/part-of: openshift-monitoring
-              prometheus: user-workload
-          namespaces:
-          - openshift-user-workload-monitoring
-          topologyKey: kubernetes.io/hostname
-        weight: 100
+      requiredDuringSchedulingIgnoredDuringExecution:
+      - labelSelector:
+          matchLabels:
+            app.kubernetes.io/component: prometheus
+            app.kubernetes.io/name: prometheus
+            app.kubernetes.io/part-of: openshift-monitoring
+            prometheus: user-workload
+        namespaces:
+        - openshift-user-workload-monitoring
+        topologyKey: kubernetes.io/hostname
   alerting:
     alertmanagers:
     - apiVersion: v2

assets/thanos-ruler/pod-disruption-budget.yaml

@@ -0,0 +1,13 @@
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  labels:
+    thanosRulerName: user-workload
+  name: thanos-ruler-user-workload
+  namespace: openshift-user-workload-monitoring
+spec:
+  minAvailable: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: thanos-ruler
+      thanos-ruler: user-workload

assets/thanos-ruler/thanos-ruler.yaml

@@ -6,6 +6,16 @@ metadata:
   name: user-workload
   namespace: openshift-user-workload-monitoring
 spec:
+  affinity:
+    podAntiAffinity:
+      requiredDuringSchedulingIgnoredDuringExecution:
+      - labelSelector:
+          matchLabels:
+            app.kubernetes.io/name: thanos-ruler
+            thanos-ruler: user-workload
+        namespaces:
+        - openshift-user-workload-monitoring
+        topologyKey: kubernetes.io/hostname
   alertmanagersConfig:
     key: alertmanagers.yaml
     name: thanos-ruler-alertmanagers-config

jsonnet/components/prometheus-user-workload.libsonnet

@@ -365,7 +365,9 @@ function(params)
         ],
       },
     },
-    // Removing PDB since it doesn't allow cluster upgrade when hard pod anti affinity is not set https://github.com/openshift/cluster-monitoring-operator/pull/1198
-    // Review hard anti-affinity changes and then we can add back PDB
-    podDisruptionBudget:: {},
+
+    // TODO: remove podDisruptionBudget once https://github.com/prometheus-operator/kube-prometheus/pull/1156 is merged
+    podDisruptionBudget+: {
+      apiVersion: 'policy/v1',
+    },
   }

jsonnet/components/prometheus.libsonnet

@@ -496,7 +496,9 @@ function(params)
         ],
       },
     },
-    // Removing PDB since it doesn't allow cluster upgrade when hard pod anti affinity is not set https://github.com/openshift/cluster-monitoring-operator/pull/1198
-    // Review hard anti-affinity changes and then we can add back PDB
-    podDisruptionBudget:: {},
+
+    // TODO: remove podDisruptionBudget once https://github.com/prometheus-operator/kube-prometheus/pull/1156 is merged
+    podDisruptionBudget+: {
+      apiVersion: 'policy/v1',
+    },
   }

jsonnet/components/thanos-ruler.libsonnet

@@ -315,6 +315,17 @@ function(params) {
       },
     },
     spec: {
+      affinity: {
+        podAntiAffinity: {
+          requiredDuringSchedulingIgnoredDuringExecution: [{
+            labelSelector: {
+              matchLabels: cfg.selectorLabels,
+            },
+            namespaces: [cfg.namespace],
+            topologyKey: 'kubernetes.io/hostname',
+          }],
+        },
+      },
       securityContext: {
         fsGroup: 65534,
         runAsNonRoot: true,
@@ -459,6 +470,24 @@ function(params) {
     },
   },
 
+  podDisruptionBudget: {
+    apiVersion: 'policy/v1',
+    kind: 'PodDisruptionBudget',
+    metadata: {
+      name: 'thanos-ruler-' + cfg.name,
+      namespace: cfg.namespace,
+      labels: {
+        thanosRulerName: cfg.name,
+      },
+    },
+    spec: {
+      minAvailable: 1,
+      selector: {
+        matchLabels: cfg.selectorLabels,
+      },
+    },
+  },
+
   // statefulSet from kube-thanos is not needed because thanosruler custom resource
   // is used instead.
   //statefulSet:: {},

jsonnet/utils/anti-affinity.libsonnet

@@ -3,7 +3,7 @@ local addon = import 'github.com/prometheus-operator/kube-prometheus/jsonnet/kub
 addon {
   values+:: {
     prometheus+: {
-      podAntiAffinity: 'soft',
+      podAntiAffinity: 'hard',
     },
     prometheusAdapter+: {
       podAntiAffinity: 'hard',

pkg/manifests/manifests.go

@@ -121,6 +121,7 @@ var (
 	PrometheusK8sTrustedCABundle             = "prometheus-k8s/trusted-ca-bundle.yaml"
 	PrometheusK8sThanosSidecarServiceMonitor = "prometheus-k8s/service-monitor-thanos-sidecar.yaml"
 	PrometheusK8sTAlertmanagerRoleBinding    = "prometheus-k8s/alertmanager-role-binding.yaml"
+	PrometheusK8sPodDisruptionBudget         = "prometheus-k8s/pod-disruption-budget.yaml"
 
 	PrometheusUserWorkloadServingCertsCABundle        = "prometheus-user-workload/serving-certs-ca-bundle.yaml"
 	PrometheusUserWorkloadServiceAccount              = "prometheus-user-workload/service-account.yaml"
@@ -137,6 +138,7 @@ var (
 	PrometheusUserWorkloadGrpcTLSSecret               = "prometheus-user-workload/grpc-tls-secret.yaml"
 	PrometheusUserWorkloadThanosSidecarServiceMonitor = "prometheus-user-workload/service-monitor-thanos-sidecar.yaml"
 	PrometheusUserWorkloadAlertmanagerRoleBinding     = "prometheus-user-workload/alertmanager-role-binding.yaml"
+	PrometheusUserWorkloadPodDisruptionBudget         = "prometheus-user-workload/pod-disruption-budget.yaml"
 
 	PrometheusAdapterAPIService                         = "prometheus-adapter/api-service.yaml"
 	PrometheusAdapterClusterRole                        = "prometheus-adapter/cluster-role.yaml"
@@ -246,6 +248,7 @@ var (
 	ThanosRulerServiceMonitor               = "thanos-ruler/service-monitor.yaml"
 	ThanosRulerPrometheusRule               = "thanos-ruler/thanos-ruler-prometheus-rule.yaml"
 	ThanosRulerAlertmanagerRoleBinding      = "thanos-ruler/alertmanager-role-binding.yaml"
+	ThanosRulerPodDisruptionBudget          = "thanos-ruler/pod-disruption-budget.yaml"
 
 	TelemeterTrustedCABundle = "telemeter-client/trusted-ca-bundle.yaml"
 
@@ -2307,6 +2310,18 @@ func (f *Factory) PrometheusK8sServiceThanosSidecar() (*v1.Service, error) {
 	return s, nil
 }
 
+func (f *Factory) PrometheusK8sPodDisruptionBudget() (*policyv1.PodDisruptionBudget, error) {
+	return f.NewPodDisruptionBudget(f.assets.MustNewAssetReader(PrometheusK8sPodDisruptionBudget))
+}
+
+func (f *Factory) PrometheusUserWorkloadPodDisruptionBudget() (*policyv1.PodDisruptionBudget, error) {
+	return f.NewPodDisruptionBudget(f.assets.MustNewAssetReader(PrometheusUserWorkloadPodDisruptionBudget))
+}
+
+func (f *Factory) ThanosRulerPodDisruptionBudget() (*policyv1.PodDisruptionBudget, error) {
+	return f.NewPodDisruptionBudget(f.assets.MustNewAssetReader(ThanosRulerPodDisruptionBudget))
+}
+
 func (f *Factory) PrometheusUserWorkloadService() (*v1.Service, error) {
 	s, err := f.NewService(f.assets.MustNewAssetReader(PrometheusUserWorkloadService))
 	if err != nil {

pkg/manifests/manifests_test.go

@@ -2570,6 +2570,20 @@ func TestPodDisruptionBudget(t *testing.T) {
 		getPDB func(f *Factory) (*policyv1.PodDisruptionBudget, error)
 		ha     bool
 	}{
+		{
+			name: "PrometheusK8s HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.PrometheusK8sPodDisruptionBudget()
+			},
+			ha: true,
+		},
+		{
+			name: "PrometheusK8s non-HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.PrometheusK8sPodDisruptionBudget()
+			},
+			ha: false,
+		},
 		{
 			name: "PrometheusAdapter HA",
 			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
@@ -2598,6 +2612,34 @@ func TestPodDisruptionBudget(t *testing.T) {
 			},
 			ha: false,
 		},
+		{
+			name: "PrometheusUWM HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.PrometheusUserWorkloadPodDisruptionBudget()
+			},
+			ha: true,
+		},
+		{
+			name: "PrometheusUWM non-HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.PrometheusUserWorkloadPodDisruptionBudget()
+			},
+			ha: false,
+		},
+		{
+			name: "ThanosRuler HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.ThanosRulerPodDisruptionBudget()
+			},
+			ha: true,
+		},
+		{
+			name: "ThanosRuler non-HA",
+			getPDB: func(f *Factory) (*policyv1.PodDisruptionBudget, error) {
+				return f.ThanosRulerPodDisruptionBudget()
+			},
+			ha: false,
+		},
 	}
 
 	for _, tc := range tests {

pkg/tasks/prometheus.go

@@ -309,6 +309,20 @@ func (t *PrometheusTask) Run(ctx context.Context) error {
 		return errors.Wrap(err, "error creating Prometheus Client GRPC TLS secret")
 	}
 
+	{
+		pdb, err := t.factory.PrometheusK8sPodDisruptionBudget()
+		if err != nil {
+			return errors.Wrap(err, "initializing Prometheus PodDisruptionBudget object failed")
+		}
+
+		if pdb != nil {
+			err = t.client.CreateOrUpdatePodDisruptionBudget(ctx, pdb)
+			if err != nil {
+				return errors.Wrap(err, "reconciling Prometheus PodDisruptionBudget object failed")
+			}
+		}
+	}
+
 	{
 		// Create trusted CA bundle ConfigMap.
 		trustedCA, err := t.factory.PrometheusK8sTrustedCABundle()

pkg/tasks/prometheus_user_workload.go

@@ -208,6 +208,18 @@ func (t *PrometheusUserWorkloadTask) create(ctx context.Context) error {
 		return errors.Wrap(err, "reconciling UserWorkload Prometheus additionalAlertmanagerConfigs secret failed")
 	}
 
+	pdb, err := t.factory.PrometheusUserWorkloadPodDisruptionBudget()
+	if err != nil {
+		return errors.Wrap(err, "initializing UserWorkload Prometheus PodDisruptionBudget object failed")
+	}
+
+	if pdb != nil {
+		err = t.client.CreateOrUpdatePodDisruptionBudget(ctx, pdb)
+		if err != nil {
+			return errors.Wrap(err, "reconciling UserWorkload Prometheus PodDisruptionBudget object failed")
+		}
+	}
+
 	klog.V(4).Info("initializing UserWorkload Prometheus object")
 	p, err := t.factory.PrometheusUserWorkload(s)
 	if err != nil {
@@ -291,6 +303,18 @@ func (t *PrometheusUserWorkloadTask) destroy(ctx context.Context) error {
 		"server.key", string(grpcTLS.Data["prometheus-server.key"]),
 	)
 
+	pdb, err := t.factory.PrometheusUserWorkloadPodDisruptionBudget()
+	if err != nil {
+		return errors.Wrap(err, "initializing UserWorkload Prometheus PodDisruptionBudget object failed")
+	}
+
+	if pdb != nil {
+		err = t.client.DeletePodDisruptionBudget(ctx, pdb)
+		if err != nil {
+			return errors.Wrap(err, "deleting UserWorkload Prometheus PodDisruptionBudget object failed")
+		}
+	}
+
 	p, err := t.factory.PrometheusUserWorkload(s)
 	if err != nil {
 		return errors.Wrap(err, "initializing UserWorkload Prometheus object failed")