feat: Add support for PDBs on deployment and statefulset (#2141)

Author: JorTurFer

.chloggen/add-pdb-support.yaml

@@ -0,0 +1,19 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Add PDB support for OpenTelemetryCollector"
+
+# One or more tracking issues related to the change
+issues:
+  - 2136
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  This PR adds support for PodDisruptionBudgets when OpenTelemetryCollector is deployed
+  as `deployment` or `statefulset`.

.github/workflows/e2e.yaml

@@ -30,6 +30,7 @@ jobs:
          - e2e-prometheuscr
          - e2e-autoscale
          - e2e-multi-instrumentation
+         - e2e-pdb
 
     steps:
     - name: Set up Go

Makefile

@@ -197,6 +197,11 @@ e2e-upgrade: undeploy
 e2e-autoscale:
 	$(KUTTL) test --config kuttl-test-autoscale.yaml
 
+# end-to-end-test for testing pdb support
+.PHONY: e2e-pdb
+e2e-pdb:
+	$(KUTTL) test --config kuttl-test-pdb.yaml
+
 # end-to-end-test for testing OpenShift cases
 .PHONY: e2e-openshift
 e2e-openshift:

apis/v1alpha1/opentelemetrycollector_types.go

@@ -19,6 +19,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 // ManagementStateType defines the type for CR management states.
@@ -123,6 +124,11 @@ type OpenTelemetryCollectorSpec struct {
 	//
 	// +optional
 	Autoscaler *AutoscalerSpec `json:"autoscaler,omitempty"`
+	// PodDisruptionBudget specifies the pod disruption budget configuration to use
+	// for the OpenTelemetryCollector workload.
+	//
+	// +optional
+	PodDisruptionBudget *PodDisruptionBudgetSpec `json:"podDisruptionBudget,omitempty"`
 	// SecurityContext configures the container security context for
 	// the opentelemetry-collector container.
 	//
@@ -448,6 +454,23 @@ type AutoscalerSpec struct {
 	TargetMemoryUtilization *int32 `json:"targetMemoryUtilization,omitempty"`
 }
 
+// PodDisruptionBudgetSpec defines the OpenTelemetryCollector's pod disruption budget specification.
+type PodDisruptionBudgetSpec struct {
+	// An eviction is allowed if at least "minAvailable" pods selected by
+	// "selector" will still be available after the eviction, i.e. even in the
+	// absence of the evicted pod.  So for example you can prevent all voluntary
+	// evictions by specifying "100%".
+	// +optional
+	MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`
+
+	// An eviction is allowed if at most "maxUnavailable" pods selected by
+	// "selector" are unavailable after the eviction, i.e. even in absence of
+	// the evicted pod. For example, one can prevent all voluntary evictions
+	// by specifying 0. This is a mutually exclusive setting with "minAvailable".
+	// +optional
+	MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
+}
+
 // MetricsConfigSpec defines a metrics config.
 type MetricsConfigSpec struct {
 	// EnableMetrics specifies if ServiceMonitor should be created for the OpenTelemetry Collector and Prometheus Exporters.

apis/v1alpha1/opentelemetrycollector_webhook.go

@@ -19,6 +19,7 @@ import (
 
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/validation"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -91,6 +92,20 @@ func (r *OpenTelemetryCollector) Default() {
 			r.Spec.Autoscaler.TargetCPUUtilization = &defaultCPUTarget
 		}
 	}
+
+	// if pod isn't provided, we set MaxUnavailable 1,
+	// which will work even if there is just one replica,
+	// not blocking node drains but preventing out-of-the-box
+	// from disruption generated by them with replicas > 1
+	if r.Spec.PodDisruptionBudget == nil {
+		r.Spec.PodDisruptionBudget = &PodDisruptionBudgetSpec{
+			MaxUnavailable: &intstr.IntOrString{
+				Type:   intstr.Int,
+				IntVal: 1,
+			},
+		}
+	}
+
 	if r.Spec.Ingress.Type == IngressTypeRoute && r.Spec.Ingress.Route.Termination == "" {
 		r.Spec.Ingress.Route.Termination = TLSRouteTerminationTypeEdge
 	}
@@ -231,6 +246,14 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
 		}
 	}
 
+	// validate pod disruption budget
+
+	if r.Spec.PodDisruptionBudget != nil {
+		if r.Spec.PodDisruptionBudget.MaxUnavailable != nil && r.Spec.PodDisruptionBudget.MinAvailable != nil {
+			return fmt.Errorf("the OpenTelemetry Spec podDisruptionBudget configuration is incorrect, minAvailable and maxUnavailable are mutually exclusive")
+		}
+	}
+
 	if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Mode == ModeSidecar {
 		return fmt.Errorf("the OpenTelemetry Spec Ingress configuiration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s",
 			ModeDeployment, ModeDaemonSet, ModeStatefulSet,

apis/v1alpha1/opentelemetrycollector_webhook_test.go

@@ -23,6 +23,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 func TestOTELColDefaultingWebhook(t *testing.T) {
@@ -48,6 +49,12 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
 				},
 			},
 		},
@@ -70,6 +77,12 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeSidecar,
 					Replicas:        &five,
 					UpgradeStrategy: "adhoc",
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
 				},
 			},
 		},
@@ -98,6 +111,12 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 						MaxReplicas:          &five,
 						MinReplicas:          &one,
 					},
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
 				},
 			},
 		},
@@ -126,6 +145,12 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 						MinReplicas: &one,
 					},
 					MaxReplicas: &five,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
 				},
 			},
 		},
@@ -155,6 +180,44 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					},
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Defined PDB",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode: ModeDeployment,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MinAvailable: &intstr.IntOrString{
+							Type:   intstr.String,
+							StrVal: "10%",
+						},
+					},
+				},
+			},
+			expected: OpenTelemetryCollector{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"app.kubernetes.io/managed-by": "opentelemetry-operator",
+					},
+				},
+				Spec: OpenTelemetryCollectorSpec{
+					Mode:            ModeDeployment,
+					Replicas:        &one,
+					UpgradeStrategy: UpgradeStrategyAutomatic,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MinAvailable: &intstr.IntOrString{
+							Type:   intstr.String,
+							StrVal: "10%",
+						},
+					},
 				},
 			},
 		},
@@ -237,6 +300,12 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 						},
 						TargetCPUUtilization: &five,
 					},
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MinAvailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
 				},
 			},
 		},
@@ -489,6 +558,25 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 			},
 			expectedErr: "the OpenTelemetry Spec autoscale configuration is incorrect, invalid pods target type",
 		},
+		{
+			name: "pdb minAvailable and maxUnavailable have been set together",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MinAvailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
+				},
+			},
+			expectedErr: "the OpenTelemetry Spec podDisruptionBudget configuration is incorrect, minAvailable and maxUnavailable are mutually exclusive",
+		},
 		{
 			name: "invalid deployment mode incompabible with ingress settings",
 			otelcol: OpenTelemetryCollector{

apis/v1alpha1/zz_generated.deepcopy.go

@@ -237,6 +237,18 @@ spec:
           - get
           - patch
           - update
+        - apiGroups:
+          - policy
+          resources:
+          - poddisruptionbudgets
+          verbs:
+          - create
+          - delete
+          - get
+          - list
+          - patch
+          - update
+          - watch
         - apiGroups:
           - route.openshift.io
           resources:

bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml

@@ -3719,6 +3719,27 @@ spec:
                 description: PodAnnotations is the set of annotations that will be
                   attached to Collector and Target Allocator pods.
                 type: object
+              podDisruptionBudget:
+                description: PodDisruptionBudget specifies the pod disruption budget
+                  configuration to use for the OpenTelemetryCollector workload.
+                properties:
+                  maxUnavailable:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    description: An eviction is allowed if at most "maxUnavailable"
+                      pods selected by "selector" are unavailable after the eviction,
+                      i.e. even in absence of the evicted pod.
+                    x-kubernetes-int-or-string: true
+                  minAvailable:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    description: An eviction is allowed if at least "minAvailable"
+                      pods selected by "selector" will still be available after the
+                      eviction, i.e. even in the absence of the evicted pod.
+                    x-kubernetes-int-or-string: true
+                type: object
               podSecurityContext:
                 description: PodSecurityContext configures the pod security context
                   for the opentelemetry-collector pod, when running as a deployment,

bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml

@@ -3716,6 +3716,27 @@ spec:
                 description: PodAnnotations is the set of annotations that will be
                   attached to Collector and Target Allocator pods.
                 type: object
+              podDisruptionBudget:
+                description: PodDisruptionBudget specifies the pod disruption budget
+                  configuration to use for the OpenTelemetryCollector workload.
+                properties:
+                  maxUnavailable:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    description: An eviction is allowed if at most "maxUnavailable"
+                      pods selected by "selector" are unavailable after the eviction,
+                      i.e. even in absence of the evicted pod.
+                    x-kubernetes-int-or-string: true
+                  minAvailable:
+                    anyOf:
+                    - type: integer
+                    - type: string
+                    description: An eviction is allowed if at least "minAvailable"
+                      pods selected by "selector" will still be available after the
+                      eviction, i.e. even in the absence of the evicted pod.
+                    x-kubernetes-int-or-string: true
+                type: object
               podSecurityContext:
                 description: PodSecurityContext configures the pod security context
                   for the opentelemetry-collector pod, when running as a deployment,