Skip to content

Commit

Permalink
fix: add env var DASH0_OTEL_COLLECTOR_BASE_URL to containers
Browse files Browse the repository at this point in the history 
This makes sure we report to the correct collector service.
  • Loading branch information
@basti1302
basti1302 committed May 22, 2024
1 parent a8f49ae commit 36492a0
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 76 deletions.
2 changes: 1 addition & 1 deletion internal/controller/dash0_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (r *Dash0Reconciler) modifyExistingResources(ctx context.Context, dash0Cust
}, &deployment); err != nil {
return fmt.Errorf("error when fetching deployment %s/%s: %w", deployment.GetNamespace(), deployment.GetName(), err)
}
hasBeenModified := k8sresources.ModifyPodSpec(&deployment.Spec.Template.Spec, logger)
hasBeenModified := k8sresources.ModifyPodSpec(&deployment.Spec.Template.Spec, deployment.GetNamespace(), logger)
if hasBeenModified {
return r.Client.Update(ctx, &deployment)
} else {
Expand Down
10 changes: 6 additions & 4 deletions internal/controller/dash0_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,12 @@ var _ = Describe("Dash0 Controller", func() {
InitContainers: 1,
Dash0InitContainerIdx: 0,
Containers: []ContainerExpectations{{
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 1,
NodeOptionsEnvVarIdx: 0,
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 2,
NodeOptionsEnvVarIdx: 0,
Dash0CollectorBaseUrlEnvVarIdx: 1,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
}},
})
VerifySuccessEvent(ctx, clientset, namespace, deploymentName, "controller")
Expand Down
40 changes: 33 additions & 7 deletions internal/k8sresources/modify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ const (
dash0InstrumentationDirectory = "/opt/dash0/instrumentation"
// envVarLdPreloadName = "LD_PRELOAD"
// envVarLdPreloadValue = "/opt/dash0/preload/inject.so"
envVarNodeOptionsName = "NODE_OPTIONS"
envVarNodeOptionsValue = "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js"
envVarNodeOptionsName = "NODE_OPTIONS"
envVarNodeOptionsValue = "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js"
envVarDash0CollectorBaseUrlName = "DASH0_OTEL_COLLECTOR_BASE_URL"
envVarDash0CollectorBaseUrlNameValueTemplate = "http://dash0-opentelemetry-collector-daemonset.%s.svc.cluster.local:4318"
)

var (
Expand All @@ -36,13 +38,13 @@ var (
initContainerReadOnlyRootFilesystem = true
)

func ModifyPodSpec(podSpec *corev1.PodSpec, logger logr.Logger) bool {
func ModifyPodSpec(podSpec *corev1.PodSpec, namespace string, logger logr.Logger) bool {
originalSpec := podSpec.DeepCopy()
addInstrumentationVolume(podSpec)
addInitContainer(podSpec)
for idx := range podSpec.Containers {
container := &podSpec.Containers[idx]
instrumentContainer(container, logger)
instrumentContainer(container, namespace, logger)
}
return !reflect.DeepEqual(originalSpec, podSpec)
}
Expand Down Expand Up @@ -129,10 +131,10 @@ func createInitContainer(podSpec *corev1.PodSpec) *corev1.Container {
}
}

func instrumentContainer(container *corev1.Container, logger logr.Logger) {
func instrumentContainer(container *corev1.Container, namespace string, logger logr.Logger) {
logger = logger.WithValues("container", container.Name)
addMount(container)
addEnvironmentVariables(container, logger)
addEnvironmentVariables(container, namespace, logger)
}

func addMount(container *corev1.Container) {
Expand All @@ -154,9 +156,14 @@ func addMount(container *corev1.Container) {
}
}

func addEnvironmentVariables(container *corev1.Container, logger logr.Logger) {
func addEnvironmentVariables(container *corev1.Container, namespace string, logger logr.Logger) {
// For now, we directly modify NODE_OPTIONS. Consider migrating to an LD_PRELOAD hook at some point.
addOrPrependToEnvironmentVariable(container, envVarNodeOptionsName, envVarNodeOptionsValue, logger)

addOrReplaceEnvironmentVariable(
container,
envVarDash0CollectorBaseUrlName,
fmt.Sprintf(envVarDash0CollectorBaseUrlNameValueTemplate, namespace))
}

func addOrPrependToEnvironmentVariable(container *corev1.Container, name string, value string, logger logr.Logger) {
Expand Down Expand Up @@ -186,3 +193,22 @@ func addOrPrependToEnvironmentVariable(container *corev1.Container, name string,
container.Env[idx].Value = fmt.Sprintf("%s %s", value, previousValue)
}
}

func addOrReplaceEnvironmentVariable(container *corev1.Container, name string, value string) {
if container.Env == nil {
container.Env = make([]corev1.EnvVar, 0)
}
idx := slices.IndexFunc(container.Env, func(c corev1.EnvVar) bool {
return c.Name == name
})

if idx < 0 {
container.Env = append(container.Env, corev1.EnvVar{
Name: name,
Value: value,
})
} else {
container.Env[idx].ValueFrom = nil
container.Env[idx].Value = value
}
}
62 changes: 36 additions & 26 deletions internal/k8sresources/modify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import (
// intentional. However, this test should be used for more fine-grained test cases, while dash0_webhook_test.go should
// be used to verify external effects (recording events etc.) that cannot be covered in this test.

var _ = Describe("Dash0 Webhook", func() {
var _ = Describe("Dash0 Resource Modification", func() {

ctx := context.Background()

Context("when mutating new deployments", func() {
It("should inject Dash into a new basic deployment", func() {
deployment := BasicDeployment(TestNamespaceName, DeploymentName)
result := ModifyPodSpec(&deployment.Spec.Template.Spec, log.FromContext(ctx))
result := ModifyPodSpec(&deployment.Spec.Template.Spec, TestNamespaceName, log.FromContext(ctx))

Expect(result).To(BeTrue())
VerifyModifiedDeployment(deployment, DeploymentExpectations{
Expand All @@ -34,17 +34,19 @@ var _ = Describe("Dash0 Webhook", func() {
InitContainers: 1,
Dash0InitContainerIdx: 0,
Containers: []ContainerExpectations{{
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 1,
NodeOptionsEnvVarIdx: 0,
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 2,
NodeOptionsEnvVarIdx: 0,
Dash0CollectorBaseUrlEnvVarIdx: 1,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
}},
})
})

It("should inject Dash into a new deployment that has multiple Containers, and already has Volumes and init Containers", func() {
deployment := DeploymentWithMoreBellsAndWhistles(TestNamespaceName, DeploymentName)
result := ModifyPodSpec(&deployment.Spec.Template.Spec, log.FromContext(ctx))
result := ModifyPodSpec(&deployment.Spec.Template.Spec, TestNamespaceName, log.FromContext(ctx))

Expect(result).To(BeTrue())
VerifyModifiedDeployment(deployment, DeploymentExpectations{
Expand All @@ -54,24 +56,28 @@ var _ = Describe("Dash0 Webhook", func() {
Dash0InitContainerIdx: 2,
Containers: []ContainerExpectations{
{
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 2,
NodeOptionsEnvVarIdx: 1,
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
Dash0CollectorBaseUrlEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
{
VolumeMounts: 3,
Dash0VolumeMountIdx: 2,
EnvVars: 3,
NodeOptionsEnvVarIdx: 2,
VolumeMounts: 3,
Dash0VolumeMountIdx: 2,
EnvVars: 4,
NodeOptionsEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarIdx: 3,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
},
})
})

It("should update existing Dash artifacts in a new deployment", func() {
deployment := DeploymentWithExistingDash0Artifacts(TestNamespaceName, DeploymentName)
result := ModifyPodSpec(&deployment.Spec.Template.Spec, log.FromContext(ctx))
result := ModifyPodSpec(&deployment.Spec.Template.Spec, TestNamespaceName, log.FromContext(ctx))

Expect(result).To(BeTrue())
VerifyModifiedDeployment(deployment, DeploymentExpectations{
Expand All @@ -81,18 +87,22 @@ var _ = Describe("Dash0 Webhook", func() {
Dash0InitContainerIdx: 1,
Containers: []ContainerExpectations{
{
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 2,
NodeOptionsEnvVarIdx: 1,
NodeOptionsUsesValueFrom: true,
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsUsesValueFrom: true,
Dash0CollectorBaseUrlEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
{
VolumeMounts: 3,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsValue: "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js --require something-else --experimental-modules",
VolumeMounts: 3,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsValue: "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js --require something-else --experimental-modules",
Dash0CollectorBaseUrlEnvVarIdx: 0,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
},
})
Expand Down
2 changes: 1 addition & 1 deletion internal/webhook/dash0_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (h *Handler) Handle(_ context.Context, request admission.Request) admission
return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error while parsing the resource: %w", err))
}

hasBeenModified := k8sresources.ModifyPodSpec(&deployment.Spec.Template.Spec, logger)
hasBeenModified := k8sresources.ModifyPodSpec(&deployment.Spec.Template.Spec, request.Namespace, logger)
if !hasBeenModified {
return admission.Allowed("no changes")
}
Expand Down
54 changes: 32 additions & 22 deletions internal/webhook/dash0_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ var _ = Describe("Dash0 Webhook", func() {
InitContainers: 1,
Dash0InitContainerIdx: 0,
Containers: []ContainerExpectations{{
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 1,
NodeOptionsEnvVarIdx: 0,
VolumeMounts: 1,
Dash0VolumeMountIdx: 0,
EnvVars: 2,
NodeOptionsEnvVarIdx: 0,
Dash0CollectorBaseUrlEnvVarIdx: 1,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
}},
})
})
Expand All @@ -51,16 +53,20 @@ var _ = Describe("Dash0 Webhook", func() {
Dash0InitContainerIdx: 2,
Containers: []ContainerExpectations{
{
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 2,
NodeOptionsEnvVarIdx: 1,
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
Dash0CollectorBaseUrlEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
{
VolumeMounts: 3,
Dash0VolumeMountIdx: 2,
EnvVars: 3,
NodeOptionsEnvVarIdx: 2,
VolumeMounts: 3,
Dash0VolumeMountIdx: 2,
EnvVars: 4,
NodeOptionsEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarIdx: 3,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
},
})
Expand All @@ -79,18 +85,22 @@ var _ = Describe("Dash0 Webhook", func() {
Dash0InitContainerIdx: 1,
Containers: []ContainerExpectations{
{
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 2,
NodeOptionsEnvVarIdx: 1,
NodeOptionsUsesValueFrom: true,
VolumeMounts: 2,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsUsesValueFrom: true,
Dash0CollectorBaseUrlEnvVarIdx: 2,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
{
VolumeMounts: 3,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsValue: "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js --require something-else --experimental-modules",
VolumeMounts: 3,
Dash0VolumeMountIdx: 1,
EnvVars: 3,
NodeOptionsEnvVarIdx: 1,
NodeOptionsValue: "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0/opentelemetry/src/index.js --require something-else --experimental-modules",
Dash0CollectorBaseUrlEnvVarIdx: 0,
Dash0CollectorBaseUrlEnvVarExpectedValue: "http://dash0-opentelemetry-collector-daemonset.test-namespace.svc.cluster.local:4318",
},
},
})
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/e2e_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ func RunMultipleFromStrings(cmdsAsStrings [][]string, logCommandArgs ...bool) er
return RunMultiple(cmds, logCommandArgs...)
}

func EnsureNamespaceExists(namespace string) bool {
err := RunAndIgnoreOutput(exec.Command("kubectl", "get", "ns", namespace), false)
if err != nil {
By(fmt.Sprintf("creating namespace %s", namespace))
ExpectWithOffset(1,
RunAndIgnoreOutput(exec.Command("kubectl", "create", "ns", namespace))).To(Succeed())
return true
}
return false
}

// InstallCertManager installs the cert manager bundle.
func InstallCertManager() error {
url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion)
Expand Down
12 changes: 5 additions & 7 deletions test/e2e/e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ const (
operatorNamespace = "dash0-operator-system"
operatorImage = "dash0-operator-controller:latest"

applicationUnderTestNamespace = "default"
applicationUnderTestNamespace = "application-under-test-namespace"

managerYaml = "config/manager/manager.yaml"
managerYamlBackup = managerYaml + ".backup"
)

var (
applicationNamespaceHasBeenCreated = false

originalKubeContext string
managerYamlNeedsRevert bool

Expand Down Expand Up @@ -88,11 +90,7 @@ var _ = Describe("Dash0 Kubernetes Operator", Ordered, func() {
ExpectWithOffset(1, InstallCertManager()).To(Succeed())
}

if applicationUnderTestNamespace != "default" {
By("creating namespace for application under test")
ExpectWithOffset(1,
RunAndIgnoreOutput(exec.Command("kubectl", "create", "ns", applicationUnderTestNamespace))).To(Succeed())
}
applicationNamespaceHasBeenCreated = EnsureNamespaceExists(applicationUnderTestNamespace)

By("installing the collector")
ExpectWithOffset(1, ReinstallCollectorAndClearExportedTelemetry(applicationUnderTestNamespace)).To(Succeed())
Expand Down Expand Up @@ -131,7 +129,7 @@ var _ = Describe("Dash0 Kubernetes Operator", Ordered, func() {
By("removing manager namespace")
_ = RunAndIgnoreOutput(exec.Command("kubectl", "delete", "ns", operatorNamespace))

if applicationUnderTestNamespace != "default" {
if applicationNamespaceHasBeenCreated && applicationUnderTestNamespace != "default" {
By("removing namespace for application under test")
_ = RunAndIgnoreOutput(exec.Command("kubectl", "delete", "ns", applicationUnderTestNamespace))
}
Expand Down
9 changes: 7 additions & 2 deletions test/util/resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,11 @@ func DeploymentWithExistingDash0Artifacts(namespace string, name string) *appsv1
Name: "NODE_OPTIONS",
ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"}},
},
{
// this ValueFrom will be removed and replaced by a simple Value
Name: "DASH0_OTEL_COLLECTOR_BASE_URL",
ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"}},
},
},
},
{
Expand All @@ -271,8 +276,8 @@ func DeploymentWithExistingDash0Artifacts(namespace string, name string) *appsv1
},
Env: []corev1.EnvVar{
{
Name: "TEST0",
Value: "value",
Name: "DASH0_OTEL_COLLECTOR_BASE_URL",
Value: "base url will be replaced",
},
{
Name: "NODE_OPTIONS",
Expand Down
Loading

0 comments on commit 36492a0

Please sign in to comment.