diff --git a/internal/backendconnection/otelcolresources/desired_state.go b/internal/backendconnection/otelcolresources/desired_state.go index 46c4762e..97a5dcd6 100644 --- a/internal/backendconnection/otelcolresources/desired_state.go +++ b/internal/backendconnection/otelcolresources/desired_state.go @@ -39,6 +39,13 @@ func (c *oTelColConfig) hasAuthentication() bool { type exportProtocol string const ( + OtlpGrpcHostPort = 40317 + OtlpHttpHostPort = 40318 + // ^ We deliberately do not use the default grpc/http ports as host ports. If there is another OTel collector + // daemonset in the cluster (which is not managed by the operator), it will very likely use the 4317/4318 as host + // ports. When the operator creates its daemonset, the pods of one of the two otelcol daemonsets would fail to start + // due to port conflicts. + grpcExportProtocol exportProtocol = "grpc" httpExportProtocol exportProtocol = "http" rbacApiVersion = "rbac.authorization.k8s.io/v1" @@ -84,13 +91,6 @@ const ( otlpGrpcPort = 4317 otlpHttpPort = 4318 - // We deliberately do not use the default grpc/http ports as host ports. If there is another OTel collector - // daemonset in the cluster (which is not managed by the operator), it will very likely use the 4317/4318 as host - // ports. When the operator creates its daemonset, the pods of one of the two otelcol daemonsets will fail to start - // due to port conflicts. - otlpGrpcHostPort = 40317 - otlpHttpHostPort = 40318 - probesHttpPort = 13133 ) @@ -519,13 +519,13 @@ func daemonSet(config *oTelColConfig) *appsv1.DaemonSet { Name: "otlp", Protocol: corev1.ProtocolTCP, ContainerPort: otlpGrpcPort, - HostPort: int32(otlpGrpcHostPort), + HostPort: int32(OtlpGrpcHostPort), }, { Name: "otlp-http", Protocol: corev1.ProtocolTCP, ContainerPort: otlpHttpPort, - HostPort: int32(otlpHttpHostPort), + HostPort: int32(OtlpHttpHostPort), }, }, Env: env, diff --git a/internal/dash0/webhook/dash0_webhook_test.go b/internal/dash0/webhook/dash0_webhook_test.go index fae8ef09..8cb57a38 100644 --- a/internal/dash0/webhook/dash0_webhook_test.go +++ b/internal/dash0/webhook/dash0_webhook_test.go @@ -196,17 +196,19 @@ var _ = Describe("The Dash0 webhook", func() { { VolumeMounts: 2, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, - Dash0CollectorBaseUrlEnvVarIdx: 2, + NodeIpIdx: 2, + Dash0CollectorBaseUrlEnvVarIdx: 3, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, { VolumeMounts: 3, Dash0VolumeMountIdx: 2, - EnvVars: 4, + EnvVars: 5, NodeOptionsEnvVarIdx: 2, - Dash0CollectorBaseUrlEnvVarIdx: 3, + NodeIpIdx: 3, + Dash0CollectorBaseUrlEnvVarIdx: 4, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, }, @@ -232,18 +234,20 @@ var _ = Describe("The Dash0 webhook", func() { { VolumeMounts: 2, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, NodeOptionsUsesValueFrom: true, - Dash0CollectorBaseUrlEnvVarIdx: 2, + NodeIpIdx: 2, + Dash0CollectorBaseUrlEnvVarIdx: 3, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, { VolumeMounts: 3, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, NodeOptionsValue: "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require something-else --experimental-modules", + NodeIpIdx: 2, Dash0CollectorBaseUrlEnvVarIdx: 0, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, diff --git a/internal/dash0/workloads/workload_modifier.go b/internal/dash0/workloads/workload_modifier.go index 2dbde2ff..64db15e7 100644 --- a/internal/dash0/workloads/workload_modifier.go +++ b/internal/dash0/workloads/workload_modifier.go @@ -17,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/dash0hq/dash0-operator/internal/backendconnection/otelcolresources" "github.com/dash0hq/dash0-operator/internal/dash0/util" ) @@ -32,6 +33,7 @@ const ( envVarNodeOptionsName = "NODE_OPTIONS" envVarNodeOptionsValue = "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry" envVarDash0CollectorBaseUrlName = "DASH0_OTEL_COLLECTOR_BASE_URL" + envVarDash0NodeIp = "DASH0_NODE_IP" ) var ( @@ -241,8 +243,21 @@ func (m *ResourceModifier) addEnvironmentVariables(container *corev1.Container, m.addOrReplaceEnvironmentVariable( container, - envVarDash0CollectorBaseUrlName, - m.instrumentationMetadata.OTelCollectorBaseUrl, + corev1.EnvVar{ + Name: envVarDash0NodeIp, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.hostIP", + }, + }, + }, + ) + m.addOrReplaceEnvironmentVariable( + container, + corev1.EnvVar{ + Name: envVarDash0CollectorBaseUrlName, + Value: fmt.Sprintf("http://$(%s):%d", envVarDash0NodeIp, otelcolresources.OtlpHttpHostPort), + }, ) } @@ -284,22 +299,22 @@ func (m *ResourceModifier) handleNodeOptionsEnvVar( } } -func (m *ResourceModifier) addOrReplaceEnvironmentVariable(container *corev1.Container, name string, value string) { +func (m *ResourceModifier) addOrReplaceEnvironmentVariable(container *corev1.Container, envVar corev1.EnvVar) { if container.Env == nil { container.Env = make([]corev1.EnvVar, 0) } idx := slices.IndexFunc(container.Env, func(c corev1.EnvVar) bool { - return c.Name == name + return c.Name == envVar.Name }) if idx < 0 { - container.Env = append(container.Env, corev1.EnvVar{ - Name: name, - Value: value, - }) - } else { + container.Env = append(container.Env, envVar) + } else if envVar.Value != "" { container.Env[idx].ValueFrom = nil - container.Env[idx].Value = value + container.Env[idx].Value = envVar.Value + } else { + container.Env[idx].Value = "" + container.Env[idx].ValueFrom = envVar.ValueFrom } } @@ -394,6 +409,7 @@ func (m *ResourceModifier) removeMount(container *corev1.Container) { func (m *ResourceModifier) removeEnvironmentVariables(container *corev1.Container) { m.removeNodeOptions(container) + m.removeEnvironmentVariable(container, envVarDash0NodeIp) m.removeEnvironmentVariable(container, envVarDash0CollectorBaseUrlName) } diff --git a/internal/dash0/workloads/workload_modifier_test.go b/internal/dash0/workloads/workload_modifier_test.go index 7475b1c1..cfe619a1 100644 --- a/internal/dash0/workloads/workload_modifier_test.go +++ b/internal/dash0/workloads/workload_modifier_test.go @@ -74,17 +74,19 @@ var _ = Describe("Dash0 Workload Modification", func() { { VolumeMounts: 2, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, - Dash0CollectorBaseUrlEnvVarIdx: 2, + NodeIpIdx: 2, + Dash0CollectorBaseUrlEnvVarIdx: 3, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, { VolumeMounts: 3, Dash0VolumeMountIdx: 2, - EnvVars: 4, + EnvVars: 5, NodeOptionsEnvVarIdx: 2, - Dash0CollectorBaseUrlEnvVarIdx: 3, + NodeIpIdx: 3, + Dash0CollectorBaseUrlEnvVarIdx: 4, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, }, @@ -105,18 +107,20 @@ var _ = Describe("Dash0 Workload Modification", func() { { VolumeMounts: 2, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, NodeOptionsUsesValueFrom: true, - Dash0CollectorBaseUrlEnvVarIdx: 2, + NodeIpIdx: 2, + Dash0CollectorBaseUrlEnvVarIdx: 3, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, { VolumeMounts: 3, Dash0VolumeMountIdx: 1, - EnvVars: 3, + EnvVars: 4, NodeOptionsEnvVarIdx: 1, NodeOptionsValue: "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require something-else --experimental-modules", + NodeIpIdx: 2, Dash0CollectorBaseUrlEnvVarIdx: 0, Dash0CollectorBaseUrlEnvVarExpectedValue: OTelCollectorBaseUrlTest, }, @@ -252,50 +256,6 @@ var _ = Describe("Dash0 Workload Modification", func() { }) }) - Describe("when updating instrumentation from 0.5.1 to 0.6.0", func() { - It("should remove the old --require from NODE_OPTIONS (when it is the only content of NODE_OPTIONS)", func() { - workload := InstrumentedDeployment(TestNamespaceName, DeploymentNamePrefix) - Expect(workload.Spec.Template.Spec.Containers[0].Env[0].Name).To(Equal("NODE_OPTIONS")) - workload.Spec.Template.Spec.Containers[0].Env[0].Value = "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0hq/opentelemetry" - hasBeenModified := workloadModifier.ModifyDeployment(workload) - Expect(hasBeenModified).To(BeTrue()) - VerifyModifiedDeployment(workload, BasicInstrumentedPodSpecExpectations()) - }) - - It("should remove the old --require from NODE_OPTIONS (when it is at the start of NODE_OPTIONS)", func() { - workload := InstrumentedDeployment(TestNamespaceName, DeploymentNamePrefix) - Expect(workload.Spec.Template.Spec.Containers[0].Env[0].Name).To(Equal("NODE_OPTIONS")) - workload.Spec.Template.Spec.Containers[0].Env[0].Value = "--require /opt/dash0/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require /something/else" - hasBeenModified := workloadModifier.ModifyDeployment(workload) - Expect(hasBeenModified).To(BeTrue()) - expectations := BasicInstrumentedPodSpecExpectations() - expectations.Containers[0].NodeOptionsValue = "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require /something/else" - VerifyModifiedDeployment(workload, expectations) - }) - - It("should remove the old --require from NODE_OPTIONS (when it is at the end of NODE_OPTIONS)", func() { - workload := InstrumentedDeployment(TestNamespaceName, DeploymentNamePrefix) - Expect(workload.Spec.Template.Spec.Containers[0].Env[0].Name).To(Equal("NODE_OPTIONS")) - workload.Spec.Template.Spec.Containers[0].Env[0].Value = "--require /something/else --require /opt/dash0/instrumentation/node.js/node_modules/@dash0hq/opentelemetry" - hasBeenModified := workloadModifier.ModifyDeployment(workload) - Expect(hasBeenModified).To(BeTrue()) - expectations := BasicInstrumentedPodSpecExpectations() - expectations.Containers[0].NodeOptionsValue = "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require /something/else" - VerifyModifiedDeployment(workload, expectations) - }) - - It("should remove the old --require from NODE_OPTIONS (when it is in the middle of NODE_OPTIONS)", func() { - workload := InstrumentedDeployment(TestNamespaceName, DeploymentNamePrefix) - Expect(workload.Spec.Template.Spec.Containers[0].Env[0].Name).To(Equal("NODE_OPTIONS")) - workload.Spec.Template.Spec.Containers[0].Env[0].Value = "--require /something/else --require /opt/dash0/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require /another/thing" - hasBeenModified := workloadModifier.ModifyDeployment(workload) - Expect(hasBeenModified).To(BeTrue()) - expectations := BasicInstrumentedPodSpecExpectations() - expectations.Containers[0].NodeOptionsValue = "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry --require /something/else --require /another/thing" - VerifyModifiedDeployment(workload, expectations) - }) - }) - Describe("when reverting workloads", func() { It("should remove Dash0 from an instrumented cron job", func() { workload := InstrumentedCronJob(TestNamespaceName, CronJobNamePrefix) @@ -337,6 +297,7 @@ var _ = Describe("Dash0 Workload Modification", func() { Dash0VolumeMountIdx: -1, EnvVars: 1, NodeOptionsEnvVarIdx: -1, + NodeIpIdx: -1, Dash0CollectorBaseUrlEnvVarIdx: -1, }, { @@ -344,6 +305,7 @@ var _ = Describe("Dash0 Workload Modification", func() { Dash0VolumeMountIdx: -1, EnvVars: 2, NodeOptionsEnvVarIdx: -1, + NodeIpIdx: -1, Dash0CollectorBaseUrlEnvVarIdx: -1, }, }, diff --git a/test/util/constants.go b/test/util/constants.go index 59853f5a..defc606d 100644 --- a/test/util/constants.go +++ b/test/util/constants.go @@ -28,7 +28,7 @@ const ( ConfigurationReloaderImageTest = "some-registry.com:1234/dash0hq/configuration-reloader:10.11.12" FilelogOffsetSynchImageTest = "some-registry.com:1234/dash0hq/filelog-offset-synch:13.14.15" - OTelCollectorBaseUrlTest = "http://dash0-operator-opentelemetry-collector.dash0-system.svc.cluster.local:4318" + OTelCollectorBaseUrlTest = "http://$(DASH0_NODE_IP):40318" EndpointTest = "endpoint.dash0.com:4317" AuthorizationTokenTest = "authorization-token" SecretRefTest = "secret-ref" diff --git a/test/util/resources.go b/test/util/resources.go index 62642525..873f538e 100644 --- a/test/util/resources.go +++ b/test/util/resources.go @@ -704,6 +704,10 @@ func DeploymentWithExistingDash0Artifacts(namespace string, name string) *appsv1 Name: "NODE_OPTIONS", ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"}}, }, + { + Name: "DASH0_NODE_IP", + ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "wrong.field.ref"}}, + }, { // this ValueFrom will be removed and replaced by a simple Value Name: "DASH0_OTEL_COLLECTOR_BASE_URL", @@ -738,7 +742,11 @@ func DeploymentWithExistingDash0Artifacts(namespace string, name string) *appsv1 Value: "--require something-else --experimental-modules", }, { - Name: "TEST2", + Name: "DASH0_NODE_IP", + Value: "will be replaced by a value from", + }, + { + Name: "TEST3", Value: "value", }, }, @@ -817,6 +825,10 @@ func InstrumentedDeploymentWithMoreBellsAndWhistles(namespace string, name strin Name: "NODE_OPTIONS", Value: "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry", }, + { + Name: "DASH0_NODE_IP", + ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "status.hostIP"}}, + }, { Name: "DASH0_OTEL_COLLECTOR_BASE_URL", Value: OTelCollectorBaseUrlTest, @@ -892,6 +904,10 @@ func simulateInstrumentedPodSpec(podSpec *corev1.PodSpec, meta *metav1.ObjectMet Name: "NODE_OPTIONS", Value: "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry", }, + { + Name: "DASH0_NODE_IP", + ValueFrom: &corev1.EnvVarSource{FieldRef: &corev1.ObjectFieldSelector{FieldPath: "status.hostIP"}}, + }, { Name: "DASH0_OTEL_COLLECTOR_BASE_URL", Value: OTelCollectorBaseUrlTest, diff --git a/test/util/verification.go b/test/util/verification.go index f57515e4..59bf574f 100644 --- a/test/util/verification.go +++ b/test/util/verification.go @@ -26,6 +26,7 @@ type ContainerExpectations struct { NodeOptionsEnvVarIdx int NodeOptionsValue string NodeOptionsUsesValueFrom bool + NodeIpIdx int Dash0CollectorBaseUrlEnvVarIdx int Dash0CollectorBaseUrlEnvVarExpectedValue string } @@ -51,9 +52,10 @@ func BasicInstrumentedPodSpecExpectations() PodSpecExpectations { Containers: []ContainerExpectations{{ VolumeMounts: 1, Dash0VolumeMountIdx: 0, - EnvVars: 2, + EnvVars: 3, NodeOptionsEnvVarIdx: 0, - Dash0CollectorBaseUrlEnvVarIdx: 1, + NodeIpIdx: 1, + Dash0CollectorBaseUrlEnvVarIdx: 2, Dash0CollectorBaseUrlEnvVarExpectedValue:// OTelCollectorBaseUrlTest, }}, @@ -243,17 +245,17 @@ func verifyPodSpec(podSpec corev1.PodSpec, expectations PodSpecExpectations) { Expect(container.Name).To(Equal(fmt.Sprintf("test-container-%d", i))) containerExpectations := expectations.Containers[i] Expect(container.VolumeMounts).To(HaveLen(containerExpectations.VolumeMounts)) - for i, volumeMount := range container.VolumeMounts { - if i == containerExpectations.Dash0VolumeMountIdx { + for j, volumeMount := range container.VolumeMounts { + if j == containerExpectations.Dash0VolumeMountIdx { Expect(volumeMount.Name).To(Equal("dash0-instrumentation")) Expect(volumeMount.MountPath).To(Equal("/__dash0__")) } else { - Expect(volumeMount.Name).To(Equal(fmt.Sprintf("test-volume-%d", i))) + Expect(volumeMount.Name).To(Equal(fmt.Sprintf("test-volume-%d", j))) } } Expect(container.Env).To(HaveLen(containerExpectations.EnvVars)) - for i, envVar := range container.Env { - if i == containerExpectations.NodeOptionsEnvVarIdx { + for j, envVar := range container.Env { + if j == containerExpectations.NodeOptionsEnvVarIdx { Expect(envVar.Name).To(Equal("NODE_OPTIONS")) if containerExpectations.NodeOptionsUsesValueFrom { Expect(envVar.Value).To(BeEmpty()) @@ -265,12 +267,19 @@ func verifyPodSpec(podSpec corev1.PodSpec, expectations PodSpecExpectations) { "--require /__dash0__/instrumentation/node.js/node_modules/@dash0hq/opentelemetry", )) } - } else if i == containerExpectations.Dash0CollectorBaseUrlEnvVarIdx { + } else if j == containerExpectations.NodeIpIdx { + Expect(envVar.Name).To(Equal("DASH0_NODE_IP")) + valueFrom := envVar.ValueFrom + Expect(valueFrom).ToNot(BeNil()) + Expect(valueFrom.FieldRef).ToNot(BeNil()) + Expect(valueFrom.FieldRef.FieldPath).To(Equal("status.hostIP")) + Expect(envVar.Value).To(BeEmpty()) + } else if j == containerExpectations.Dash0CollectorBaseUrlEnvVarIdx { Expect(envVar.Name).To(Equal("DASH0_OTEL_COLLECTOR_BASE_URL")) Expect(envVar.Value).To(Equal(containerExpectations.Dash0CollectorBaseUrlEnvVarExpectedValue)) Expect(envVar.ValueFrom).To(BeNil()) } else { - Expect(envVar.Name).To(Equal(fmt.Sprintf("TEST%d", i))) + Expect(envVar.Name).To(Equal(fmt.Sprintf("TEST%d", j))) } } }