From 31b8e80d44da5701acb2824f99e95adab9ee5290 Mon Sep 17 00:00:00 2001 From: "Mateusz \"mat\" Rumian" <58699800+mat-rumian@users.noreply.github.com> Date: Mon, 4 Jul 2022 16:53:03 +0200 Subject: [PATCH] Fix Pod Mutation loop (#953) * chore(webhookhandler): check if pod already instrumented * chore(instrumentations): add check for volume and instrumentation env var value * chore(webhookhandler): add helper tests * chore(instrumentation): add helper tests * chore(instrumentation): update volume checking method * chore(lint): fix * chore(webhookhandler): move out pod instrumentation check * chore(instrumentation): add pod instrumentation inject check * chore(instrumentation): clean up --- pkg/instrumentation/helper.go | 12 ++++++- pkg/instrumentation/helper_test.go | 54 +++++++++++++++++++++++++++++- pkg/instrumentation/javaagent.go | 5 ++- pkg/instrumentation/nodejs.go | 5 ++- pkg/instrumentation/podmutator.go | 6 ++++ pkg/instrumentation/python.go | 4 ++- 6 files changed, 81 insertions(+), 5 deletions(-) diff --git a/pkg/instrumentation/helper.go b/pkg/instrumentation/helper.go index 92a66df995..7ade3b7b3f 100644 --- a/pkg/instrumentation/helper.go +++ b/pkg/instrumentation/helper.go @@ -17,7 +17,7 @@ package instrumentation import corev1 "k8s.io/api/core/v1" // Calculate if we already inject InitContainers. -func IsInitContainerMissing(pod corev1.Pod) bool { +func isInitContainerMissing(pod corev1.Pod) bool { for _, initContainer := range pod.Spec.InitContainers { if initContainer.Name == initContainerName { return false @@ -25,3 +25,13 @@ func IsInitContainerMissing(pod corev1.Pod) bool { } return true } + +// Checks if Pod is already instrumented by checking Instrumentation InitContainer presence. +func isAutoInstrumentationInjected(pod corev1.Pod) bool { + for _, cont := range pod.Spec.InitContainers { + if cont.Name == initContainerName { + return true + } + } + return false +} diff --git a/pkg/instrumentation/helper_test.go b/pkg/instrumentation/helper_test.go index 2de3132b09..b2f37d1c0b 100644 --- a/pkg/instrumentation/helper_test.go +++ b/pkg/instrumentation/helper_test.go @@ -67,7 +67,59 @@ func TestInitContainerMissing(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result := IsInitContainerMissing(test.pod) + result := isInitContainerMissing(test.pod) + assert.Equal(t, test.expected, result) + }) + } +} + +func TestAutoInstrumentationInjected(t *testing.T) { + tests := []struct { + name string + pod corev1.Pod + expected bool + }{ + { + name: "AutoInstrumentation_Already_Inject", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "magic-init", + }, + { + Name: initContainerName, + }, + }, + }, + }, + expected: true, + }, + { + name: "AutoInstrumentation_Absent_1", + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "magic-init", + }, + }, + }, + }, + expected: false, + }, + { + name: "AutoInstrumentation_Absent_2", + pod: corev1.Pod{ + Spec: corev1.PodSpec{}, + }, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := isAutoInstrumentationInjected(test.pod) assert.Equal(t, test.expected, result) }) } diff --git a/pkg/instrumentation/javaagent.go b/pkg/instrumentation/javaagent.go index 61a3d54e2f..1d534a4b1d 100644 --- a/pkg/instrumentation/javaagent.go +++ b/pkg/instrumentation/javaagent.go @@ -50,15 +50,18 @@ func injectJavaagent(logger logr.Logger, javaSpec v1alpha1.Java, pod corev1.Pod, logger.Info("Skipping javaagent injection, the container defines JAVA_TOOL_OPTIONS env var value via ValueFrom", "container", container.Name) return pod } + container.Env[idx].Value = container.Env[idx].Value + javaJVMArgument + } + container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ Name: volumeName, MountPath: "/otel-auto-instrumentation", }) // We just inject Volumes and init containers for the first processed container - if IsInitContainerMissing(pod) { + if isInitContainerMissing(pod) { pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ diff --git a/pkg/instrumentation/nodejs.go b/pkg/instrumentation/nodejs.go index 645f9356a0..5a26fd6adf 100644 --- a/pkg/instrumentation/nodejs.go +++ b/pkg/instrumentation/nodejs.go @@ -50,15 +50,18 @@ func injectNodeJSSDK(logger logr.Logger, nodeJSSpec v1alpha1.NodeJS, pod corev1. logger.Info("Skipping NodeJS SDK injection, the container defines NODE_OPTIONS env var value via ValueFrom", "container", container.Name) return pod } + container.Env[idx].Value = container.Env[idx].Value + nodeRequireArgument + } + container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ Name: volumeName, MountPath: "/otel-auto-instrumentation", }) // We just inject Volumes and init containers for the first processed container - if IsInitContainerMissing(pod) { + if isInitContainerMissing(pod) { pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ diff --git a/pkg/instrumentation/podmutator.go b/pkg/instrumentation/podmutator.go index 8f9c4c237b..cc49a8eab1 100644 --- a/pkg/instrumentation/podmutator.go +++ b/pkg/instrumentation/podmutator.go @@ -61,6 +61,12 @@ func NewMutator(logger logr.Logger, client client.Client) *instPodMutator { func (pm *instPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod corev1.Pod) (corev1.Pod, error) { logger := pm.Logger.WithValues("namespace", pod.Namespace, "name", pod.Name) + // We check if Pod is already instrumented. + if isAutoInstrumentationInjected(pod) { + logger.Info("Skipping pod instrumentation - already instrumented") + return pod, nil + } + var inst *v1alpha1.Instrumentation var err error diff --git a/pkg/instrumentation/python.go b/pkg/instrumentation/python.go index 6f2a77bdda..e2c7495b0a 100644 --- a/pkg/instrumentation/python.go +++ b/pkg/instrumentation/python.go @@ -54,7 +54,9 @@ func injectPythonSDK(logger logr.Logger, pythonSpec v1alpha1.Python, pod corev1. logger.Info("Skipping Python SDK injection, the container defines PYTHONPATH env var value via ValueFrom", "container", container.Name) return pod } + container.Env[idx].Value = fmt.Sprintf("%s:%s:%s", pythonPathPrefix, container.Env[idx].Value, pythonPathSuffix) + } // Set OTEL_TRACES_EXPORTER to HTTP exporter if not set by user because it is what our autoinstrumentation supports. @@ -72,7 +74,7 @@ func injectPythonSDK(logger logr.Logger, pythonSpec v1alpha1.Python, pod corev1. }) // We just inject Volumes and init containers for the first processed container - if IsInitContainerMissing(pod) { + if isInitContainerMissing(pod) { pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{