From b4bfdb87d68ddceb30e8c3d9b8ade99c74c61d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Mon, 17 Jun 2024 13:08:06 +0000 Subject: [PATCH] Support more common attributes in TargetAllocator CRD (#3040) * Support more container attributes from TA CRD * Support more Deployment attributes from TA CRD --- controllers/builder_test.go | 10 +- internal/manifests/collector/daemonset.go | 2 +- internal/manifests/collector/deployment.go | 2 +- internal/manifests/collector/statefulset.go | 2 +- .../utils.go => manifestutils/dns.go} | 9 +- .../manifests/targetallocator/container.go | 18 ++- .../targetallocator/container_test.go | 149 +++++++++++++++++- .../manifests/targetallocator/deployment.go | 22 ++- .../targetallocator/deployment_test.go | 142 +++++++++++++++++ 9 files changed, 331 insertions(+), 25 deletions(-) rename internal/manifests/{collector/utils.go => manifestutils/dns.go} (79%) diff --git a/controllers/builder_test.go b/controllers/builder_test.go index 0396cc7567..d3dadfd3db 100644 --- a/controllers/builder_test.go +++ b/controllers/builder_test.go @@ -1495,8 +1495,9 @@ prometheus_cr: }, }, }, - DNSPolicy: "", - ServiceAccountName: "test-targetallocator", + DNSPolicy: "ClusterFirst", + ShareProcessNamespace: ptr.To(false), + ServiceAccountName: "test-targetallocator", }, }, }, @@ -1888,8 +1889,9 @@ prometheus_cr: }, }, }, - DNSPolicy: "", - ServiceAccountName: "test-targetallocator", + DNSPolicy: "ClusterFirst", + ShareProcessNamespace: ptr.To(false), + ServiceAccountName: "test-targetallocator", }, }, }, diff --git a/internal/manifests/collector/daemonset.go b/internal/manifests/collector/daemonset.go index c006dbd10a..4aaffa30b3 100644 --- a/internal/manifests/collector/daemonset.go +++ b/internal/manifests/collector/daemonset.go @@ -64,7 +64,7 @@ func DaemonSet(params manifests.Params) (*appsv1.DaemonSet, error) { NodeSelector: params.OtelCol.Spec.NodeSelector, HostNetwork: params.OtelCol.Spec.HostNetwork, ShareProcessNamespace: ¶ms.OtelCol.Spec.ShareProcessNamespace, - DNSPolicy: getDNSPolicy(params.OtelCol), + DNSPolicy: manifestutils.GetDNSPolicy(params.OtelCol.Spec.HostNetwork), SecurityContext: params.OtelCol.Spec.PodSecurityContext, PriorityClassName: params.OtelCol.Spec.PriorityClassName, Affinity: params.OtelCol.Spec.Affinity, diff --git a/internal/manifests/collector/deployment.go b/internal/manifests/collector/deployment.go index 1cc105114b..3a7e72fa15 100644 --- a/internal/manifests/collector/deployment.go +++ b/internal/manifests/collector/deployment.go @@ -61,7 +61,7 @@ func Deployment(params manifests.Params) (*appsv1.Deployment, error) { InitContainers: params.OtelCol.Spec.InitContainers, Containers: append(params.OtelCol.Spec.AdditionalContainers, Container(params.Config, params.Log, params.OtelCol, true)), Volumes: Volumes(params.Config, params.OtelCol), - DNSPolicy: getDNSPolicy(params.OtelCol), + DNSPolicy: manifestutils.GetDNSPolicy(params.OtelCol.Spec.HostNetwork), HostNetwork: params.OtelCol.Spec.HostNetwork, ShareProcessNamespace: ¶ms.OtelCol.Spec.ShareProcessNamespace, Tolerations: params.OtelCol.Spec.Tolerations, diff --git a/internal/manifests/collector/statefulset.go b/internal/manifests/collector/statefulset.go index bfb3a70964..bd0ecae892 100644 --- a/internal/manifests/collector/statefulset.go +++ b/internal/manifests/collector/statefulset.go @@ -61,7 +61,7 @@ func StatefulSet(params manifests.Params) (*appsv1.StatefulSet, error) { InitContainers: params.OtelCol.Spec.InitContainers, Containers: append(params.OtelCol.Spec.AdditionalContainers, Container(params.Config, params.Log, params.OtelCol, true)), Volumes: Volumes(params.Config, params.OtelCol), - DNSPolicy: getDNSPolicy(params.OtelCol), + DNSPolicy: manifestutils.GetDNSPolicy(params.OtelCol.Spec.HostNetwork), HostNetwork: params.OtelCol.Spec.HostNetwork, ShareProcessNamespace: ¶ms.OtelCol.Spec.ShareProcessNamespace, Tolerations: params.OtelCol.Spec.Tolerations, diff --git a/internal/manifests/collector/utils.go b/internal/manifests/manifestutils/dns.go similarity index 79% rename from internal/manifests/collector/utils.go rename to internal/manifests/manifestutils/dns.go index da75763f0d..b9b038f315 100644 --- a/internal/manifests/collector/utils.go +++ b/internal/manifests/manifestutils/dns.go @@ -12,17 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -package collector +package manifestutils import ( corev1 "k8s.io/api/core/v1" - - "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) -func getDNSPolicy(otelcol v1beta1.OpenTelemetryCollector) corev1.DNSPolicy { +// Get the Pod DNS Policy depending on whether we're using a host network. +func GetDNSPolicy(hostNetwork bool) corev1.DNSPolicy { dnsPolicy := corev1.DNSClusterFirst - if otelcol.Spec.HostNetwork { + if hostNetwork { dnsPolicy = corev1.DNSClusterFirstWithHostNet } return dnsPolicy diff --git a/internal/manifests/targetallocator/container.go b/internal/manifests/targetallocator/container.go index 802b7cfb50..4409912a76 100644 --- a/internal/manifests/targetallocator/container.go +++ b/internal/manifests/targetallocator/container.go @@ -42,11 +42,20 @@ func Container(cfg config.Config, logger logr.Logger, instance v1alpha1.TargetAl ContainerPort: 8080, Protocol: corev1.ProtocolTCP, }) + for _, p := range instance.Spec.Ports { + ports = append(ports, corev1.ContainerPort{ + Name: p.Name, + ContainerPort: p.Port, + Protocol: p.Protocol, + HostPort: p.HostPort, + }) + } volumeMounts := []corev1.VolumeMount{{ Name: naming.TAConfigMapVolume(), MountPath: "/conf", }} + volumeMounts = append(volumeMounts, instance.Spec.VolumeMounts...) var envVars = instance.Spec.Env if envVars == nil { @@ -123,13 +132,16 @@ func Container(cfg config.Config, logger logr.Logger, instance v1alpha1.TargetAl return corev1.Container{ Name: naming.TAContainer(), Image: image, + ImagePullPolicy: instance.Spec.ImagePullPolicy, Ports: ports, - Env: envVars, VolumeMounts: volumeMounts, - Resources: instance.Spec.Resources, Args: args, + Env: envVars, + EnvFrom: instance.Spec.EnvFrom, + Resources: instance.Spec.Resources, + SecurityContext: instance.Spec.SecurityContext, LivenessProbe: livenessProbe, ReadinessProbe: readinessProbe, - SecurityContext: instance.Spec.SecurityContext, + Lifecycle: instance.Spec.Lifecycle, } } diff --git a/internal/manifests/targetallocator/container_test.go b/internal/manifests/targetallocator/container_test.go index f0473a89ca..6bfcce4eb9 100644 --- a/internal/manifests/targetallocator/container_test.go +++ b/internal/manifests/targetallocator/container_test.go @@ -63,7 +63,7 @@ func TestContainerWithImageOverridden(t *testing.T) { assert.Equal(t, "overridden-image", c.Image) } -func TestContainerPorts(t *testing.T) { +func TestContainerDefaultPorts(t *testing.T) { // prepare targetAllocator := v1alpha1.TargetAllocator{} cfg := config.New() @@ -77,7 +77,7 @@ func TestContainerPorts(t *testing.T) { assert.Equal(t, int32(8080), c.Ports[0].ContainerPort) } -func TestContainerVolumes(t *testing.T) { +func TestContainerDefaultVolumes(t *testing.T) { // prepare targetAllocator := v1alpha1.TargetAllocator{} cfg := config.New() @@ -383,3 +383,148 @@ func TestArgs(t *testing.T) { expected := []string{"--akey=avalue", "--key=value"} assert.Equal(t, expected, c.Args) } + +func TestContainerCustomVolumes(t *testing.T) { + // prepare + targetAllocator := v1alpha1.TargetAllocator{ + Spec: v1alpha1.TargetAllocatorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "custom-volume-mount", + }}, + }, + }, + } + cfg := config.New() + + // test + c := Container(cfg, logger, targetAllocator) + + // verify + assert.Len(t, c.VolumeMounts, 2) + assert.Equal(t, "custom-volume-mount", c.VolumeMounts[1].Name) +} + +func TestContainerCustomPorts(t *testing.T) { + // prepare + targetAllocator := v1alpha1.TargetAllocator{ + Spec: v1alpha1.TargetAllocatorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Ports: []v1beta1.PortsSpec{ + { + ServicePort: corev1.ServicePort{ + Name: "testport1", + Port: 12345, + Protocol: corev1.ProtocolTCP, + }, + HostPort: 54321, + }, + }, + }, + }, + } + cfg := config.New() + + // test + c := Container(cfg, logger, targetAllocator) + + // verify + assert.Len(t, c.Ports, 2) + actual := c.Ports[1] + expected := corev1.ContainerPort{ + Name: "testport1", + ContainerPort: 12345, + Protocol: corev1.ProtocolTCP, + HostPort: 54321, + } + assert.Equal(t, expected, actual) +} + +func TestContainerLifecycle(t *testing.T) { + // prepare + targetAllocator := v1alpha1.TargetAllocator{ + Spec: v1alpha1.TargetAllocatorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + Lifecycle: &corev1.Lifecycle{ + PostStart: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{Command: []string{"sh", "sleep 100"}}, + }, + PreStop: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{Command: []string{"sh", "sleep 300"}}, + }, + }, + }, + }, + } + cfg := config.New() + + // test + c := Container(cfg, logger, targetAllocator) + + expectedLifecycleHooks := corev1.Lifecycle{ + PostStart: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{Command: []string{"sh", "sleep 100"}}, + }, + PreStop: &corev1.LifecycleHandler{ + Exec: &corev1.ExecAction{Command: []string{"sh", "sleep 300"}}, + }, + } + + // verify + assert.Equal(t, expectedLifecycleHooks, *c.Lifecycle) +} + +func TestContainerEnvFrom(t *testing.T) { + //prepare + envFrom1 := corev1.EnvFromSource{ + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "env-as-secret", + }, + }, + } + envFrom2 := corev1.EnvFromSource{ + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "env-as-configmap", + }, + }, + } + // prepare + targetAllocator := v1alpha1.TargetAllocator{ + Spec: v1alpha1.TargetAllocatorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + EnvFrom: []corev1.EnvFromSource{ + envFrom1, + envFrom2, + }, + }, + }, + } + cfg := config.New() + + // test + c := Container(cfg, logger, targetAllocator) + + // verify + assert.Contains(t, c.EnvFrom, envFrom1) + assert.Contains(t, c.EnvFrom, envFrom2) +} + +func TestContainerImagePullPolicy(t *testing.T) { + // prepare + targetAllocator := v1alpha1.TargetAllocator{ + Spec: v1alpha1.TargetAllocatorSpec{ + OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, + } + cfg := config.New() + + // test + c := Container(cfg, logger, targetAllocator) + + // verify + assert.Equal(t, c.ImagePullPolicy, corev1.PullIfNotPresent) +} diff --git a/internal/manifests/targetallocator/deployment.go b/internal/manifests/targetallocator/deployment.go index db36d214fb..fc93b1f4b0 100644 --- a/internal/manifests/targetallocator/deployment.go +++ b/internal/manifests/targetallocator/deployment.go @@ -53,14 +53,20 @@ func Deployment(params manifests.Params) (*appsv1.Deployment, error) { Annotations: annotations, }, Spec: corev1.PodSpec{ - ServiceAccountName: ServiceAccountName(params.TargetAllocator), - Containers: []corev1.Container{Container(params.Config, params.Log, params.TargetAllocator)}, - Volumes: Volumes(params.Config, params.TargetAllocator), - NodeSelector: params.TargetAllocator.Spec.NodeSelector, - Tolerations: params.TargetAllocator.Spec.Tolerations, - TopologySpreadConstraints: params.TargetAllocator.Spec.TopologySpreadConstraints, - Affinity: params.TargetAllocator.Spec.Affinity, - SecurityContext: params.TargetAllocator.Spec.PodSecurityContext, + ServiceAccountName: ServiceAccountName(params.TargetAllocator), + InitContainers: params.TargetAllocator.Spec.InitContainers, + Containers: append(params.TargetAllocator.Spec.AdditionalContainers, Container(params.Config, params.Log, params.TargetAllocator)), + Volumes: Volumes(params.Config, params.TargetAllocator), + DNSPolicy: manifestutils.GetDNSPolicy(params.TargetAllocator.Spec.HostNetwork), + HostNetwork: params.TargetAllocator.Spec.HostNetwork, + ShareProcessNamespace: ¶ms.TargetAllocator.Spec.ShareProcessNamespace, + Tolerations: params.TargetAllocator.Spec.Tolerations, + NodeSelector: params.TargetAllocator.Spec.NodeSelector, + SecurityContext: params.TargetAllocator.Spec.PodSecurityContext, + PriorityClassName: params.TargetAllocator.Spec.PriorityClassName, + Affinity: params.TargetAllocator.Spec.Affinity, + TerminationGracePeriodSeconds: params.TargetAllocator.Spec.TerminationGracePeriodSeconds, + TopologySpreadConstraints: params.TargetAllocator.Spec.TopologySpreadConstraints, }, }, }, diff --git a/internal/manifests/targetallocator/deployment_test.go b/internal/manifests/targetallocator/deployment_test.go index e4d5759f0a..a634ea6d2e 100644 --- a/internal/manifests/targetallocator/deployment_test.go +++ b/internal/manifests/targetallocator/deployment_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" go_yaml "gopkg.in/yaml.v3" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -390,3 +391,144 @@ func TestDeploymentTopologySpreadConstraints(t *testing.T) { assert.NotEmpty(t, d2.Spec.Template.Spec.TopologySpreadConstraints) assert.Equal(t, testTopologySpreadConstraintValue, d2.Spec.Template.Spec.TopologySpreadConstraints) } + +func TestDeploymentSetInitContainer(t *testing.T) { + // prepare + targetAllocator := targetAllocatorInstance() + targetAllocator.Spec.InitContainers = []v1.Container{ + { + Name: "test", + }, + } + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + // test + d, err := Deployment(params) + require.NoError(t, err) + assert.Len(t, d.Spec.Template.Spec.InitContainers, 1) +} + +func TestDeploymentAdditionalContainers(t *testing.T) { + // prepare + targetAllocator := targetAllocatorInstance() + targetAllocator.Spec.AdditionalContainers = []v1.Container{ + { + Name: "test", + }, + } + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + // test + d, err := Deployment(params) + require.NoError(t, err) + assert.Len(t, d.Spec.Template.Spec.Containers, 2) + assert.Equal(t, v1.Container{Name: "test"}, d.Spec.Template.Spec.Containers[0]) +} + +func TestDeploymentHostNetwork(t *testing.T) { + // Test default + targetAllocator := targetAllocatorInstance() + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + d1, err := Deployment(params) + require.NoError(t, err) + + assert.Equal(t, d1.Spec.Template.Spec.HostNetwork, false) + assert.Equal(t, d1.Spec.Template.Spec.DNSPolicy, v1.DNSClusterFirst) + + // Test hostNetwork=true + params.TargetAllocator.Spec.HostNetwork = true + + d2, err := Deployment(params) + require.NoError(t, err) + assert.Equal(t, d2.Spec.Template.Spec.HostNetwork, true) + assert.Equal(t, d2.Spec.Template.Spec.DNSPolicy, v1.DNSClusterFirstWithHostNet) +} + +func TestDeploymentShareProcessNamespace(t *testing.T) { + // Test default + targetAllocator := targetAllocatorInstance() + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + d1, err := Deployment(params) + require.NoError(t, err) + assert.False(t, *d1.Spec.Template.Spec.ShareProcessNamespace) + + // Test ShareProcessNamespace=true + params.TargetAllocator.Spec.ShareProcessNamespace = true + + d2, err := Deployment(params) + require.NoError(t, err) + assert.True(t, *d2.Spec.Template.Spec.ShareProcessNamespace) +} + +func TestDeploymentPriorityClassName(t *testing.T) { + // Test default + targetAllocator := targetAllocatorInstance() + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + d1, err := Deployment(params) + require.NoError(t, err) + assert.Empty(t, d1.Spec.Template.Spec.PriorityClassName) + + // Test PriorityClassName + params.TargetAllocator.Spec.PriorityClassName = "test-class" + + d2, err := Deployment(params) + require.NoError(t, err) + assert.Equal(t, params.TargetAllocator.Spec.PriorityClassName, d2.Spec.Template.Spec.PriorityClassName) +} + +func TestDeploymentTerminationGracePeriodSeconds(t *testing.T) { + // Test default + targetAllocator := targetAllocatorInstance() + otelcol := collectorInstance() + params := manifests.Params{ + OtelCol: otelcol, + TargetAllocator: targetAllocator, + Config: config.New(), + Log: logger, + } + + d1, err := Deployment(params) + require.NoError(t, err) + assert.Nil(t, d1.Spec.Template.Spec.TerminationGracePeriodSeconds) + + // Test TerminationGracePeriodSeconds + gracePeriod := int64(100) + params.TargetAllocator.Spec.TerminationGracePeriodSeconds = &gracePeriod + + d2, err := Deployment(params) + require.NoError(t, err) + assert.Equal(t, gracePeriod, *d2.Spec.Template.Spec.TerminationGracePeriodSeconds) +}