From 0bfbea9e250899365856173d842bee4b797bc42b Mon Sep 17 00:00:00 2001 From: Martin Divis Date: Wed, 28 Jun 2023 17:08:43 +0200 Subject: [PATCH] Fix for #1820 and #1821, unit tests (#1847) * fix #1820 & #1821 plus unit tests * unit test fix * chloggen * mount point fix --------- Co-authored-by: Pavol Loffay --- .chloggen/1847-fix-issues-1820-1821.yaml | 16 ++ pkg/instrumentation/apachehttpd.go | 27 +++- pkg/instrumentation/apachehttpd_test.go | 184 ++++++++++++++++++++++- pkg/instrumentation/podmutator_test.go | 2 +- pkg/instrumentation/sdk_test.go | 2 +- 5 files changed, 223 insertions(+), 8 deletions(-) create mode 100644 .chloggen/1847-fix-issues-1820-1821.yaml diff --git a/.chloggen/1847-fix-issues-1820-1821.yaml b/.chloggen/1847-fix-issues-1820-1821.yaml new file mode 100644 index 0000000000..7d8efde725 --- /dev/null +++ b/.chloggen/1847-fix-issues-1820-1821.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Fix for #1820 and #1821 plus added covering unit tests." + +# One or more tracking issues related to the change +issues: [1847] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/pkg/instrumentation/apachehttpd.go b/pkg/instrumentation/apachehttpd.go index 30e6f7b75c..c3aafe56d2 100644 --- a/pkg/instrumentation/apachehttpd.go +++ b/pkg/instrumentation/apachehttpd.go @@ -26,7 +26,7 @@ import ( ) const ( - apacheConfigDirectory = "/usr/local/apache2/conf" + apacheDefaultConfigDirectory = "/usr/local/apache2/conf" apacheConfigFile = "httpd.conf" apacheAgentConfigFile = "opentemetry_agent.conf" apacheAgentDirectory = "/opt/opentelemetry-webserver" @@ -81,10 +81,12 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod EmptyDir: &corev1.EmptyDirVolumeSource{}, }}) + apacheConfDir := getApacheConfDir(apacheSpec.ConfigPath) + cloneContainer := container.DeepCopy() cloneContainer.Name = apacheAgentCloneContainerName cloneContainer.Command = []string{"/bin/sh", "-c"} - cloneContainer.Args = []string{"cp -r /usr/local/apache2/conf/* " + apacheAgentConfDirFull} + cloneContainer.Args = []string{"cp -r " + apacheConfDir + "/* " + apacheAgentConfDirFull} cloneContainer.VolumeMounts = append(cloneContainer.VolumeMounts, corev1.VolumeMount{ Name: apacheAgentConfigVolume, MountPath: apacheAgentConfDirFull, @@ -92,6 +94,10 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod // remove resource requirements since those are then reserved for the lifetime of a pod // and we definitely do not need them for the init container for cp command cloneContainer.Resources = apacheSpec.Resources + // remove livenessProbe, readinessProbe, and startupProbe, since not supported on init containers + cloneContainer.LivenessProbe = nil + cloneContainer.ReadinessProbe = nil + cloneContainer.StartupProbe = nil pod.Spec.InitContainers = append(pod.Spec.InitContainers, *cloneContainer) @@ -99,7 +105,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod // since it could over-write configuration provided by the injection idxFound := -1 for idx, volume := range container.VolumeMounts { - if strings.Contains(volume.MountPath, apacheConfigDirectory) { // potentially passes config, which we want to pass to init copy only + if strings.Contains(volume.MountPath, apacheConfDir) { // potentially passes config, which we want to pass to init copy only idxFound = idx break } @@ -117,7 +123,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod }) container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ Name: apacheAgentConfigVolume, - MountPath: apacheConfigDirectory, + MountPath: apacheConfDir, }) } @@ -146,7 +152,7 @@ func injectApacheHttpdagent(_ logr.Logger, apacheSpec v1alpha1.ApacheHttpd, pod "echo \"$" + apacheAttributesEnvVar + "\" > " + apacheAgentConfDirFull + "/" + apacheAgentConfigFile + " && " + "sed -i 's/" + apacheServiceInstanceId + "/'${" + apacheServiceInstanceIdEnvVar + "}'/g' " + apacheAgentConfDirFull + "/" + apacheAgentConfigFile + " && " + // Include a link to include Apache agent configuration file into httpd.conf - "echo 'Include " + apacheConfigDirectory + "/" + apacheAgentConfigFile + "' >> " + apacheAgentConfDirFull + "/" + apacheConfigFile, + "echo 'Include " + getApacheConfDir(apacheSpec.ConfigPath) + "/" + apacheAgentConfigFile + "' >> " + apacheAgentConfDirFull + "/" + apacheConfigFile, }, Env: []corev1.EnvVar{ { @@ -256,3 +262,14 @@ LoadModule otel_apache_module %[1]s/WebServerModule/Apache/libmod_apache_otel%[2 return configFileContent } + +func getApacheConfDir(configuredDir string) string { + apacheConfDir := apacheDefaultConfigDirectory + if configuredDir != "" { + apacheConfDir = configuredDir + if apacheConfDir[len(apacheConfDir)-1] == '/' { + apacheConfDir = apacheConfDir[:len(apacheConfDir)-1] + } + } + return apacheConfDir +} diff --git a/pkg/instrumentation/apachehttpd_test.go b/pkg/instrumentation/apachehttpd_test.go index b9d3782c38..e74d9630b7 100644 --- a/pkg/instrumentation/apachehttpd_test.go +++ b/pkg/instrumentation/apachehttpd_test.go @@ -109,7 +109,7 @@ func TestInjectApacheHttpdagent(t *testing.T) { }, { Name: apacheAgentConfigVolume, - MountPath: apacheConfigDirectory, + MountPath: apacheDefaultConfigDirectory, }, }, }, @@ -117,6 +117,188 @@ func TestInjectApacheHttpdagent(t *testing.T) { }, }, }, + // === Test ConfigPath configuration ============================= + { + name: "ConfigPath honored", + ApacheHttpd: v1alpha1.ApacheHttpd{ + Image: "foo/bar:1", + ConfigPath: "/opt/customPath", + }, + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {}, + }, + }, + }, + expected: corev1.Pod{ + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: apacheAgentConfigVolume, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: apacheAgentVolume, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, + InitContainers: []corev1.Container{ + { + Name: apacheAgentCloneContainerName, + Image: "", + Command: []string{"/bin/sh", "-c"}, + Args: []string{"cp -r /opt/customPath/* " + apacheAgentDirectory + apacheAgentConfigDirectory}, + VolumeMounts: []corev1.VolumeMount{{ + Name: apacheAgentConfigVolume, + MountPath: apacheAgentDirectory + apacheAgentConfigDirectory, + }}, + }, + { + Name: apacheAgentInitContainerName, + Image: "foo/bar:1", + Command: []string{"/bin/sh", "-c"}, + Args: []string{ + "cp -ar /opt/opentelemetry/* /opt/opentelemetry-webserver/agent && export agentLogDir=$(echo \"/opt/opentelemetry-webserver/agent/logs\" | sed 's,/,\\\\/,g') && cat /opt/opentelemetry-webserver/agent/conf/appdynamics_sdk_log4cxx.xml.template | sed 's/__agent_log_dir__/'${agentLogDir}'/g' > /opt/opentelemetry-webserver/agent/conf/appdynamics_sdk_log4cxx.xml &&echo \"$OTEL_APACHE_AGENT_CONF\" > /opt/opentelemetry-webserver/source-conf/opentemetry_agent.conf && sed -i 's/<>/'${APACHE_SERVICE_INSTANCE_ID}'/g' /opt/opentelemetry-webserver/source-conf/opentemetry_agent.conf && echo 'Include /opt/customPath/opentemetry_agent.conf' >> /opt/opentelemetry-webserver/source-conf/httpd.conf"}, + Env: []corev1.EnvVar{ + { + Name: apacheAttributesEnvVar, + Value: "\n#Load the Otel Webserver SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_common.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_resources.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_trace.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_otlp_recordable.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_ostream_span.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_otlp_grpc.so\n#Load the Otel ApacheModule SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_webserver_sdk.so\n#Load the Apache Module. In this example for Apache 2.4\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Load the Apache Module. In this example for Apache 2.2\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel22.so\nLoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Attributes\nApacheModuleEnabled ON\nApacheModuleOtelExporterEndpoint http://otlp-endpoint:4317\nApacheModuleOtelSpanExporter otlp\nApacheModuleResolveBackends ON\nApacheModuleServiceInstanceId <>\nApacheModuleServiceName apache-httpd-service-name\nApacheModuleServiceNamespace \nApacheModuleTraceAsError ON\n", + }, + {Name: apacheServiceInstanceIdEnvVar, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: apacheAgentVolume, + MountPath: apacheAgentDirectory + apacheAgentSubDirectory, + }, + { + Name: apacheAgentConfigVolume, + MountPath: apacheAgentConfDirFull, + }, + }, + }, + }, + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: apacheAgentVolume, + MountPath: apacheAgentDirectory + apacheAgentSubDirectory, + }, + { + Name: apacheAgentConfigVolume, + MountPath: "/opt/customPath", + }, + }, + }, + }, + }, + }, + }, + // === Test Removal of probes ============================= + { + name: "Probes removed on clone init container", + ApacheHttpd: v1alpha1.ApacheHttpd{Image: "foo/bar:1"}, + pod: corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + ReadinessProbe: &corev1.Probe{}, + StartupProbe: &corev1.Probe{}, + LivenessProbe: &corev1.Probe{}, + }, + }, + }, + }, + expected: corev1.Pod{ + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: apacheAgentConfigVolume, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: apacheAgentVolume, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, + InitContainers: []corev1.Container{ + { + Name: apacheAgentCloneContainerName, + Image: "", + Command: []string{"/bin/sh", "-c"}, + Args: []string{"cp -r /usr/local/apache2/conf/* " + apacheAgentDirectory + apacheAgentConfigDirectory}, + VolumeMounts: []corev1.VolumeMount{{ + Name: apacheAgentConfigVolume, + MountPath: apacheAgentDirectory + apacheAgentConfigDirectory, + }}, + }, + { + Name: apacheAgentInitContainerName, + Image: "foo/bar:1", + Command: []string{"/bin/sh", "-c"}, + Args: []string{ + "cp -ar /opt/opentelemetry/* /opt/opentelemetry-webserver/agent && export agentLogDir=$(echo \"/opt/opentelemetry-webserver/agent/logs\" | sed 's,/,\\\\/,g') && cat /opt/opentelemetry-webserver/agent/conf/appdynamics_sdk_log4cxx.xml.template | sed 's/__agent_log_dir__/'${agentLogDir}'/g' > /opt/opentelemetry-webserver/agent/conf/appdynamics_sdk_log4cxx.xml &&echo \"$OTEL_APACHE_AGENT_CONF\" > /opt/opentelemetry-webserver/source-conf/opentemetry_agent.conf && sed -i 's/<>/'${APACHE_SERVICE_INSTANCE_ID}'/g' /opt/opentelemetry-webserver/source-conf/opentemetry_agent.conf && echo 'Include /usr/local/apache2/conf/opentemetry_agent.conf' >> /opt/opentelemetry-webserver/source-conf/httpd.conf"}, + Env: []corev1.EnvVar{ + { + Name: apacheAttributesEnvVar, + Value: "\n#Load the Otel Webserver SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_common.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_resources.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_trace.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_otlp_recordable.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_ostream_span.so\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_exporter_otlp_grpc.so\n#Load the Otel ApacheModule SDK\nLoadFile /opt/opentelemetry-webserver/agent/sdk_lib/lib/libopentelemetry_webserver_sdk.so\n#Load the Apache Module. In this example for Apache 2.4\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Load the Apache Module. In this example for Apache 2.2\n#LoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel22.so\nLoadModule otel_apache_module /opt/opentelemetry-webserver/agent/WebServerModule/Apache/libmod_apache_otel.so\n#Attributes\nApacheModuleEnabled ON\nApacheModuleOtelExporterEndpoint http://otlp-endpoint:4317\nApacheModuleOtelSpanExporter otlp\nApacheModuleResolveBackends ON\nApacheModuleServiceInstanceId <>\nApacheModuleServiceName apache-httpd-service-name\nApacheModuleServiceNamespace \nApacheModuleTraceAsError ON\n", + }, + {Name: apacheServiceInstanceIdEnvVar, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: apacheAgentVolume, + MountPath: apacheAgentDirectory + apacheAgentSubDirectory, + }, + { + Name: apacheAgentConfigVolume, + MountPath: apacheAgentConfDirFull, + }, + }, + }, + }, + Containers: []corev1.Container{ + { + VolumeMounts: []corev1.VolumeMount{ + { + Name: apacheAgentVolume, + MountPath: apacheAgentDirectory + apacheAgentSubDirectory, + }, + { + Name: apacheAgentConfigVolume, + MountPath: apacheDefaultConfigDirectory, + }, + }, + ReadinessProbe: &corev1.Probe{}, + StartupProbe: &corev1.Probe{}, + LivenessProbe: &corev1.Probe{}, + }, + }, + }, + }, + }, } resourceMap := map[string]string{ diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 01e7326269..71a6793810 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -1477,7 +1477,7 @@ func TestMutatePod(t *testing.T) { }, { Name: apacheAgentConfigVolume, - MountPath: apacheConfigDirectory, + MountPath: apacheDefaultConfigDirectory, }, }, Env: []corev1.EnvVar{ diff --git a/pkg/instrumentation/sdk_test.go b/pkg/instrumentation/sdk_test.go index 7d4e73489f..84b0b1e128 100644 --- a/pkg/instrumentation/sdk_test.go +++ b/pkg/instrumentation/sdk_test.go @@ -1314,7 +1314,7 @@ func TestInjectApacheHttpd(t *testing.T) { }, { Name: apacheAgentConfigVolume, - MountPath: apacheConfigDirectory, + MountPath: apacheDefaultConfigDirectory, }, }, Env: []corev1.EnvVar{