From 41938a9508f13871025633fe7ec24ae43c08e58d Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 1 Feb 2023 18:09:17 +0100 Subject: [PATCH] Inject otelcol sidecar into any namespace (#1395) * Inject otelcol sidecar into any namespace Signed-off-by: Pavol Loffay * review comments Signed-off-by: Pavol Loffay * review comments Signed-off-by: Pavol Loffay * Consume otelcol config directly from env var Signed-off-by: Pavol Loffay * Fix lint Signed-off-by: Pavol Loffay * revert Signed-off-by: Pavol Loffay * revert init prepper image Signed-off-by: Pavol Loffay * Change order Signed-off-by: Pavol Loffay --------- Signed-off-by: Pavol Loffay --- .chloggen/inject-any-namespace.yaml | 16 ++++++++ README.md | 9 ++++- config/manager/kustomization.yaml | 2 +- .../webhookhandler/webhookhandler_test.go | 3 +- pkg/collector/container.go | 25 ++++++------ pkg/collector/container_test.go | 30 +++++++-------- pkg/collector/daemonset.go | 2 +- pkg/collector/deployment.go | 2 +- pkg/collector/reconcile/config_replace.go | 13 ++++--- .../reconcile/config_replace_test.go | 4 +- pkg/collector/reconcile/configmap.go | 2 +- pkg/collector/statefulset.go | 2 +- pkg/sidecar/pod.go | 18 ++++++--- pkg/sidecar/pod_test.go | 38 ++++++++++++++++++- pkg/sidecar/podmutator.go | 9 ++++- .../00-install.yaml | 29 ++++++++++++++ .../01-assert.yaml | 21 ++++++++++ .../01-install-app.yaml | 20 ++++++++++ tests/e2e/smoke-sidecar/01-assert.yaml | 1 + 19 files changed, 195 insertions(+), 51 deletions(-) create mode 100755 .chloggen/inject-any-namespace.yaml create mode 100644 tests/e2e/smoke-sidecar-other-namespace/00-install.yaml create mode 100644 tests/e2e/smoke-sidecar-other-namespace/01-assert.yaml create mode 100644 tests/e2e/smoke-sidecar-other-namespace/01-install-app.yaml diff --git a/.chloggen/inject-any-namespace.yaml b/.chloggen/inject-any-namespace.yaml new file mode 100755 index 0000000000..9288e9c39d --- /dev/null +++ b/.chloggen/inject-any-namespace.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. 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: Support sidecar injecton into any namespace. + +# One or more tracking issues related to the change +issues: [199] + +# (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/README.md b/README.md index 39dbf9266e..840051d942 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ The `CustomResource` for the `OpenTelemetryCollector` exposes a property named ` #### Sidecar injection -A sidecar with the OpenTelemetry Collector can be injected into pod-based workloads by setting the pod annotation `sidecar.opentelemetry.io/inject` to either `"true"`, or to the name of a concrete `OpenTelemetryCollector` from the same namespace, like in the following example: +A sidecar with the OpenTelemetry Collector can be injected into pod-based workloads by setting the pod annotation `sidecar.opentelemetry.io/inject` to either `"true"`, or to the name of a concrete `OpenTelemetryCollector`, like in the following example: ```yaml kubectl apply -f - < 0 { volumeMounts = append(volumeMounts, otelcol.Spec.VolumeMounts...) } diff --git a/pkg/collector/container_test.go b/pkg/collector/container_test.go index a0d7155e59..2d4ffba360 100644 --- a/pkg/collector/container_test.go +++ b/pkg/collector/container_test.go @@ -41,7 +41,7 @@ func TestContainerNewDefault(t *testing.T) { cfg := config.New(config.WithCollectorImage("default-image")) // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Equal(t, "default-image", c.Image) @@ -58,7 +58,7 @@ func TestContainerWithImageOverridden(t *testing.T) { cfg := config.New(config.WithCollectorImage("default-image")) // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Equal(t, "overridden-image", c.Image) @@ -191,7 +191,7 @@ service: cfg := config.New(config.WithCollectorImage("default-image")) // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.ElementsMatch(t, testCase.expectedPorts, c.Ports) @@ -212,7 +212,7 @@ func TestContainerConfigFlagIsIgnored(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Len(t, c.Args, 2) @@ -232,7 +232,7 @@ func TestContainerCustomVolumes(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Len(t, c.VolumeMounts, 2) @@ -241,7 +241,7 @@ func TestContainerCustomVolumes(t *testing.T) { func TestContainerCustomSecurityContext(t *testing.T) { // default config without security context - c1 := Container(config.New(), logger, v1alpha1.OpenTelemetryCollector{Spec: v1alpha1.OpenTelemetryCollectorSpec{}}) + c1 := Container(config.New(), logger, v1alpha1.OpenTelemetryCollector{Spec: v1alpha1.OpenTelemetryCollectorSpec{}}, true) // verify assert.Nil(t, c1.SecurityContext) @@ -258,7 +258,7 @@ func TestContainerCustomSecurityContext(t *testing.T) { RunAsUser: &uid, }, }, - }) + }, true) // verify assert.NotNil(t, c2.SecurityContext) @@ -281,7 +281,7 @@ func TestContainerEnvVarsOverridden(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Len(t, c.Env, 2) @@ -297,7 +297,7 @@ func TestContainerDefaultEnvVars(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Len(t, c.Env, 1) @@ -323,7 +323,7 @@ func TestContainerResourceRequirements(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Equal(t, resource.MustParse("100m"), *c.Resources.Limits.Cpu()) @@ -340,7 +340,7 @@ func TestContainerDefaultResourceRequirements(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Empty(t, c.Resources) @@ -359,7 +359,7 @@ func TestContainerArgs(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Contains(t, c.Args, "--metrics-level=detailed") @@ -376,7 +376,7 @@ func TestContainerImagePullPolicy(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Equal(t, c.ImagePullPolicy, corev1.PullIfNotPresent) @@ -409,7 +409,7 @@ func TestContainerEnvFrom(t *testing.T) { cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Contains(t, c.EnvFrom, envFrom1) @@ -429,7 +429,7 @@ service: cfg := config.New() // test - c := Container(cfg, logger, otelcol) + c := Container(cfg, logger, otelcol, true) // verify assert.Equal(t, "/", c.LivenessProbe.HTTPGet.Path) diff --git a/pkg/collector/daemonset.go b/pkg/collector/daemonset.go index 565d1a92bc..591a020cec 100644 --- a/pkg/collector/daemonset.go +++ b/pkg/collector/daemonset.go @@ -50,7 +50,7 @@ func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem }, Spec: corev1.PodSpec{ ServiceAccountName: ServiceAccountName(otelcol), - Containers: []corev1.Container{Container(cfg, logger, otelcol)}, + Containers: []corev1.Container{Container(cfg, logger, otelcol, true)}, Volumes: Volumes(cfg, otelcol), Tolerations: otelcol.Spec.Tolerations, NodeSelector: otelcol.Spec.NodeSelector, diff --git a/pkg/collector/deployment.go b/pkg/collector/deployment.go index cb0211fbf8..5df6b347e6 100644 --- a/pkg/collector/deployment.go +++ b/pkg/collector/deployment.go @@ -52,7 +52,7 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele }, Spec: corev1.PodSpec{ ServiceAccountName: ServiceAccountName(otelcol), - Containers: []corev1.Container{Container(cfg, logger, otelcol)}, + Containers: []corev1.Container{Container(cfg, logger, otelcol, true)}, Volumes: Volumes(cfg, otelcol), DNSPolicy: getDNSPolicy(otelcol), HostNetwork: otelcol.Spec.HostNetwork, diff --git a/pkg/collector/reconcile/config_replace.go b/pkg/collector/reconcile/config_replace.go index 8be892b654..6ad5ea0984 100644 --- a/pkg/collector/reconcile/config_replace.go +++ b/pkg/collector/reconcile/config_replace.go @@ -25,6 +25,7 @@ import ( _ "github.com/prometheus/prometheus/discovery/install" // Package install has the side-effect of registering all builtin. "gopkg.in/yaml.v2" + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" @@ -34,16 +35,16 @@ type Config struct { PromConfig *promconfig.Config `yaml:"config"` } -func ReplaceConfig(params Params) (string, error) { - if !params.Instance.Spec.TargetAllocator.Enabled { - return params.Instance.Spec.Config, nil +func ReplaceConfig(instance v1alpha1.OpenTelemetryCollector) (string, error) { + if !instance.Spec.TargetAllocator.Enabled { + return instance.Spec.Config, nil } - config, getStringErr := adapters.ConfigFromString(params.Instance.Spec.Config) + config, getStringErr := adapters.ConfigFromString(instance.Spec.Config) if getStringErr != nil { return "", getStringErr } - promCfgMap, getCfgPromErr := ta.ConfigToPromConfig(params.Instance.Spec.Config) + promCfgMap, getCfgPromErr := ta.ConfigToPromConfig(instance.Spec.Config) if getCfgPromErr != nil { return "", getCfgPromErr } @@ -65,7 +66,7 @@ func ReplaceConfig(params Params) (string, error) { escapedJob := url.QueryEscape(cfg.PromConfig.ScrapeConfigs[i].JobName) cfg.PromConfig.ScrapeConfigs[i].ServiceDiscoveryConfigs = discovery.Configs{ &http.SDConfig{ - URL: fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", naming.TAService(params.Instance), escapedJob), + URL: fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", naming.TAService(instance), escapedJob), }, } } diff --git a/pkg/collector/reconcile/config_replace_test.go b/pkg/collector/reconcile/config_replace_test.go index 0d867492b8..3ccd43f079 100644 --- a/pkg/collector/reconcile/config_replace_test.go +++ b/pkg/collector/reconcile/config_replace_test.go @@ -29,7 +29,7 @@ func TestPrometheusParser(t *testing.T) { assert.NoError(t, err) t.Run("should update config with http_sd_config", func(t *testing.T) { - actualConfig, err := ReplaceConfig(param) + actualConfig, err := ReplaceConfig(param.Instance) assert.NoError(t, err) // prepare @@ -63,7 +63,7 @@ func TestPrometheusParser(t *testing.T) { t.Run("should not update config with http_sd_config", func(t *testing.T) { param.Instance.Spec.TargetAllocator.Enabled = false - actualConfig, err := ReplaceConfig(param) + actualConfig, err := ReplaceConfig(param.Instance) assert.NoError(t, err) // prepare diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go index 24a6f7cef3..b5e9906ed8 100644 --- a/pkg/collector/reconcile/configmap.go +++ b/pkg/collector/reconcile/configmap.go @@ -74,7 +74,7 @@ func desiredConfigMap(_ context.Context, params Params) corev1.ConfigMap { } else { labels["app.kubernetes.io/version"] = "latest" } - config, err := ReplaceConfig(params) + config, err := ReplaceConfig(params.Instance) if err != nil { params.Log.V(2).Info("failed to update prometheus config to use sharded targets: ", err) } diff --git a/pkg/collector/statefulset.go b/pkg/collector/statefulset.go index ba76d47324..2ea7d77a0c 100644 --- a/pkg/collector/statefulset.go +++ b/pkg/collector/statefulset.go @@ -52,7 +52,7 @@ func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTel }, Spec: corev1.PodSpec{ ServiceAccountName: ServiceAccountName(otelcol), - Containers: []corev1.Container{Container(cfg, logger, otelcol)}, + Containers: []corev1.Container{Container(cfg, logger, otelcol, true)}, Volumes: Volumes(cfg, otelcol), DNSPolicy: getDNSPolicy(otelcol), HostNetwork: otelcol.Spec.HostNetwork, diff --git a/pkg/sidecar/pod.go b/pkg/sidecar/pod.go index 593d3d0c3a..c86d2f3ae8 100644 --- a/pkg/sidecar/pod.go +++ b/pkg/sidecar/pod.go @@ -24,23 +24,31 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + "github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile" "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) const ( - label = "sidecar.opentelemetry.io/injected" + label = "sidecar.opentelemetry.io/injected" + confEnvVar = "OTEL_CONFIG" ) // add a new sidecar container to the given pod, based on the given OpenTelemetryCollector. func add(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector, pod corev1.Pod, attributes []corev1.EnvVar) (corev1.Pod, error) { - // add the container - volumes := collector.Volumes(cfg, otelcol) - container := collector.Container(cfg, logger, otelcol) + otelColCfg, err := reconcile.ReplaceConfig(otelcol) + if err != nil { + return pod, err + } + + container := collector.Container(cfg, logger, otelcol, false) + container.Args = append(container.Args, fmt.Sprintf("--config=env:%s", confEnvVar)) + + container.Env = append(container.Env, corev1.EnvVar{Name: confEnvVar, Value: otelColCfg}) if !hasResourceAttributeEnvVar(container.Env) { container.Env = append(container.Env, attributes...) } pod.Spec.Containers = append(pod.Spec.Containers, container) - pod.Spec.Volumes = append(pod.Spec.Volumes, volumes...) + pod.Spec.Volumes = append(pod.Spec.Volumes, otelcol.Spec.Volumes...) if pod.Labels == nil { pod.Labels = map[string]string{} diff --git a/pkg/sidecar/pod_test.go b/pkg/sidecar/pod_test.go index 1982e53153..caee349b5b 100644 --- a/pkg/sidecar/pod_test.go +++ b/pkg/sidecar/pod_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -45,6 +46,13 @@ func TestAddSidecarWhenNoSidecarExists(t *testing.T) { Name: "otelcol-sample", Namespace: "some-app", }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: ` +receivers: +exporters: +processors: +`, + }, } cfg := config.New(config.WithCollectorImage("some-default-image")) @@ -53,9 +61,35 @@ func TestAddSidecarWhenNoSidecarExists(t *testing.T) { // verify assert.NoError(t, err) - assert.Len(t, changed.Spec.Containers, 2) - assert.Len(t, changed.Spec.Volumes, 2) + require.Len(t, changed.Spec.Containers, 2) + require.Len(t, changed.Spec.Volumes, 1) assert.Equal(t, "some-app.otelcol-sample", changed.Labels["sidecar.opentelemetry.io/injected"]) + assert.Equal(t, corev1.Container{ + Name: "otc-container", + Image: "some-default-image", + Args: []string{"--config=env:OTEL_CONFIG"}, + Env: []corev1.EnvVar{ + { + Name: "POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "OTEL_CONFIG", + Value: otelcol.Spec.Config, + }, + }, + Ports: []corev1.ContainerPort{ + { + Name: "metrics", + ContainerPort: 8888, + Protocol: corev1.ProtocolTCP, + }, + }, + }, changed.Spec.Containers[1]) } // this situation should never happen in the current code path, but it should not fail diff --git a/pkg/sidecar/podmutator.go b/pkg/sidecar/podmutator.go index 22302a44ea..7f1956e4dd 100644 --- a/pkg/sidecar/podmutator.go +++ b/pkg/sidecar/podmutator.go @@ -106,7 +106,14 @@ func (p *sidecarPodMutator) getCollectorInstance(ctx context.Context, ns corev1. } otelcol := v1alpha1.OpenTelemetryCollector{} - err := p.client.Get(ctx, types.NamespacedName{Name: ann, Namespace: ns.Name}, &otelcol) + var nsnOtelcol types.NamespacedName + instNamespace, instName, namespaced := strings.Cut(ann, "/") + if namespaced { + nsnOtelcol = types.NamespacedName{Name: instName, Namespace: instNamespace} + } else { + nsnOtelcol = types.NamespacedName{Name: ann, Namespace: ns.Name} + } + err := p.client.Get(ctx, nsnOtelcol, &otelcol) if err != nil { return otelcol, err } diff --git a/tests/e2e/smoke-sidecar-other-namespace/00-install.yaml b/tests/e2e/smoke-sidecar-other-namespace/00-install.yaml new file mode 100644 index 0000000000..9f64141343 --- /dev/null +++ b/tests/e2e/smoke-sidecar-other-namespace/00-install.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: kuttl-otel-sidecar-other-namespace +--- +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar-for-my-app + namespace: kuttl-otel-sidecar-other-namespace +spec: + mode: sidecar + config: | + receivers: + jaeger: + protocols: + grpc: + processors: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [jaeger] + processors: [] + exporters: [logging] diff --git a/tests/e2e/smoke-sidecar-other-namespace/01-assert.yaml b/tests/e2e/smoke-sidecar-other-namespace/01-assert.yaml new file mode 100644 index 0000000000..af10887f64 --- /dev/null +++ b/tests/e2e/smoke-sidecar-other-namespace/01-assert.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: Pod +metadata: + namespace: kuttl-otel-sidecar-other-namespace + annotations: + sidecar.opentelemetry.io/inject: "kuttl-otel-sidecar-other-namespace/sidecar-for-my-app" + labels: + app: my-pod-with-sidecar +spec: + containers: + - name: myapp + - name: otc-container + env: + - name: POD_NAME + - name: OTEL_CONFIG + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + - name: OTEL_RESOURCE_ATTRIBUTES_POD_UID + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + - name: OTEL_RESOURCE_ATTRIBUTES +status: + phase: Running diff --git a/tests/e2e/smoke-sidecar-other-namespace/01-install-app.yaml b/tests/e2e/smoke-sidecar-other-namespace/01-install-app.yaml new file mode 100644 index 0000000000..b85e38b91d --- /dev/null +++ b/tests/e2e/smoke-sidecar-other-namespace/01-install-app.yaml @@ -0,0 +1,20 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-deployment-with-sidecar + namespace: kuttl-otel-sidecar-other-namespace +spec: + selector: + matchLabels: + app: my-pod-with-sidecar + replicas: 1 + template: + metadata: + labels: + app: my-pod-with-sidecar + annotations: + sidecar.opentelemetry.io/inject: "kuttl-otel-sidecar-other-namespace/sidecar-for-my-app" + spec: + containers: + - name: myapp + image: k8s.gcr.io/echoserver:1.4 diff --git a/tests/e2e/smoke-sidecar/01-assert.yaml b/tests/e2e/smoke-sidecar/01-assert.yaml index 358711a288..b4bd1b9603 100644 --- a/tests/e2e/smoke-sidecar/01-assert.yaml +++ b/tests/e2e/smoke-sidecar/01-assert.yaml @@ -11,6 +11,7 @@ spec: - name: otc-container env: - name: POD_NAME + - name: OTEL_CONFIG - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME - name: OTEL_RESOURCE_ATTRIBUTES_POD_UID - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME