Skip to content

Commit

Permalink
fix: re-introduce collector service url as fallback for ipv6
Browse files Browse the repository at this point in the history
  • Loading branch information
@basti1302
basti1302 committed Oct 30, 2024
1 parent 52f386c commit 41700e5
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 77 deletions.
40 changes: 26 additions & 14 deletions cmd/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,11 @@ func startDash0Controllers(
os.Exit(1)
}

oTelCollectorBaseUrl :=
fmt.Sprintf(
"http://%s-opentelemetry-collector-service.%s.svc.cluster.local:4318",
envVars.oTelCollectorNamePrefix,
envVars.operatorNamespace)
images := util.Images{
OperatorImage: envVars.operatorImage,
InitContainerImage: envVars.initContainerImage,
Expand All @@ -486,6 +491,7 @@ func startDash0Controllers(
mgr.GetEventRecorderFor("dash0-startup-tasks"),
operatorConfiguration,
images,
oTelCollectorBaseUrl,
isIPv6Cluster,
&setupLog,
)
Expand All @@ -494,11 +500,12 @@ func startDash0Controllers(

k8sClient := mgr.GetClient()
instrumenter := &instrumentation.Instrumenter{
Client: k8sClient,
Clientset: clientset,
Recorder: mgr.GetEventRecorderFor("dash0-monitoring-controller"),
Images: images,
IsIPv6Cluster: isIPv6Cluster,
Client: k8sClient,
Clientset: clientset,
Recorder: mgr.GetEventRecorderFor("dash0-monitoring-controller"),
Images: images,
OTelCollectorBaseUrl: oTelCollectorBaseUrl,
IsIPv6Cluster: isIPv6Cluster,
}
oTelColResourceManager := &otelcolresources.OTelColResourceManager{
Client: k8sClient,
Expand Down Expand Up @@ -590,10 +597,11 @@ func startDash0Controllers(
)

if err := (&webhooks.InstrumentationWebhookHandler{
Client: k8sClient,
Recorder: mgr.GetEventRecorderFor("dash0-instrumentation-webhook"),
Images: images,
IsIPv6Cluster: isIPv6Cluster,
Client: k8sClient,
Recorder: mgr.GetEventRecorderFor("dash0-instrumentation-webhook"),
Images: images,
OTelCollectorBaseUrl: oTelCollectorBaseUrl,
IsIPv6Cluster: isIPv6Cluster,
}).SetupWebhookWithManager(mgr); err != nil {
return fmt.Errorf("unable to create the instrumentation webhook: %w", err)
}
Expand Down Expand Up @@ -656,6 +664,7 @@ func executeStartupTasks(
eventRecorder record.EventRecorder,
operatorConfiguration *startup.OperatorConfigurationValues,
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) {
Expand All @@ -671,6 +680,7 @@ func executeStartupTasks(
clientset,
eventRecorder,
images,
oTelCollectorBaseUrl,
isIPv6Cluster,
)
}
Expand All @@ -681,14 +691,16 @@ func instrumentAtStartup(
clientset *kubernetes.Clientset,
eventRecorder record.EventRecorder,
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
) {
startupInstrumenter := &instrumentation.Instrumenter{
Client: startupTasksK8sClient,
Clientset: clientset,
Recorder: eventRecorder,
Images: images,
IsIPv6Cluster: isIPv6Cluster,
Client: startupTasksK8sClient,
Clientset: clientset,
Recorder: eventRecorder,
Images: images,
OTelCollectorBaseUrl: oTelCollectorBaseUrl,
IsIPv6Cluster: isIPv6Cluster,
}

// Trigger an unconditional apply/update of instrumentation for all workloads in Dash0-enabled namespaces, according
Expand Down
11 changes: 6 additions & 5 deletions internal/controller/monitoring_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ var _ = Describe("The monitoring resource controller", Ordered, func() {
createdObjects = make([]client.Object, 0)

instrumenter := &instrumentation.Instrumenter{
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
IsIPv6Cluster: false,
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
OTelCollectorBaseUrl: OTelCollectorBaseUrlTest,
IsIPv6Cluster: false,
}
oTelColResourceManager := &otelcolresources.OTelColResourceManager{
Client: k8sClient,
Expand Down
34 changes: 22 additions & 12 deletions internal/instrumentation/instrumentable_workloads.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ type instrumentableWorkload interface {
getKind() string
asRuntimeObject() runtime.Object
asClientObject() client.Object
instrument(images util.Images, isIPv6Cluster bool, logger *logr.Logger) bool
instrument(images util.Images, oTelCollectorBaseUrl string, isIPv6Cluster bool, logger *logr.Logger) bool
// Strictly speaking, for reverting we do not need the images nor the isIPv6Cluster setting, but for symmetry with
// the instrument method and to make sure any WorkloadModifier instance we create actually has valid values, the
// revert method accepts them as arguments as well.
revert(images util.Images, isIPv6Cluster bool, logger *logr.Logger) bool
revert(images util.Images, oTelCollectorBaseUrl string, isIPv6Cluster bool, logger *logr.Logger) bool
}

type cronJobWorkload struct {
Expand All @@ -36,17 +36,19 @@ func (w *cronJobWorkload) asRuntimeObject() runtime.Object { return w.cronJob
func (w *cronJobWorkload) asClientObject() client.Object { return w.cronJob }
func (w *cronJobWorkload) instrument(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).ModifyCronJob(w.cronJob)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).ModifyCronJob(w.cronJob)
}
func (w *cronJobWorkload) revert(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).RevertCronJob(w.cronJob)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).RevertCronJob(w.cronJob)
}

type daemonSetWorkload struct {
Expand All @@ -59,17 +61,19 @@ func (w *daemonSetWorkload) asRuntimeObject() runtime.Object { return w.daemon
func (w *daemonSetWorkload) asClientObject() client.Object { return w.daemonSet }
func (w *daemonSetWorkload) instrument(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).ModifyDaemonSet(w.daemonSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).ModifyDaemonSet(w.daemonSet)
}
func (w *daemonSetWorkload) revert(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).RevertDaemonSet(w.daemonSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).RevertDaemonSet(w.daemonSet)
}

type deploymentWorkload struct {
Expand All @@ -82,17 +86,19 @@ func (w *deploymentWorkload) asRuntimeObject() runtime.Object { return w.deplo
func (w *deploymentWorkload) asClientObject() client.Object { return w.deployment }
func (w *deploymentWorkload) instrument(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).ModifyDeployment(w.deployment)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).ModifyDeployment(w.deployment)
}
func (w *deploymentWorkload) revert(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).RevertDeployment(w.deployment)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).RevertDeployment(w.deployment)
}

type replicaSetWorkload struct {
Expand All @@ -105,17 +111,19 @@ func (w *replicaSetWorkload) asRuntimeObject() runtime.Object { return w.repli
func (w *replicaSetWorkload) asClientObject() client.Object { return w.replicaSet }
func (w *replicaSetWorkload) instrument(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).ModifyReplicaSet(w.replicaSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).ModifyReplicaSet(w.replicaSet)
}
func (w *replicaSetWorkload) revert(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).RevertReplicaSet(w.replicaSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).RevertReplicaSet(w.replicaSet)
}

type statefulSetWorkload struct {
Expand All @@ -128,15 +136,17 @@ func (w *statefulSetWorkload) asRuntimeObject() runtime.Object { return w.stat
func (w *statefulSetWorkload) asClientObject() client.Object { return w.statefulSet }
func (w *statefulSetWorkload) instrument(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).ModifyStatefulSet(w.statefulSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).ModifyStatefulSet(w.statefulSet)
}
func (w *statefulSetWorkload) revert(
images util.Images,
oTelCollectorBaseUrl string,
isIPv6Cluster bool,
logger *logr.Logger,
) bool {
return newWorkloadModifier(images, isIPv6Cluster, logger).RevertStatefulSet(w.statefulSet)
return newWorkloadModifier(images, oTelCollectorBaseUrl, isIPv6Cluster, logger).RevertStatefulSet(w.statefulSet)
}
30 changes: 16 additions & 14 deletions internal/instrumentation/instrumenter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ import (

type Instrumenter struct {
client.Client
Clientset *kubernetes.Clientset
Recorder record.EventRecorder
Images util.Images
IsIPv6Cluster bool
Clientset *kubernetes.Clientset
Recorder record.EventRecorder
Images util.Images
OTelCollectorBaseUrl string
IsIPv6Cluster bool
}

type ImmutableWorkloadError struct {
Expand Down Expand Up @@ -356,9 +357,9 @@ func (i *Instrumenter) handleJobJobOnInstrumentation(
hasBeenModified := false
switch requiredAction {
case util.ModificationModeInstrumentation:
hasBeenModified = newWorkloadModifier(i.Images, i.IsIPv6Cluster, &logger).AddLabelsToImmutableJob(&job)
hasBeenModified = newWorkloadModifier(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger).AddLabelsToImmutableJob(&job)
case util.ModificationModeUninstrumentation:
hasBeenModified = newWorkloadModifier(i.Images, i.IsIPv6Cluster, &logger).RemoveLabelsFromImmutableJob(&job)
hasBeenModified = newWorkloadModifier(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger).RemoveLabelsFromImmutableJob(&job)
}

if hasBeenModified {
Expand Down Expand Up @@ -497,9 +498,9 @@ func (i *Instrumenter) instrumentWorkload(

switch requiredAction {
case util.ModificationModeInstrumentation:
hasBeenModified = workload.instrument(i.Images, i.IsIPv6Cluster, &logger)
hasBeenModified = workload.instrument(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger)
case util.ModificationModeUninstrumentation:
hasBeenModified = workload.revert(i.Images, i.IsIPv6Cluster, &logger)
hasBeenModified = workload.revert(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger)
}

if hasBeenModified {
Expand Down Expand Up @@ -726,7 +727,7 @@ func (i *Instrumenter) handleJobOnUninstrumentation(ctx context.Context, job bat
} else if util.InstrumentationAttemptHasFailed(&job.ObjectMeta) {
// There was an attempt to instrument this job (probably by the controller), which has not been successful.
// We only need remove the labels from that instrumentation attempt to clean up.
newWorkloadModifier(i.Images, i.IsIPv6Cluster, &logger).RemoveLabelsFromImmutableJob(&job)
newWorkloadModifier(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger).RemoveLabelsFromImmutableJob(&job)

// Apparently for jobs we do not need to set the "dash0.com/webhook-ignore-once" label, since changing their
// labels does not trigger a new admission request.
Expand Down Expand Up @@ -846,7 +847,7 @@ func (i *Instrumenter) revertWorkloadInstrumentation(
err,
)
}
hasBeenModified = workload.revert(i.Images, i.IsIPv6Cluster, &logger)
hasBeenModified = workload.revert(i.Images, i.OTelCollectorBaseUrl, i.IsIPv6Cluster, &logger)
if hasBeenModified {
// Changing the workload spec sometimes triggers a new admission request, which would re-instrument the
// workload via the webhook immediately. To prevent this, we add a label that the webhook can check to
Expand Down Expand Up @@ -888,12 +889,13 @@ func (i *Instrumenter) postProcessUninstrumentation(
}
}

func newWorkloadModifier(images util.Images, isIPv6Cluster bool, logger *logr.Logger) *workloads.ResourceModifier {
func newWorkloadModifier(images util.Images, oTelCollectorBaseUrl string, isIPv6Cluster bool, logger *logr.Logger) *workloads.ResourceModifier {
return workloads.NewResourceModifier(
util.InstrumentationMetadata{
Images: images,
InstrumentedBy: "controller",
IsIPv6Cluster: isIPv6Cluster,
Images: images,
InstrumentedBy: "controller",
OTelCollectorBaseUrl: oTelCollectorBaseUrl,
IsIPv6Cluster: isIPv6Cluster,
},
logger,
)
Expand Down
11 changes: 6 additions & 5 deletions internal/instrumentation/instrumenter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ var _ = Describe("The instrumenter", Ordered, func() {
createdObjects = make([]client.Object, 0)

instrumenter = &Instrumenter{
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
IsIPv6Cluster: false,
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
OTelCollectorBaseUrl: OTelCollectorBaseUrlTest,
IsIPv6Cluster: false,
}
})

Expand Down
11 changes: 6 additions & 5 deletions internal/predelete/pre_delete_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ var _ = BeforeSuite(func() {
Expect(mgr).NotTo(BeNil())

instrumenter := &instrumentation.Instrumenter{
Client: k8sClient,
Clientset: clientset,
Recorder: mgr.GetEventRecorderFor("dash0-monitoring-controller"),
Images: TestImages,
IsIPv6Cluster: false,
Client: k8sClient,
Clientset: clientset,
Recorder: mgr.GetEventRecorderFor("dash0-monitoring-controller"),
Images: TestImages,
OTelCollectorBaseUrl: OTelCollectorBaseUrlTest,
IsIPv6Cluster: false,
}
oTelColResourceManager := &otelcolresources.OTelColResourceManager{
Client: k8sClient,
Expand Down
5 changes: 3 additions & 2 deletions internal/util/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ func getImageVersion(image string) string {

type InstrumentationMetadata struct {
Images
InstrumentedBy string
IsIPv6Cluster bool
OTelCollectorBaseUrl string
IsIPv6Cluster bool
InstrumentedBy string
}

type ModificationMode string
Expand Down
11 changes: 6 additions & 5 deletions internal/webhooks/attach_dangling_events_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ var _ = Describe("The Dash0 webhook and the Dash0 controller", Ordered, func() {

recorder := manager.GetEventRecorderFor("dash0-monitoring-controller")
instrumenter := &instrumentation.Instrumenter{
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
IsIPv6Cluster: false,
Client: k8sClient,
Clientset: clientset,
Recorder: recorder,
Images: TestImages,
OTelCollectorBaseUrl: OTelCollectorBaseUrlTest,
IsIPv6Cluster: false,
}
oTelColResourceManager := &otelcolresources.OTelColResourceManager{
Client: k8sClient,
Expand Down
Loading

0 comments on commit 41700e5

Please sign in to comment.