feat: APM config via configmap #185
Conversation
# Conflicts: # internal/apm/helper.go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
==========================================
+ Coverage 60.54% 60.88% +0.33%
==========================================
Files 36 36
Lines 2945 2973 +28
==========================================
+ Hits 1783 1810 +27
Misses 1015 1015
- Partials 147 148 +1 ☔ View full report in Codecov by Sentry. |
…igmap' into dbudziwojski/apm-config-via-configmap
internal/apm/java.go
Outdated
| @@ -88,6 +89,35 @@ func (i *JavaInjector) Inject(ctx context.Context, inst v1beta1.Instrumentation, | |||
| container.Env[idx].Value = container.Env[idx].Value + javaJVMArgument | |||
| } | |||
|
|
|||
| if inst.Spec.AgentConfigMap != "" { | |||
| pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ | |||
There was a problem hiding this comment.
We need to check the existence of the same pod your volume your injecting to ensure the volume doesn't get injected multiple times. Being called multiple times should be idempotent.
Each "mutater" has the potential to be called more than once, as different mutators may make changes, which can cause previously called mutators to be called again (no prefined order)
internal/apm/java.go
Outdated
| }, | ||
| }) | ||
|
|
||
| container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ |
| Value: apmConfigPath, | ||
| }) | ||
| } else { | ||
| container.Env[apmIdx].Value = apmConfigPath |
There was a problem hiding this comment.
Is the intention to overwrite the value if it's already configured on the container?
There was a problem hiding this comment.
Fair point. I suppose that if a customer has already provided a file we should not override their choice.
There was a problem hiding this comment.
Does that mean this else block need to be prunned?
…igmap' into dbudziwojski/apm-config-via-configmap
Agreed. This is something we can look into once we begin supporting the other agents. |
Description
Type of change
Checklist: