From 60e8a8e5c99aac9836547b47b7c966e3d8afbdc7 Mon Sep 17 00:00:00 2001 From: Benedikt Bongartz Date: Fri, 29 Mar 2024 23:03:20 +0100 Subject: [PATCH] add featuregate for k8s 1.28 native sidecar container Signed-off-by: Benedikt Bongartz --- .chloggen/native_sidecar.yaml | 16 +++ .github/workflows/e2e.yaml | 3 + Makefile | 7 ++ pkg/featuregate/featuregate.go | 12 +++ pkg/sidecar/pod.go | 30 +++++- pkg/sidecar/pod_test.go | 108 ++++++++++++++++++++ tests/e2e-native-sidecar/00-assert.yaml | 21 ++++ tests/e2e-native-sidecar/00-install.yaml | 41 ++++++++ tests/e2e-native-sidecar/chainsaw-test.yaml | 14 +++ 9 files changed, 251 insertions(+), 1 deletion(-) create mode 100755 .chloggen/native_sidecar.yaml create mode 100644 tests/e2e-native-sidecar/00-assert.yaml create mode 100644 tests/e2e-native-sidecar/00-install.yaml create mode 100755 tests/e2e-native-sidecar/chainsaw-test.yaml diff --git a/.chloggen/native_sidecar.yaml b/.chloggen/native_sidecar.yaml new file mode 100755 index 0000000000..7c07ec66ab --- /dev/null +++ b/.chloggen/native_sidecar.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. collector, target allocator, auto-instrumentation, opamp, github action) +component: pkg/sidecar + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add native sidecar injection behind a feature gate which is disabled by default. + +# One or more tracking issues related to the change +issues: [2376] + +# (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/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 922467d9b0..fe40f48fe0 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -46,6 +46,9 @@ jobs: setup: "add-operator-arg OPERATOR_ARG='--feature-gates=operator.targetallocator.mtls' add-certmanager-permissions prepare-e2e" - group: e2e-automatic-rbac setup: "add-rbac-permissions-to-operator prepare-e2e" + - group: e2e-native-sidecar + setup: "add-operator-arg OPERATOR_ARG='--feature-gates=operator.sidecarcontainers.native' prepare-e2e" + kube-version: "1.31" steps: - name: Check out code into the Go module directory uses: actions/checkout@v4 diff --git a/Makefile b/Makefile index 2e73d990d9..384a7ea1f7 100644 --- a/Makefile +++ b/Makefile @@ -267,6 +267,13 @@ generate: controller-gen e2e: chainsaw $(CHAINSAW) test --test-dir ./tests/e2e +# e2e-native-sidecar +# NOTE: make sure the k8s featuregate "SidecarContainers" is set to true. +# NOTE: make sure the operator featuregate "operator.sidecarcontainers.native" is enabled. +.PHONY: e2e-native-sidecar +e2e-native-sidecar: chainsaw + $(CHAINSAW) test --test-dir ./tests/e2e-native-sidecar + # end-to-end-test for testing automatic RBAC creation .PHONY: e2e-automatic-rbac e2e-automatic-rbac: chainsaw diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index 5b452da235..3637da0e26 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -25,6 +25,18 @@ const ( ) var ( + // EnableNativeSidecarContainers is the feature gate that controls whether a + // sidecar should be injected as a native sidecar or the classic way. + // Native sidecar containers have been available since kubernetes v1.28 in + // alpha and v1.29 in beta. + // It needs to be enabled with +featureGate=SidecarContainers. + // See: + // https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-alpha-or-beta-features + EnableNativeSidecarContainers = featuregate.GlobalRegistry().MustRegister( + "operator.sidecarcontainers.native", + featuregate.StageAlpha, + featuregate.WithRegisterDescription("controls whether the operator supports sidecar containers as init containers"), + ) // PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator. PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister( "operator.observability.prometheus", diff --git a/pkg/sidecar/pod.go b/pkg/sidecar/pod.go index d7db13484c..dd44369e23 100644 --- a/pkg/sidecar/pod.go +++ b/pkg/sidecar/pod.go @@ -25,6 +25,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) const ( @@ -47,7 +48,17 @@ func add(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTelemetryCol container.Env = append(container.Env, attributes...) } pod.Spec.InitContainers = append(pod.Spec.InitContainers, otelcol.Spec.InitContainers...) - pod.Spec.Containers = append(pod.Spec.Containers, container) + + if featuregate.EnableNativeSidecarContainers.IsEnabled() { + policy := corev1.ContainerRestartPolicyAlways + container.RestartPolicy = &policy + // NOTE: Use ReadinessProbe as startup probe. + // See https://github.com/open-telemetry/opentelemetry-operator/pull/2801#discussion_r1547571121 + container.StartupProbe = container.ReadinessProbe + pod.Spec.InitContainers = append(pod.Spec.InitContainers, container) + } else { + pod.Spec.Containers = append(pod.Spec.Containers, container) + } pod.Spec.Volumes = append(pod.Spec.Volumes, otelcol.Spec.Volumes...) if pod.Labels == nil { @@ -71,6 +82,16 @@ func remove(pod corev1.Pod) corev1.Pod { } } pod.Spec.Containers = containers + + // NOTE: we also remove init containers (native sidecars) since k8s 1.28. + // This should have no side effects. + var initContainers []corev1.Container + for _, initContainer := range pod.Spec.InitContainers { + if initContainer.Name != naming.Container() { + initContainers = append(initContainers, initContainer) + } + } + pod.Spec.InitContainers = initContainers return pod } @@ -81,5 +102,12 @@ func existsIn(pod corev1.Pod) bool { return true } } + // NOTE: we also check init containers (native sidecars) since k8s 1.28. + // This should have no side effects. + for _, container := range pod.Spec.InitContainers { + if container.Name == naming.Container() { + return true + } + } return false } diff --git a/pkg/sidecar/pod_test.go b/pkg/sidecar/pod_test.go index c941961181..731550b9cd 100644 --- a/pkg/sidecar/pod_test.go +++ b/pkg/sidecar/pod_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + colfeaturegate "go.opentelemetry.io/collector/featuregate" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -26,10 +27,99 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) var logger = logf.Log.WithName("unit-tests") +func sidecarFeatureGate(t *testing.T) { + originalVal := featuregate.EnableNativeSidecarContainers.IsEnabled() + t.Logf("original is: %+v", originalVal) + require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNativeSidecarContainers.ID(), true)) + t.Cleanup(func() { + require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNativeSidecarContainers.ID(), originalVal)) + }) +} + +func TestAddNativeSidecar(t *testing.T) { + sidecarFeatureGate(t) + // prepare + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-app"}, + }, + InitContainers: []corev1.Container{ + { + Name: "my-init", + }, + }, + // cross-test: the pod has a volume already, make sure we don't remove it + Volumes: []corev1.Volume{{}}, + }, + } + + otelcol := v1beta1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "otelcol-native-sidecar", + Namespace: "some-app", + }, + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: v1beta1.ModeSidecar, + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + InitContainers: []corev1.Container{ + { + Name: "test", + }, + }, + }, + }, + } + + otelcolYaml, err := otelcol.Spec.Config.Yaml() + require.NoError(t, err) + cfg := config.New(config.WithCollectorImage("some-default-image")) + + // test + changed, err := add(cfg, logger, otelcol, pod, nil) + + // verify + assert.NoError(t, err) + require.Len(t, changed.Spec.Containers, 1) + require.Len(t, changed.Spec.InitContainers, 3) + require.Len(t, changed.Spec.Volumes, 1) + assert.Equal(t, "some-app.otelcol-native-sidecar", + changed.Labels["sidecar.opentelemetry.io/injected"]) + expectedPolicy := corev1.ContainerRestartPolicyAlways + assert.Equal(t, corev1.Container{ + Name: "otc-container", + Image: "some-default-image", + Args: []string{"--config=env:OTEL_CONFIG"}, + RestartPolicy: &expectedPolicy, + Env: []corev1.EnvVar{ + { + Name: "POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "OTEL_CONFIG", + Value: string(otelcolYaml), + }, + }, + Ports: []corev1.ContainerPort{ + { + Name: "metrics", + ContainerPort: 8888, + Protocol: corev1.ProtocolTCP, + }, + }, + }, changed.Spec.InitContainers[2]) +} + func TestAddSidecarWhenNoSidecarExists(t *testing.T) { // prepare pod := corev1.Pod{ @@ -146,6 +236,11 @@ func TestRemoveSidecar(t *testing.T) { {Name: naming.Container()}, {Name: naming.Container()}, // two sidecars! should remove both }, + InitContainers: []corev1.Container{ + {Name: "something"}, + {Name: naming.Container()}, // NOTE: native sidecar since k8s 1.28. + {Name: naming.Container()}, // two sidecars! should remove both + }, }, } @@ -190,6 +285,19 @@ func TestExistsIn(t *testing.T) { }, true}, + {"does-have-native-sidecar", + corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "my-app"}, + }, + InitContainers: []corev1.Container{ + {Name: naming.Container()}, + }, + }, + }, + true}, + {"does-not-have-sidecar", corev1.Pod{ Spec: corev1.PodSpec{ diff --git a/tests/e2e-native-sidecar/00-assert.yaml b/tests/e2e-native-sidecar/00-assert.yaml new file mode 100644 index 0000000000..8e67bb081e --- /dev/null +++ b/tests/e2e-native-sidecar/00-assert.yaml @@ -0,0 +1,21 @@ +--- +apiVersion: v1 +kind: Pod +metadata: + annotations: + sidecar.opentelemetry.io/inject: "true" + name: myapp +spec: + containers: + - name: myapp + initContainers: + - name: otc-container +status: + containerStatuses: + - name: myapp + ready: true + started: true + initContainerStatuses: + - name: otc-container + ready: true + started: true diff --git a/tests/e2e-native-sidecar/00-install.yaml b/tests/e2e-native-sidecar/00-install.yaml new file mode 100644 index 0000000000..e07768b415 --- /dev/null +++ b/tests/e2e-native-sidecar/00-install.yaml @@ -0,0 +1,41 @@ +--- +apiVersion: opentelemetry.io/v1beta1 +kind: OpenTelemetryCollector +metadata: + name: a-sidecar +spec: + mode: sidecar + resources: + limits: + cpu: 500m + memory: 128Mi + requests: + cpu: 5m + memory: 64Mi + + config: + receivers: + otlp: + protocols: + http: {} + exporters: + debug: {} + service: + pipelines: + metrics: + receivers: [otlp] + exporters: [debug] +--- +apiVersion: v1 +kind: Pod +metadata: + name: myapp + annotations: + sidecar.opentelemetry.io/inject: "true" +spec: + containers: + - name: myapp + image: jaegertracing/vertx-create-span:operator-e2e-tests + ports: + - containerPort: 8080 + protocol: TCP diff --git a/tests/e2e-native-sidecar/chainsaw-test.yaml b/tests/e2e-native-sidecar/chainsaw-test.yaml new file mode 100755 index 0000000000..0d93db6d15 --- /dev/null +++ b/tests/e2e-native-sidecar/chainsaw-test.yaml @@ -0,0 +1,14 @@ +# 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: native-sidecar +spec: + steps: + - name: step-00 + try: + - apply: + file: 00-install.yaml + - assert: + file: 00-assert.yaml