Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Pod Mutation loop #953

Merged
merged 10 commits into from
Jul 4, 2022
+81 −5
Merged

Fix Pod Mutation loop #953

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c7974da
chore(webhookhandler): check if pod already instrumented
mat-rumian Jun 27, 2022
cfb8b81
chore(instrumentations): add check for volume and instrumentation env…
mat-rumian Jun 27, 2022
5acbe82
Merge branch 'open-telemetry:main' into fix-pod-mutation-loop
mat-rumian Jun 27, 2022
7f3989c
chore(webhookhandler): add helper tests
mat-rumian Jun 28, 2022
5dbafa7
chore(instrumentation): add helper tests
mat-rumian Jun 28, 2022
b906f7d
chore(instrumentation): update volume checking method
mat-rumian Jun 28, 2022
b5f1cc1
chore(lint): fix
mat-rumian Jun 28, 2022
99346db
chore(webhookhandler): move out pod instrumentation check
mat-rumian Jun 30, 2022
1fdb631
chore(instrumentation): add pod instrumentation inject check
mat-rumian Jun 30, 2022
08e0c1d
chore(instrumentation): clean up
mat-rumian Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions internal/webhookhandler/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package webhookhandler
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved

import corev1 "k8s.io/api/core/v1"

// Checks if Pod is already instrumented by checking Instrumentation InitContainer presence
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved
func IsPodInstrumentationMissing(pod corev1.Pod) bool {
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved
for _, cont := range pod.Spec.InitContainers {
if cont.Name == initContainerName {
return false
}
}
return true
}
14 changes: 9 additions & 5 deletions internal/webhookhandler/webhookhandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import (
// +kubebuilder:rbac:groups=opentelemetry.io,resources=instrumentations,verbs=get;list;watch
// +kubebuilder:rbac:groups="apps",resources=replicasets,verbs=get;list;watch

const initContainerName = "opentelemetry-auto-instrumentation"
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved

var _ WebhookHandler = (*podSidecarInjector)(nil)

// WebhookHandler is a webhook handler that analyzes new pods and injects appropriate sidecars into it.
Expand Down Expand Up @@ -89,11 +91,13 @@ func (p *podSidecarInjector) Handle(ctx context.Context, req admission.Request)
}

for _, m := range p.podMutators {
pod, err = m.Mutate(ctx, ns, pod)
if err != nil {
res := admission.Errored(http.StatusInternalServerError, err)
res.Allowed = true
return res
if IsPodInstrumentationMissing(pod) {
pod, err = m.Mutate(ctx, ns, pod)
if err != nil {
res := admission.Errored(http.StatusInternalServerError, err)
res.Allowed = true
return res
}
}
}

Expand Down
25 changes: 24 additions & 1 deletion pkg/instrumentation/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@

package instrumentation

import corev1 "k8s.io/api/core/v1"
import (
"strings"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
)

// Calculate if we already inject InitContainers.
func IsInitContainerMissing(pod corev1.Pod) bool {
Expand All @@ -25,3 +30,21 @@ func IsInitContainerMissing(pod corev1.Pod) bool {
}
return true
}

// Check if opentelemetry-auto-instrumentation volume is on the list.
func IsOtAIVolumeMissing(volumeMounts []corev1.VolumeMount, logger logr.Logger) bool {
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved
for _, volumeMount := range volumeMounts {
if volumeMount.Name == volumeName {
return false
}
}
return true
}

// Check if EnvVar value contains instrumentation string
func IsEnvVarValueInstrumentationMissing(envVar corev1.EnvVar, instrumentation string) bool {
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved
if strings.Contains(envVar.Value, instrumentation) {
return false
}
return true
}
16 changes: 11 additions & 5 deletions pkg/instrumentation/javaagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +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

if IsEnvVarValueInstrumentationMissing(container.Env[idx], javaJVMArgument) {
mat-rumian marked this conversation as resolved.
Show resolved Hide resolved
container.Env[idx].Value = container.Env[idx].Value + javaJVMArgument
}
}

if IsOtAIVolumeMissing(container.VolumeMounts, logger) {
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Name: volumeName,
MountPath: "/otel-auto-instrumentation",
})
}
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) {
Expand Down
16 changes: 11 additions & 5 deletions pkg/instrumentation/nodejs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +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

if IsEnvVarValueInstrumentationMissing(container.Env[idx], nodeRequireArgument) {
container.Env[idx].Value = container.Env[idx].Value + nodeRequireArgument
}
}

if IsOtAIVolumeMissing(container.VolumeMounts, logger) {
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Name: volumeName,
MountPath: "/otel-auto-instrumentation",
})
}
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) {
Expand Down
15 changes: 10 additions & 5 deletions pkg/instrumentation/python.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ 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)

if IsEnvVarValueInstrumentationMissing(container.Env[idx], pythonPathPrefix) {
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.
Expand All @@ -66,10 +69,12 @@ func injectPythonSDK(logger logr.Logger, pythonSpec v1alpha1.Python, pod corev1.
})
}

container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Name: volumeName,
MountPath: "/otel-auto-instrumentation",
})
if IsOtAIVolumeMissing(container.VolumeMounts, logger) {
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) {
Expand Down