From 83849107b74e9b2fcf798416fa5b865d2a555fdf 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 +++++ pkg/featuregate/featuregate.go | 13 ++++ pkg/sidecar/pod.go | 29 ++++++++- pkg/sidecar/pod_test.go | 115 +++++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 1 deletion(-) create mode 100755 .chloggen/native_sidecar.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/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index ca3ecde484..98fd3df1c6 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -25,6 +25,19 @@ 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"), + featuregate.WithRegisterFromVersion("v0.98.0"), + ) EnableJavaAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( "operator.autoinstrumentation.java", featuregate.StageBeta, diff --git a/pkg/sidecar/pod.go b/pkg/sidecar/pod.go index 445af47e47..cac673e2bc 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,15 @@ 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 + // TODO(frzifus): Add StartupProbe + 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 +80,17 @@ func remove(pod corev1.Pod) (corev1.Pod, error) { } } 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, nil } @@ -81,5 +101,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 0e843d7bee..6c9b7a176b 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,106 @@ 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{ + Ports: []corev1.ServicePort{ + { + Name: "metrics", + Port: 8888, + Protocol: corev1.ProtocolTCP, + }, + }, + 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{ @@ -144,6 +241,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 +292,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{