feat: Add support for PDBs on deployment and statefulset

Signed-off-by: Jorge Turrado <[email protected]>
open-telemetry · Sep 28, 2023 · c9ec369 · c9ec369
1 parent 37cf662
commit c9ec369
Show file tree

Hide file tree

Showing 22 changed files with 668 additions and 2 deletions.
diff --git a/.chloggen/add-pdb-support.yaml b/.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`.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -29,6 +29,7 @@ jobs:
          - e2e-upgrade
          - e2e-prometheuscr
          - e2e-autoscale
+         - e2e-pdb
 
     steps:
     - name: Set up Go

diff --git a/Makefile b/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:

diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/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.
 	//
@@ -444,6 +450,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.

diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go
diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml
@@ -31,7 +31,7 @@ metadata:
     categories: Logging & Tracing,Monitoring
     certified: "false"
     containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
-    createdAt: "2023-09-20T15:05:15Z"
+    createdAt: "2023-09-21T11:33:59Z"
     description: Provides the OpenTelemetry components, including the Collector
     operators.operatorframework.io/builder: operator-sdk-v1.29.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
@@ -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:

diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.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,

diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/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,

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -135,6 +135,18 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - policy
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - route.openshift.io
   resources:

diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go
@@ -26,6 +26,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
+	policyV1 "k8s.io/api/policy/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -208,6 +209,7 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
 // +kubebuilder:rbac:groups=apps,resources=daemonsets;deployments;statefulsets,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update
 // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete
@@ -304,6 +306,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er
 	}
 
 	builder = builder.Owns(&autoscalingv2.HorizontalPodAutoscaler{})
+	builder = builder.Owns(&policyV1.PodDisruptionBudget{})
 
 	return builder.Complete(r)
 }
diff --git a/controllers/reconcile_test.go b/controllers/reconcile_test.go
@@ -26,6 +26,7 @@ import (
 	autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2"
 	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
+	policyV1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/record"
@@ -318,6 +319,45 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "policy v1 deployment collector",
+			args: args{
+				params:  paramsWithPolicy(1, 0),
+				updates: []manifests.Params{paramsWithPolicy(0, 1)},
+			},
+			want: []want{
+				{
+					result: controllerruntime.Result{},
+					checks: []check{
+						func(t *testing.T, appliedInstance v1alpha1.OpenTelemetryCollector) {
+							actual := policyV1.PodDisruptionBudget{}
+							exists, pdbErr := populateObjectIfExists(t, &actual, namespacedObjectName(appliedInstance, naming.HorizontalPodAutoscaler))
+							assert.NoError(t, pdbErr)
+							assert.Equal(t, int32(1), actual.Spec.MinAvailable.IntVal)
+							assert.Nil(t, actual.Spec.MaxUnavailable)
+							assert.True(t, exists)
+						},
+					},
+					wantErr:     assert.NoError,
+					validateErr: assert.NoError,
+				},
+				{
+					result: controllerruntime.Result{},
+					checks: []check{
+						func(t *testing.T, appliedInstance v1alpha1.OpenTelemetryCollector) {
+							actual := policyV1.PodDisruptionBudget{}
+							exists, pdbErr := populateObjectIfExists(t, &actual, namespacedObjectName(appliedInstance, naming.HorizontalPodAutoscaler))
+							assert.NoError(t, pdbErr)
+							assert.Nil(t, actual.Spec.MinAvailable)
+							assert.Equal(t, int32(1), actual.Spec.MaxUnavailable.IntVal)
+							assert.True(t, exists)
+						},
+					},
+					wantErr:     assert.NoError,
+					validateErr: assert.NoError,
+				},
+			},
+		},
 		{
 			name: "daemonset collector",
 			args: args{

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -323,6 +323,68 @@ func paramsWithHPA(minReps, maxReps int32) manifests.Params {
 	}
 }
 
+func paramsWithPolicy(minAvailable, maxUnavailable int32) manifests.Params {
+	configYAML, err := os.ReadFile("testdata/test.yaml")
+	if err != nil {
+		fmt.Printf("Error getting yaml file: %v", err)
+	}
+
+	configuration := config.New(config.WithAutoDetect(mockAutoDetector), config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage))
+	err = configuration.AutoDetect()
+	if err != nil {
+		logger.Error(err, "configuration.autodetect failed")
+	}
+
+	pdb := &v1alpha1.PodDisruptionBudgetSpec{}
+
+	if maxUnavailable > 0 && minAvailable > 0 {
+		fmt.Printf("worng configuration: %v", fmt.Errorf("minAvailable and maxUnavailable cannot be both set"))
+	}
+	if maxUnavailable > 0 {
+		pdb.MaxUnavailable = &intstr.IntOrString{
+			Type:   intstr.Int,
+			IntVal: maxUnavailable,
+		}
+	} else {
+		pdb.MinAvailable = &intstr.IntOrString{
+			Type:   intstr.Int,
+			IntVal: minAvailable,
+		}
+	}
+
+	return manifests.Params{
+		Config: configuration,
+		Client: k8sClient,
+		Instance: v1alpha1.OpenTelemetryCollector{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "opentelemetry.io",
+				APIVersion: "v1",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "policytest",
+				Namespace: "default",
+				UID:       instanceUID,
+			},
+			Spec: v1alpha1.OpenTelemetryCollectorSpec{
+				Ports: []v1.ServicePort{{
+					Name: "web",
+					Port: 80,
+					TargetPort: intstr.IntOrString{
+						Type:   intstr.Int,
+						IntVal: 80,
+					},
+					NodePort: 0,
+				}},
+				Config:              string(configYAML),
+				PodDisruptionBudget: pdb,
+			},
+		},
+		Scheme:   testScheme,
+		Log:      logger,
+		Recorder: record.NewFakeRecorder(10),
+	}
+}
+
 func populateObjectIfExists(t testing.TB, object client.Object, namespacedName types.NamespacedName) (bool, error) {
 	t.Helper()
 	err := k8sClient.Get(context.Background(), namespacedName, object)