Avoid rerunning the pod mutator for the instrumentation injection mor…

…e than one time (#2048)

* Avoid running the instrumentation pod mutator for already instrumented pods

Signed-off-by: Israel Blancas <[email protected]>

* Add missing changelog

Signed-off-by: Israel Blancas <[email protected]>

* Fix E2E tests

Signed-off-by: Israel Blancas <[email protected]>

* Apply changes requested in code review

Signed-off-by: Israel Blancas <[email protected]>

* Add unit test for hte golang and env variable situations

Signed-off-by: Israel Blancas <[email protected]>

* Fix lint

Signed-off-by: Israel Blancas <[email protected]>

---------

Signed-off-by: Israel Blancas <[email protected]>

.chloggen/1366-avoid-running-the-instrumentation-pod-mutator-for-instrumented-pods.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# 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: "Avoid running the auto-instrumentation pod mutator for pods already auto-instrumented"
+
+# One or more tracking issues related to the change
+issues: [1366]
+
+# (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:

pkg/instrumentation/helper.go

@@ -16,6 +16,9 @@ package instrumentation
 
 import (
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/constants"
 )
 
 // Calculate if we already inject InitContainers.
@@ -35,11 +38,24 @@ func isAutoInstrumentationInjected(pod corev1.Pod) bool {
 			return true
 		}
 	}
-	// Go uses a side car
+
 	for _, cont := range pod.Spec.Containers {
+		// Go uses a sidecar
 		if cont.Name == sideCarName {
 			return true
 		}
+
+		// This environment variable is set in the sidecar and in the
+		// collector containers. We look for it in any container that is not
+		// the sidecar container to check if we already injected the
+		// instrumentation or not
+		if cont.Name != naming.Container() {
+			for _, envVar := range cont.Env {
+				if envVar.Name == constants.EnvNodeName {
+					return true
+				}
+			}
+		}
 	}
 	return false
 }

pkg/instrumentation/helper_test.go

@@ -19,6 +19,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+
+	"github.com/open-telemetry/opentelemetry-operator/pkg/constants"
 )
 
 func TestInitContainerMissing(t *testing.T) {
@@ -95,6 +97,40 @@ func TestAutoInstrumentationInjected(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: "AutoInstrumentation_Already_Inject_go",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					InitContainers: []corev1.Container{},
+					Containers: []corev1.Container{
+						{
+							Name: sideCarName,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "AutoInstrumentation_Already_Inject_no_init_containers",
+			pod: corev1.Pod{
+				Spec: corev1.PodSpec{
+					InitContainers: []corev1.Container{},
+					Containers: []corev1.Container{
+						{
+							Name: "my-app",
+							Env: []corev1.EnvVar{
+								{
+									Name:  constants.EnvNodeName,
+									Value: "value",
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: true,
+		},
 		{
 			name: "AutoInstrumentation_Absent_1",
 			pod: corev1.Pod{

tests/e2e/instrumentation-sdk/00-install-collector.yaml

@@ -21,3 +21,12 @@ spec:
           receivers: [otlp]
           processors: []
           exporters: [logging]
+---
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  # Annotate the namespace to allow the application to run using an specific group and user in OpenShift
+  # https://docs.openshift.com/dedicated/authentication/managing-security-context-constraints.html
+  # This annotation has no effect in Kubernetes
+  - command: kubectl annotate namespace ${NAMESPACE} openshift.io/sa.scc.uid-range=1000/1000 --overwrite
+  - command: kubectl annotate namespace ${NAMESPACE} openshift.io/sa.scc.supplemental-groups=2000/1000 --overwrite