Inject otelcol sidecar into any namespace (#1395)

* Inject otelcol sidecar into any namespace Signed-off-by: Pavol Loffay <[email protected]> * review comments Signed-off-by: Pavol Loffay <[email protected]> * review comments Signed-off-by: Pavol Loffay <[email protected]> * Consume otelcol config directly from env var Signed-off-by: Pavol Loffay <[email protected]> * Fix lint Signed-off-by: Pavol Loffay <[email protected]> * revert Signed-off-by: Pavol Loffay <[email protected]> * revert init prepper image Signed-off-by: Pavol Loffay <[email protected]> * Change order Signed-off-by: Pavol Loffay <[email protected]> --------- Signed-off-by: Pavol Loffay <[email protected]>
open-telemetry · Feb 1, 2023 · 41938a9 · 41938a9
1 parent 55c37bc
commit 41938a9
Show file tree

Hide file tree

Showing 19 changed files with 195 additions and 51 deletions.
diff --git a/.chloggen/inject-any-namespace.yaml 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
@@ -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 - <<EOF
@@ -139,6 +139,13 @@ The annotation value can come either from the namespace, or from the pod. The mo
 * the pod annotation is used when it's set to a concrete instance name or to `"false"`
 * namespace annotation is used when the pod annotation is either absent or set to `"true"`, and the namespace is set to a concrete instance or to `"false"`
 
+The possible values for the annotation can be:
+
+* "true" - inject `OpenTelemetryCollector` resource from the namespace.
+* "sidecar-for-my-app" - name of `OpenTelemetryCollector` CR instance in the current namespace.
+* "my-other-namespace/my-instrumentation" - name and namespace of `OpenTelemetryCollector` CR instance in another namespace.
+* "false" - do not inject
+
 When using a pod-based workload, such as `Deployment` or `Statefulset`, make sure to add the annotation to the `PodTemplate` part. Like:
 
 ```yaml

diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml
@@ -1,2 +1,2 @@
 resources:
-- manager.yaml
+- manager.yaml
diff --git a/internal/webhookhandler/webhookhandler_test.go b/internal/webhookhandler/webhookhandler_test.go
@@ -163,11 +163,10 @@ func TestShouldInjectSidecar(t *testing.T) {
 			// verify
 			assert.True(t, res.Allowed)
 			assert.Nil(t, res.AdmissionResponse.Result)
-			assert.Len(t, res.Patches, 3)
+			assert.Len(t, res.Patches, 2)
 
 			expectedMap := map[string]bool{
 				"/metadata/labels": false, // add a new label
-				"/spec/volumes":    false, // add a new volume with the configmap
 				"/spec/containers": false, // replace the containers, adding one new container
 			}
 			for _, patch := range res.Patches {

diff --git a/pkg/collector/container.go b/pkg/collector/container.go
@@ -36,7 +36,7 @@ import (
 const maxPortLen = 15
 
 // Container builds a container for the given collector.
-func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) corev1.Container {
+func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector, addConfig bool) corev1.Container {
 	image := otelcol.Spec.Image
 	if len(image) == 0 {
 		image = cfg.CollectorImage()
@@ -52,28 +52,29 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem
 		}
 	}
 
+	var volumeMounts []corev1.VolumeMount
 	argsMap := otelcol.Spec.Args
 	if argsMap == nil {
 		argsMap = map[string]string{}
 	}
-
-	if _, exists := argsMap["config"]; exists {
-		logger.Info("the 'config' flag isn't allowed and is being ignored")
+	if addConfig {
+		if _, exists := argsMap["config"]; exists {
+			logger.Info("the 'config' flag isn't allowed and is being ignored")
+		}
+		// this effectively overrides any 'config' entry that might exist in the CR
+		argsMap["config"] = fmt.Sprintf("/conf/%s", cfg.CollectorConfigMapEntry())
+		volumeMounts = append(volumeMounts,
+			corev1.VolumeMount{
+				Name:      naming.ConfigMapVolume(),
+				MountPath: "/conf",
+			})
 	}
 
-	// this effectively overrides any 'config' entry that might exist in the CR
-	argsMap["config"] = fmt.Sprintf("/conf/%s", cfg.CollectorConfigMapEntry())
-
 	var args []string
 	for k, v := range argsMap {
 		args = append(args, fmt.Sprintf("--%s=%s", k, v))
 	}
 
-	volumeMounts := []corev1.VolumeMount{{
-		Name:      naming.ConfigMapVolume(),
-		MountPath: "/conf",
-	}}
-
 	if len(otelcol.Spec.VolumeMounts) > 0 {
 		volumeMounts = append(volumeMounts, otelcol.Spec.VolumeMounts...)
 	}

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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{}