From 72fadb07b569a432e11d3395b57a367a2e12b2f6 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Mon, 15 Jan 2024 10:46:59 -0500 Subject: [PATCH] Set update strategy in crd for deployment (#2513) --- .chloggen/set-rollout-strat-deployment.yaml | 16 ++++ apis/v1alpha1/collector_webhook.go | 5 ++ apis/v1alpha1/collector_webhook_test.go | 16 ++++ apis/v1alpha1/opentelemetrycollector_types.go | 5 ++ apis/v1alpha1/zz_generated.deepcopy.go | 1 + apis/v1alpha2/opentelemetrycollector_types.go | 11 +++ apis/v1alpha2/zz_generated.deepcopy.go | 2 + ...ntelemetry.io_opentelemetrycollectors.yaml | 31 ++++++++ ...ntelemetry.io_opentelemetrycollectors.yaml | 31 ++++++++ docs/api.md | 75 +++++++++++++++++++ internal/manifests/collector/deployment.go | 1 + .../manifests/collector/deployment_test.go | 33 ++++++++ 12 files changed, 227 insertions(+) create mode 100755 .chloggen/set-rollout-strat-deployment.yaml diff --git a/.chloggen/set-rollout-strat-deployment.yaml b/.chloggen/set-rollout-strat-deployment.yaml new file mode 100755 index 0000000000..2e5b6622f2 --- /dev/null +++ b/.chloggen/set-rollout-strat-deployment.yaml @@ -0,0 +1,16 @@ +# 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: Adds deployment rollout strategy to CRD fields + +# One or more tracking issues related to the change +issues: [2512] + +# (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: diff --git a/apis/v1alpha1/collector_webhook.go b/apis/v1alpha1/collector_webhook.go index 48aeb34f6e..96e9e8472a 100644 --- a/apis/v1alpha1/collector_webhook.go +++ b/apis/v1alpha1/collector_webhook.go @@ -357,6 +357,11 @@ func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollecto return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'updateStrategy'", r.Spec.Mode) } + // validate updateStrategy for Deployment + if r.Spec.Mode != ModeDeployment && len(r.Spec.DeploymentUpdateStrategy.Type) > 0 { + return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'deploymentUpdateStrategy'", r.Spec.Mode) + } + return warnings, nil } diff --git a/apis/v1alpha1/collector_webhook_test.go b/apis/v1alpha1/collector_webhook_test.go index ea67ac5886..c527409ded 100644 --- a/apis/v1alpha1/collector_webhook_test.go +++ b/apis/v1alpha1/collector_webhook_test.go @@ -1041,6 +1041,22 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Collector mode is set to deployment, which does not support the attribute 'updateStrategy'", }, + { + name: "invalid updateStrategy for Statefulset mode", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + Mode: ModeStatefulSet, + DeploymentUpdateStrategy: appsv1.DeploymentStrategy{ + Type: "RollingUpdate", + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + }, + }, + }, + }, + expectedErr: "the OpenTelemetry Collector mode is set to statefulset, which does not support the attribute 'deploymentUpdateStrategy'", + }, } for _, test := range tests { diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index db894716e6..130d0ad9b5 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -281,6 +281,11 @@ type OpenTelemetryCollectorSpec struct { // This is only applicable to Daemonset mode. // +optional UpdateStrategy appsv1.DaemonSetUpdateStrategy `json:"updateStrategy,omitempty"` + // UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods + // https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/deployment-v1/#DeploymentSpec + // This is only applicable to Deployment mode. + // +optional + DeploymentUpdateStrategy appsv1.DeploymentStrategy `json:"deploymentUpdateStrategy,omitempty"` } // OpenTelemetryTargetAllocator defines the configurations for the Prometheus target allocator. diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 62be78954c..586cd8ab34 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -919,6 +919,7 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp copy(*out, *in) } in.UpdateStrategy.DeepCopyInto(&out.UpdateStrategy) + in.DeploymentUpdateStrategy.DeepCopyInto(&out.DeploymentUpdateStrategy) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenTelemetryCollectorSpec. diff --git a/apis/v1alpha2/opentelemetrycollector_types.go b/apis/v1alpha2/opentelemetrycollector_types.go index 335935cbef..74d844da1b 100644 --- a/apis/v1alpha2/opentelemetrycollector_types.go +++ b/apis/v1alpha2/opentelemetrycollector_types.go @@ -17,6 +17,7 @@ package v1alpha2 import ( + appsv1 "k8s.io/api/apps/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -152,6 +153,16 @@ type OpenTelemetryCollectorSpec struct { // object, which shall be mounted into the Collector Pods. // Each ConfigMap will be added to the Collector's Deployments as a volume named `configmap-`. ConfigMaps []v1alpha1.ConfigMapsSpec `json:"configmaps,omitempty"` + // UpdateStrategy represents the strategy the operator will take replacing existing DaemonSet pods with new pods + // https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/daemon-set-v1/#DaemonSetSpec + // This is only applicable to Daemonset mode. + // +optional + DaemonSetUpdateStrategy appsv1.DaemonSetUpdateStrategy `json:"daemonSetUpdateStrategy,omitempty"` + // UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods + // https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/deployment-v1/#DeploymentSpec + // This is only applicable to Deployment mode. + // +optional + DeploymentUpdateStrategy appsv1.DeploymentStrategy `json:"deploymentUpdateStrategy,omitempty"` } // OpenTelemetryCollectorStatus defines the observed state of OpenTelemetryCollector. diff --git a/apis/v1alpha2/zz_generated.deepcopy.go b/apis/v1alpha2/zz_generated.deepcopy.go index 6cfb468826..2ac3b048fe 100644 --- a/apis/v1alpha2/zz_generated.deepcopy.go +++ b/apis/v1alpha2/zz_generated.deepcopy.go @@ -560,6 +560,8 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp *out = make([]v1alpha1.ConfigMapsSpec, len(*in)) copy(*out, *in) } + in.DaemonSetUpdateStrategy.DeepCopyInto(&out.DaemonSetUpdateStrategy) + in.DeploymentUpdateStrategy.DeepCopyInto(&out.DeploymentUpdateStrategy) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenTelemetryCollectorSpec. diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 0e5983e97f..ac0098401b 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -2221,6 +2221,37 @@ spec: - name type: object type: array + deploymentUpdateStrategy: + description: UpdateStrategy represents the strategy the operator will + take replacing existing Deployment pods with new pods https://kubernetes. + properties: + rollingUpdate: + description: 'Rolling update config params. Present only if DeploymentStrategyType + = RollingUpdate. --- TODO: Update this to follow our convention + for oneOf, whatever we decide it to be.' + properties: + maxSurge: + anyOf: + - type: integer + - type: string + description: 'The maximum number of pods that can be scheduled + above the desired number of pods. Value can be an absolute + number (ex: 5) or a percentage of desired pods (ex: 10%).' + x-kubernetes-int-or-string: true + maxUnavailable: + anyOf: + - type: integer + - type: string + description: 'The maximum number of pods that can be unavailable + during the update. Value can be an absolute number (ex: + 5) or a percentage of desired pods (ex: 10%).' + x-kubernetes-int-or-string: true + type: object + type: + description: Type of deployment. Can be "Recreate" or "RollingUpdate". + Default is RollingUpdate. + type: string + type: object env: description: ENV vars to set on the OpenTelemetry Collector's Pods. These can then in certain cases be consumed in the config file for diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index cbb808876a..c505cb08b7 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -2218,6 +2218,37 @@ spec: - name type: object type: array + deploymentUpdateStrategy: + description: UpdateStrategy represents the strategy the operator will + take replacing existing Deployment pods with new pods https://kubernetes. + properties: + rollingUpdate: + description: 'Rolling update config params. Present only if DeploymentStrategyType + = RollingUpdate. --- TODO: Update this to follow our convention + for oneOf, whatever we decide it to be.' + properties: + maxSurge: + anyOf: + - type: integer + - type: string + description: 'The maximum number of pods that can be scheduled + above the desired number of pods. Value can be an absolute + number (ex: 5) or a percentage of desired pods (ex: 10%).' + x-kubernetes-int-or-string: true + maxUnavailable: + anyOf: + - type: integer + - type: string + description: 'The maximum number of pods that can be unavailable + during the update. Value can be an absolute number (ex: + 5) or a percentage of desired pods (ex: 10%).' + x-kubernetes-int-or-string: true + type: object + type: + description: Type of deployment. Can be "Recreate" or "RollingUpdate". + Default is RollingUpdate. + type: string + type: object env: description: ENV vars to set on the OpenTelemetry Collector's Pods. These can then in certain cases be consumed in the config file for diff --git a/docs/api.md b/docs/api.md index f8a9d8c1f9..af7f09965a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -9642,6 +9642,13 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. ConfigMaps is a list of ConfigMaps in the same namespace as the OpenTelemetryCollector object, which shall be mounted into the Collector Pods.
false + + deploymentUpdateStrategy + object + + UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods https://kubernetes.
+ + false env []object @@ -14212,6 +14219,74 @@ target specifies the target value for the given metric +### OpenTelemetryCollector.spec.deploymentUpdateStrategy +[↩ Parent](#opentelemetrycollectorspec) + + + +UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods https://kubernetes. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
rollingUpdateobject + Rolling update config params. Present only if DeploymentStrategyType = RollingUpdate. --- TODO: Update this to follow our convention for oneOf, whatever we decide it to be.
+ 		false
typestring + Type of deployment. Can be "Recreate" or "RollingUpdate". Default is RollingUpdate.
+ 		false
+ + +### OpenTelemetryCollector.spec.deploymentUpdateStrategy.rollingUpdate +[↩ Parent](#opentelemetrycollectorspecdeploymentupdatestrategy) + + + +Rolling update config params. Present only if DeploymentStrategyType = RollingUpdate. --- TODO: Update this to follow our convention for oneOf, whatever we decide it to be. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
maxSurgeint or string + The maximum number of pods that can be scheduled above the desired number of pods. Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%).
+ 		false
maxUnavailableint or string + The maximum number of pods that can be unavailable during the update. Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%).
+ 		false
+ + ### OpenTelemetryCollector.spec.env[index] [↩ Parent](#opentelemetrycollectorspec) diff --git a/internal/manifests/collector/deployment.go b/internal/manifests/collector/deployment.go index 3a1b0abf21..899cc765b2 100644 --- a/internal/manifests/collector/deployment.go +++ b/internal/manifests/collector/deployment.go @@ -44,6 +44,7 @@ func Deployment(params manifests.Params) *appsv1.Deployment { Selector: &metav1.LabelSelector{ MatchLabels: manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector), }, + Strategy: params.OtelCol.Spec.DeploymentUpdateStrategy, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, diff --git a/internal/manifests/collector/deployment_test.go b/internal/manifests/collector/deployment_test.go index baa66f42ad..f6004998ee 100644 --- a/internal/manifests/collector/deployment_test.go +++ b/internal/manifests/collector/deployment_test.go @@ -18,8 +18,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" @@ -203,6 +205,37 @@ func TestDeploymenttPodSecurityContext(t *testing.T) { assert.Equal(t, &runasGroup, d.Spec.Template.Spec.SecurityContext.RunAsGroup) } +func TestDeploymentUpdateStrategy(t *testing.T) { + otelcol := v1alpha1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-instance", + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + DeploymentUpdateStrategy: appsv1.DeploymentStrategy{ + Type: "RollingUpdate", + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)}, + }, + }, + }, + } + + cfg := config.New() + + params := manifests.Params{ + Config: cfg, + OtelCol: otelcol, + Log: logger, + } + + d := Deployment(params) + + assert.Equal(t, "RollingUpdate", string(d.Spec.Strategy.Type)) + assert.Equal(t, 1, d.Spec.Strategy.RollingUpdate.MaxSurge.IntValue()) + assert.Equal(t, 1, d.Spec.Strategy.RollingUpdate.MaxUnavailable.IntValue()) +} + func TestDeploymentHostNetwork(t *testing.T) { // Test default otelcol1 := v1alpha1.OpenTelemetryCollector{