From 1b5a3413dd5e090e73c5e6dca9d4f6e90e406881 Mon Sep 17 00:00:00 2001 From: Darren Wang Date: Tue, 21 Nov 2023 11:18:43 +0800 Subject: [PATCH 1/4] add additional check before creating sa for collector --- .chloggen/sa-fix.yaml | 16 ++++++++++++++++ internal/manifests/collector/collector.go | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100755 .chloggen/sa-fix.yaml diff --git a/.chloggen/sa-fix.yaml b/.chloggen/sa-fix.yaml new file mode 100755 index 0000000000..8b3a74e52d --- /dev/null +++ b/.chloggen/sa-fix.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: check if service account specified in otelcol before creating service account resource for collectors + +# One or more tracking issues related to the change +issues: [2372] + +# (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/internal/manifests/collector/collector.go b/internal/manifests/collector/collector.go index f33f479266..37d958a4d4 100644 --- a/internal/manifests/collector/collector.go +++ b/internal/manifests/collector/collector.go @@ -45,12 +45,14 @@ func Build(params manifests.Params) ([]client.Object, error) { manifestFactories = append(manifestFactories, []manifests.K8sManifestFactory{ manifests.FactoryWithoutError(ConfigMap), manifests.FactoryWithoutError(HorizontalPodAutoscaler), - manifests.FactoryWithoutError(ServiceAccount), manifests.FactoryWithoutError(Service), manifests.FactoryWithoutError(HeadlessService), manifests.FactoryWithoutError(MonitoringService), manifests.FactoryWithoutError(Ingress), }...) + if params.OtelCol.Spec.ServiceAccount == "" { + manifestFactories = append(manifestFactories, manifests.FactoryWithoutError(ServiceAccount)) + } if params.OtelCol.Spec.Observability.Metrics.EnableMetrics && featuregate.PrometheusOperatorIsAvailable.IsEnabled() { manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor)) } From aecd0ed1813e2aa44216951ba17ae2cb31f03384 Mon Sep 17 00:00:00 2001 From: Darren Wang Date: Tue, 21 Nov 2023 11:55:44 +0800 Subject: [PATCH 2/4] update builder test for collector --- controllers/builder_test.go | 60 ++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/controllers/builder_test.go b/controllers/builder_test.go index 7adcc158da..52744a2963 100644 --- a/controllers/builder_test.go +++ b/controllers/builder_test.go @@ -211,21 +211,6 @@ service: "collector.yaml": "receivers:\n examplereceiver:\n endpoint: \"0.0.0.0:12345\"\nservice:\n pipelines:\n metrics:\n receivers: [examplereceiver]\n exporters: [logging]\n", }, }, - &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", - Namespace: "test", - Labels: map[string]string{ - "app.kubernetes.io/component": "opentelemetry-collector", - "app.kubernetes.io/instance": "test.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", - "app.kubernetes.io/part-of": "opentelemetry", - "app.kubernetes.io/version": "latest", - }, - Annotations: nil, - }, - }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test-collector", @@ -304,6 +289,21 @@ service: Selector: selectorLabels, }, }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + }, }, wantErr: false, }, @@ -454,21 +454,6 @@ service: "collector.yaml": "receivers:\n examplereceiver:\n endpoint: \"0.0.0.0:12345\"\nservice:\n pipelines:\n metrics:\n receivers: [examplereceiver]\n exporters: [logging]\n", }, }, - &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", - Namespace: "test", - Labels: map[string]string{ - "app.kubernetes.io/component": "opentelemetry-collector", - "app.kubernetes.io/instance": "test.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", - "app.kubernetes.io/part-of": "opentelemetry", - "app.kubernetes.io/version": "latest", - }, - Annotations: nil, - }, - }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test-collector", @@ -586,6 +571,21 @@ service: }, }, }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + }, }, wantErr: false, }, From 5ab9aaa81ec080c696e9649dee19915e6e9ef226 Mon Sep 17 00:00:00 2001 From: Darren Wang Date: Wed, 22 Nov 2023 10:43:21 +0800 Subject: [PATCH 3/4] change check collector sa specified in sa manifest --- internal/manifests/collector/collector.go | 4 +--- internal/manifests/collector/serviceaccount.go | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/manifests/collector/collector.go b/internal/manifests/collector/collector.go index 37d958a4d4..f33f479266 100644 --- a/internal/manifests/collector/collector.go +++ b/internal/manifests/collector/collector.go @@ -45,14 +45,12 @@ func Build(params manifests.Params) ([]client.Object, error) { manifestFactories = append(manifestFactories, []manifests.K8sManifestFactory{ manifests.FactoryWithoutError(ConfigMap), manifests.FactoryWithoutError(HorizontalPodAutoscaler), + manifests.FactoryWithoutError(ServiceAccount), manifests.FactoryWithoutError(Service), manifests.FactoryWithoutError(HeadlessService), manifests.FactoryWithoutError(MonitoringService), manifests.FactoryWithoutError(Ingress), }...) - if params.OtelCol.Spec.ServiceAccount == "" { - manifestFactories = append(manifestFactories, manifests.FactoryWithoutError(ServiceAccount)) - } if params.OtelCol.Spec.Observability.Metrics.EnableMetrics && featuregate.PrometheusOperatorIsAvailable.IsEnabled() { manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor)) } diff --git a/internal/manifests/collector/serviceaccount.go b/internal/manifests/collector/serviceaccount.go index 19af012444..a3648a593a 100644 --- a/internal/manifests/collector/serviceaccount.go +++ b/internal/manifests/collector/serviceaccount.go @@ -35,6 +35,9 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { // ServiceAccount returns the service account for the given instance. func ServiceAccount(params manifests.Params) *corev1.ServiceAccount { + if params.OtelCol.Spec.ServiceAccount != "" { + return nil + } name := naming.ServiceAccount(params.OtelCol.Name) labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) From 5ef0a20281bf6dffe9f041e0350843fd1d2126ec Mon Sep 17 00:00:00 2001 From: Darren Wang Date: Wed, 22 Nov 2023 10:57:38 +0800 Subject: [PATCH 4/4] add specified account test case to collector building --- controllers/builder_test.go | 254 +++++++++++++++++++++++++++++++++--- 1 file changed, 238 insertions(+), 16 deletions(-) diff --git a/controllers/builder_test.go b/controllers/builder_test.go index 52744a2963..f40b4c0d2e 100644 --- a/controllers/builder_test.go +++ b/controllers/builder_test.go @@ -211,6 +211,21 @@ service: "collector.yaml": "receivers:\n examplereceiver:\n endpoint: \"0.0.0.0:12345\"\nservice:\n pipelines:\n metrics:\n receivers: [examplereceiver]\n exporters: [logging]\n", }, }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test-collector", @@ -289,21 +304,6 @@ service: Selector: selectorLabels, }, }, - &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-collector", - Namespace: "test", - Labels: map[string]string{ - "app.kubernetes.io/component": "opentelemetry-collector", - "app.kubernetes.io/instance": "test.test", - "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/name": "test-collector", - "app.kubernetes.io/part-of": "opentelemetry", - "app.kubernetes.io/version": "latest", - }, - Annotations: nil, - }, - }, }, wantErr: false, }, @@ -454,6 +454,21 @@ service: "collector.yaml": "receivers:\n examplereceiver:\n endpoint: \"0.0.0.0:12345\"\nservice:\n pipelines:\n metrics:\n receivers: [examplereceiver]\n exporters: [logging]\n", }, }, + &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test-collector", @@ -571,7 +586,133 @@ service: }, }, }, - &corev1.ServiceAccount{ + }, + wantErr: false, + }, + { + name: "specified service account case", + args: args{ + instance: v1alpha1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Replicas: &one, + Mode: "deployment", + Image: "test", + Config: goodConfig, + ServiceAccount: "my-special-sa", + }, + }, + }, + want: []client.Object{ + &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: map[string]string{ + "opentelemetry-operator-config/sha256": "baf97852b8beb44fb46a120f8c31873ded3129088e50cd6c69f3208ba60bd661", + "prometheus.io/path": "/metrics", + "prometheus.io/port": "8888", + "prometheus.io/scrape": "true", + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &one, + Selector: &metav1.LabelSelector{ + MatchLabels: selectorLabels, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: map[string]string{ + "opentelemetry-operator-config/sha256": "baf97852b8beb44fb46a120f8c31873ded3129088e50cd6c69f3208ba60bd661", + "prometheus.io/path": "/metrics", + "prometheus.io/port": "8888", + "prometheus.io/scrape": "true", + }, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "otc-internal", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "test-collector", + }, + Items: []corev1.KeyToPath{ + { + Key: "collector.yaml", + Path: "collector.yaml", + }, + }, + }, + }, + }, + }, + Containers: []corev1.Container{ + { + Name: "otc-container", + Image: "test", + Args: []string{ + "--config=/conf/collector.yaml", + }, + Ports: []corev1.ContainerPort{ + { + Name: "examplereceiver", + HostPort: 0, + ContainerPort: 12345, + }, + { + Name: "metrics", + HostPort: 0, + ContainerPort: 8888, + Protocol: "TCP", + }, + }, + Env: []corev1.EnvVar{ + { + Name: "POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "otc-internal", + MountPath: "/conf", + }, + }, + }, + }, + DNSPolicy: "ClusterFirst", + ServiceAccountName: "my-special-sa", + }, + }, + }, + }, + &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "test-collector", Namespace: "test", @@ -585,6 +726,87 @@ service: }, Annotations: nil, }, + Data: map[string]string{ + "collector.yaml": "receivers:\n examplereceiver:\n endpoint: \"0.0.0.0:12345\"\nservice:\n pipelines:\n metrics:\n receivers: [examplereceiver]\n exporters: [logging]\n", + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "examplereceiver", + Port: 12345, + }, + }, + Selector: selectorLabels, + InternalTrafficPolicy: &basePolicy, + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector-headless", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + "operator.opentelemetry.io/collector-headless-service": "Exists", + }, + Annotations: map[string]string{ + "service.beta.openshift.io/serving-cert-secret-name": "test-collector-headless-tls", + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "examplereceiver", + Port: 12345, + }, + }, + Selector: selectorLabels, + InternalTrafficPolicy: &basePolicy, + ClusterIP: "None", + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-collector-monitoring", + Namespace: "test", + Labels: map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "test.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/name": "test-collector-monitoring", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "latest", + }, + Annotations: nil, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "monitoring", + Port: 8888, + }, + }, + Selector: selectorLabels, + }, }, }, wantErr: false,