Skip to content

Commit

Permalink
Fix Pod NodeSelector changes from OpenTelemetryCollector (#2941)
Browse files Browse the repository at this point in the history
* Add node-selector test case

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

* Change to use merge WithOverwriteWithEmptyValue

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

* Add changelog

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

* Add changelog

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

* Change to merge-override-empty only the nodeSelector

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

* Invert order of nodeSelector override

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>

---------

Signed-off-by: Janario Oliveira <janario.oliveira@gmail.com>
  • Loading branch information
janario committed May 10, 2024
1 parent edae5b4 commit 904b7ec
Show file tree
Hide file tree
Showing 11 changed files with 321 additions and 1 deletion.
17 changes: 17 additions & 0 deletions .chloggen/fix-collector-node-selector.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: collector

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix to reflect changes of OpenTelemetryCollector.spec.nodeSelector in the collector Pods

# One or more tracking issues related to the change
issues: [2940]

# (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: |
When updating `OpenTelemetryCollector.spec.nodeSelector` it was not removing previous selector from the final collector pod (Deployment/Daemonset/Statefulset).
14 changes: 13 additions & 1 deletion internal/manifests/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ func mergeWithOverride(dst, src interface{}) error {
return mergo.Merge(dst, src, mergo.WithOverride)
}

func mergeWithOverwriteWithEmptyValue(dst, src interface{}) error {
return mergo.Merge(dst, src, mergo.WithOverwriteWithEmptyValue)
}

func mutateSecret(existing, desired *corev1.Secret) {
existing.Labels = desired.Labels
existing.Annotations = desired.Annotations
Expand Down Expand Up @@ -270,10 +274,12 @@ func mutateDaemonset(existing, desired *appsv1.DaemonSet) error {
if existing.CreationTimestamp.IsZero() {
existing.Spec.Selector = desired.Spec.Selector
}

if err := mergeWithOverride(&existing.Spec, desired.Spec); err != nil {
return err
}
if err := mergeWithOverwriteWithEmptyValue(&existing.Spec.Template.Spec.NodeSelector, desired.Spec.Template.Spec.NodeSelector); err != nil {
return err
}
return nil
}

Expand All @@ -290,6 +296,9 @@ func mutateDeployment(existing, desired *appsv1.Deployment) error {
if err := mergeWithOverride(&existing.Spec.Template, desired.Spec.Template); err != nil {
return err
}
if err := mergeWithOverwriteWithEmptyValue(&existing.Spec.Template.Spec.NodeSelector, desired.Spec.Template.Spec.NodeSelector); err != nil {
return err
}
if err := mergeWithOverride(&existing.Spec.Strategy, desired.Spec.Strategy); err != nil {
return err
}
Expand All @@ -316,6 +325,9 @@ func mutateStatefulSet(existing, desired *appsv1.StatefulSet) error {
if err := mergeWithOverride(&existing.Spec.Template, desired.Spec.Template); err != nil {
return err
}
if err := mergeWithOverwriteWithEmptyValue(&existing.Spec.Template.Spec.NodeSelector, desired.Spec.Template.Spec.NodeSelector); err != nil {
return err
}
return nil
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: daemonset-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: without
spec:
(nodeSelector == null): true
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: deployment-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: without
spec:
(nodeSelector == null): true
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: statefulset-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: without
spec:
(nodeSelector == null): true
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: deployment
labels:
node-selector-mode: without
spec:
mode: deployment
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: daemonset
labels:
node-selector-mode: without
spec:
mode: daemonset
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: statefulset
labels:
node-selector-mode: without
spec:
mode: statefulset
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: daemonset-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: with
spec:
nodeSelector:
kubernetes.io/os: linux
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: deployment-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: with
spec:
nodeSelector:
kubernetes.io/os: linux
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: v1
kind: Pod
metadata:
labels:
app.kubernetes.io/component: opentelemetry-collector
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/name: statefulset-collector
app.kubernetes.io/part-of: opentelemetry
node-selector-mode: with
spec:
nodeSelector:
kubernetes.io/os: linux
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: deployment
labels:
node-selector-mode: with
spec:
mode: deployment
nodeSelector:
kubernetes.io/os: linux
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: daemonset
labels:
node-selector-mode: with
spec:
mode: daemonset
nodeSelector:
kubernetes.io/os: linux
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
---
apiVersion: opentelemetry.io/v1alpha1
kind: OpenTelemetryCollector
metadata:
name: statefulset
labels:
node-selector-mode: with
spec:
mode: statefulset
nodeSelector:
kubernetes.io/os: linux
config: |
receivers:
otlp:
protocols:
grpc:
http:
processors:
exporters:
debug:
service:
pipelines:
traces:
receivers: [otlp]
processors: []
exporters: [debug]
58 changes: 58 additions & 0 deletions tests/e2e/node-selector-collector/chainsaw-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
creationTimestamp: null
name: node-selector-collector
spec:
steps:
- name: step-00
description: collectors without nodeSelector
try:
- apply:
file: 00-install-collectors-without-node-selector.yaml
# deployment
- assert:
file: 00-assert-deployment-without-node-selector.yaml
# daemonset
- assert:
file: 00-assert-daemonset-without-node-selector.yaml
# statefulset
- assert:
file: 00-assert-statefulset-without-node-selector.yaml

- name: step-01
description: collectors with nodeSelector
try:
- apply:
file: 01-install-collectors-with-node-selector.yaml
# deployment
- assert:
file: 01-assert-deployment-with-node-selector.yaml
# daemonset
- assert:
file: 01-assert-daemonset-with-node-selector.yaml
# statefulset
- assert:
file: 01-assert-statefulset-with-node-selector.yaml

- name: step-02
description: back to no nodeSelector
try:
- command: # while update is not supported on 0.1.7, but 0.1.9 https://kyverno.github.io/chainsaw/0.1.9/operations/update/
entrypoint: kubectl
args:
- -n
- ${NAMESPACE}
- replace
- -f
- 00-install-collectors-without-node-selector.yaml
# deployment
- assert:
file: 00-assert-deployment-without-node-selector.yaml
# daemonset
- assert:
file: 00-assert-daemonset-without-node-selector.yaml
# statefulset
- assert:
file: 00-assert-statefulset-without-node-selector.yaml

0 comments on commit 904b7ec

Please sign in to comment.