Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set update strategy in crd for deployment #2513

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .chloggen/set-rollout-strat-deployment.yaml
Original file line number Diff line number Diff line change
@@ -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:
5 changes: 5 additions & 0 deletions apis/v1alpha1/collector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
16 changes: 16 additions & 0 deletions apis/v1alpha1/collector_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions apis/v1alpha2/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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-<configmap-name>`.
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"`
Comment on lines +156 to +165
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional that we're also adding the DaemonSet setting to v1alpha2? Or is it currently missing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's missing by error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it was missing. Note, i renamed it though with a daemonset prefix

}

// OpenTelemetryCollectorStatus defines the observed state of OpenTelemetryCollector.
Expand Down
2 changes: 2 additions & 0 deletions apis/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 75 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b><a href="#opentelemetrycollectorspecdeploymentupdatestrategy">deploymentUpdateStrategy</a></b></td>
<td>object</td>
<td>
UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods https://kubernetes.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b><a href="#opentelemetrycollectorspecenvindex">env</a></b></td>
<td>[]object</td>
Expand Down Expand Up @@ -14212,6 +14219,74 @@ target specifies the target value for the given metric
</table>


### OpenTelemetryCollector.spec.deploymentUpdateStrategy
<sup><sup>[↩ Parent](#opentelemetrycollectorspec)</sup></sup>



UpdateStrategy represents the strategy the operator will take replacing existing Deployment pods with new pods https://kubernetes.

<table>
<thead>
<tr>
<th>Name</th>
<th>Type</th>
<th>Description</th>
<th>Required</th>
</tr>
</thead>
<tbody><tr>
<td><b><a href="#opentelemetrycollectorspecdeploymentupdatestrategyrollingupdate">rollingUpdate</a></b></td>
<td>object</td>
<td>
Rolling update config params. Present only if DeploymentStrategyType = RollingUpdate. --- TODO: Update this to follow our convention for oneOf, whatever we decide it to be.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>type</b></td>
<td>string</td>
<td>
Type of deployment. Can be "Recreate" or "RollingUpdate". Default is RollingUpdate.<br/>
</td>
<td>false</td>
</tr></tbody>
</table>


### OpenTelemetryCollector.spec.deploymentUpdateStrategy.rollingUpdate
<sup><sup>[↩ Parent](#opentelemetrycollectorspecdeploymentupdatestrategy)</sup></sup>



Rolling update config params. Present only if DeploymentStrategyType = RollingUpdate. --- TODO: Update this to follow our convention for oneOf, whatever we decide it to be.

<table>
<thead>
<tr>
<th>Name</th>
<th>Type</th>
<th>Description</th>
<th>Required</th>
</tr>
</thead>
<tbody><tr>
<td><b>maxSurge</b></td>
<td>int or string</td>
<td>
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%).<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>maxUnavailable</b></td>
<td>int or string</td>
<td>
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%).<br/>
</td>
<td>false</td>
</tr></tbody>
</table>


### OpenTelemetryCollector.spec.env[index]
<sup><sup>[↩ Parent](#opentelemetrycollectorspec)</sup></sup>

Expand Down
1 change: 1 addition & 0 deletions internal/manifests/collector/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 33 additions & 0 deletions internal/manifests/collector/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down
Loading