Fix Pod Mutation loop #953
Conversation
pkg/instrumentation/javaagent.go
Outdated
| } | ||
| } | ||
|
|
||
| if IsOtAIVolumeMissing(container.VolumeMounts) { |
There was a problem hiding this comment.
Why is this check needed?
The IsAutoInstrumentationInjected is not enough?
There was a problem hiding this comment.
So IsOtAIVolumeMissing and IsEnvVarValueInstrumentationMissing were applied before I've added check for pod mutation. When Pod was mutated few times k8s was throwing errors like duplicated volume name. In case of env var it happened that e.g. javaJVMArgument was added few times to JAVA_TOOL_OPTIONS. So I left this checks for safety for other unexpected mutations.
There was a problem hiding this comment.
I would prefer to remove this and IsEnvVarValueInstrumentationMissing as well and rely only on the init container for simplicity.
We should keep it here only if there is a known scenario which the init container does not cover
| 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 { |
There was a problem hiding this comment.
nit: this function is almost the same as isInitContainerMissing
* 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
Functionality changes:
initContainerexists)PR fixes #940