Skip to content

Commit

Permalink
add featuregate for k8s 1.28 native sidecar container
Browse files Browse the repository at this point in the history
Signed-off-by: Benedikt Bongartz <[email protected]>
  • Loading branch information
frzifus committed Jul 16, 2024
1 parent a515234 commit 5261059
Show file tree
Hide file tree
Showing 8 changed files with 250 additions and 1 deletion.
16 changes: 16 additions & 0 deletions .chloggen/native_sidecar.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. 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:
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,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
Expand Down
13 changes: 13 additions & 0 deletions pkg/featuregate/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.105.0"),
)
// PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator.
PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister(
"operator.observability.prometheus",
Expand Down
30 changes: 29 additions & 1 deletion pkg/sidecar/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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.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 {
Expand All @@ -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
}

Expand All @@ -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
}
115 changes: 115 additions & 0 deletions pkg/sidecar/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,114 @@ 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"

"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{

Check failure on line 70 in pkg/sidecar/pod_test.go

View workflow job for this annotation

GitHub Actions / Code standards (linting)

cannot use []corev1.ServicePort{…} (value of type []"k8s.io/api/core/v1".ServicePort) as []"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1".PortsSpec value in struct literal (typecheck)
{
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{
Expand Down Expand Up @@ -146,6 +243,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
},
},
}

Expand Down Expand Up @@ -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{
Expand Down
15 changes: 15 additions & 0 deletions tests/e2e-native-sidecar/00-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
apiVersion: v1
kind: Pod
metadata:
sidecar.opentelemetry.io/inject: "true"
name: myapp
status:
containerStatuses:
- name: myapp
ready: true
started: true
initContainerStatuses:
- name: otc-container
ready: true
started: true
41 changes: 41 additions & 0 deletions tests/e2e-native-sidecar/00-install.yaml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions tests/e2e-native-sidecar/chainsaw-test.yaml
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5261059

Please sign in to comment.