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
Merged

Fix Pod Mutation loop #953

Show file tree
Hide file tree
Changes from all 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
12 changes: 11 additions & 1 deletion pkg/instrumentation/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,21 @@ 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
}
}
return true
}

// Checks if Pod is already instrumented by checking Instrumentation InitContainer presence.
func isAutoInstrumentationInjected(pod corev1.Pod) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this function is almost the same as isInitContainerMissing

for _, cont := range pod.Spec.InitContainers {
if cont.Name == initContainerName {
return true
}
}
return false
}
54 changes: 53 additions & 1 deletion pkg/instrumentation/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/instrumentation/javaagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 4 additions & 1 deletion pkg/instrumentation/nodejs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
6 changes: 6 additions & 0 deletions pkg/instrumentation/podmutator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion pkg/instrumentation/python.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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{
Expand Down