From f7c8e7a17f705d2d8ae1bcca5398f9069a35bedd Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Sun, 13 Nov 2022 10:40:12 -0600 Subject: [PATCH 1/7] Only create SAs if existing SA is not specified --- pkg/collector/reconcile/serviceaccount.go | 19 ++++++++----- .../reconcile/serviceaccount_test.go | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/pkg/collector/reconcile/serviceaccount.go b/pkg/collector/reconcile/serviceaccount.go index 2aa55e50d9..b19c40aa10 100644 --- a/pkg/collector/reconcile/serviceaccount.go +++ b/pkg/collector/reconcile/serviceaccount.go @@ -33,13 +33,7 @@ import ( // ServiceAccounts reconciles the service account(s) required for the instance in the current context. func ServiceAccounts(ctx context.Context, params Params) error { - desired := []corev1.ServiceAccount{} - if params.Instance.Spec.Mode != v1alpha1.ModeSidecar { - desired = append(desired, collector.ServiceAccount(params.Instance)) - } - if params.Instance.Spec.TargetAllocator.Enabled { - desired = append(desired, targetallocator.ServiceAccount(params.Instance)) - } + desired := desiredServiceAccounts(params) // first, handle the create/update parts if err := expectedServiceAccounts(ctx, params, desired); err != nil { @@ -54,6 +48,17 @@ func ServiceAccounts(ctx context.Context, params Params) error { return nil } +func desiredServiceAccounts(params Params) []corev1.ServiceAccount { + desired := []corev1.ServiceAccount{} + if params.Instance.Spec.Mode != v1alpha1.ModeSidecar && len(params.Instance.Spec.ServiceAccount) == 0 { + desired = append(desired, collector.ServiceAccount(params.Instance)) + } + if params.Instance.Spec.TargetAllocator.Enabled && len(params.Instance.Spec.TargetAllocator.ServiceAccount) == 0 { + desired = append(desired, targetallocator.ServiceAccount(params.Instance)) + } + return desired +} + func expectedServiceAccounts(ctx context.Context, params Params, expected []corev1.ServiceAccount) error { for _, obj := range expected { desired := obj diff --git a/pkg/collector/reconcile/serviceaccount_test.go b/pkg/collector/reconcile/serviceaccount_test.go index db310237c0..df6d693971 100644 --- a/pkg/collector/reconcile/serviceaccount_test.go +++ b/pkg/collector/reconcile/serviceaccount_test.go @@ -117,3 +117,30 @@ func TestDeleteServiceAccounts(t *testing.T) { }) } + +func TestDesiredServiceAccounts(t *testing.T) { + t.Run("should not create any service account", func(t *testing.T) { + params := params() + params.Instance.Spec.ServiceAccount = "existing-collector-sa" + params.Instance.Spec.TargetAllocator.Enabled = true + params.Instance.Spec.TargetAllocator.ServiceAccount = "existing-allocator-sa" + desired := desiredServiceAccounts(params) + assert.Len(t, desired, 0) + }) + + t.Run("should create collector service account", func(t *testing.T) { + params := params() + desired := desiredServiceAccounts(params) + assert.Len(t, desired, 1) + assert.Equal(t, collector.ServiceAccount(params.Instance), desired[0]) + }) + + t.Run("should create targetallocator service account", func(t *testing.T) { + params := params() + params.Instance.Spec.ServiceAccount = "existing-collector-sa" + params.Instance.Spec.TargetAllocator.Enabled = true + desired := desiredServiceAccounts(params) + assert.Len(t, desired, 1) + assert.Equal(t, targetallocator.ServiceAccount(params.Instance), desired[0]) + }) +} From 0b523f6723316a93e737c5b91cbea8382d51c6c0 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Mon, 14 Nov 2022 08:22:01 -0600 Subject: [PATCH 2/7] Add note to types about ServiceAccount creation --- apis/v1alpha1/opentelemetrycollector_types.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index d16abab40f..cc57905e58 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -84,7 +84,8 @@ type OpenTelemetryCollectorSpec struct { // Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) // +optional Mode Mode `json:"mode,omitempty"` - // ServiceAccount indicates the name of an existing service account to use with this instance. + // ServiceAccount indicates the name of an existing service account to use with this instance. When set, + // the operator will not automatically create a ServiceAccount for the collector. // +optional ServiceAccount string `json:"serviceAccount,omitempty"` // Image indicates the container image to use for the OpenTelemetry Collector. @@ -164,7 +165,8 @@ type OpenTelemetryTargetAllocator struct { // Filtering is disabled by default. // +optional FilterStrategy string `json:"filterStrategy,omitempty"` - // ServiceAccount indicates the name of an existing service account to use with this instance. + // ServiceAccount indicates the name of an existing service account to use with this instance. When set, + // the operator will not automatically create a ServiceAccount for the TargetAllocator. // +optional ServiceAccount string `json:"serviceAccount,omitempty"` // Image indicates the container image to use for the OpenTelemetry TargetAllocator. From e652e05b3f4b5be555c059a6d4adcce7745fa207 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Mon, 14 Nov 2022 10:12:39 -0600 Subject: [PATCH 3/7] Remove ServiceAccount from TA e2e test assertion, generate to update docs --- .../crd/bases/opentelemetry.io_opentelemetrycollectors.yaml | 6 ++++-- config/manager/kustomization.yaml | 6 ++++++ docs/api.md | 4 ++-- tests/e2e/smoke-targetallocator/00-assert.yaml | 5 ----- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 1196232386..0a209ed2af 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -1683,7 +1683,8 @@ spec: type: object serviceAccount: description: ServiceAccount indicates the name of an existing service - account to use with this instance. + account to use with this instance. When set, the operator will not + automatically create a ServiceAccount for the collector. type: string targetAllocator: description: TargetAllocator indicates a value which determines whether @@ -1731,7 +1732,8 @@ spec: type: integer serviceAccount: description: ServiceAccount indicates the name of an existing - service account to use with this instance. + service account to use with this instance. When set, the operator + will not automatically create a ServiceAccount for the TargetAllocator. type: string type: object tolerations: diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 5c5f0b84cb..46c1861f30 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,2 +1,8 @@ resources: - manager.yaml +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +images: +- name: controller + newName: local/opentelemetry-operator + newTag: e2e diff --git a/docs/api.md b/docs/api.md index 015c87490a..2fd6bbcc88 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1843,7 +1843,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. serviceAccount string - ServiceAccount indicates the name of an existing service account to use with this instance.
+ ServiceAccount indicates the name of an existing service account to use with this instance. When set, the operator will not automatically create a ServiceAccount for the collector.
false @@ -4576,7 +4576,7 @@ TargetAllocator indicates a value which determines whether to spawn a target all serviceAccount string - ServiceAccount indicates the name of an existing service account to use with this instance.
+ ServiceAccount indicates the name of an existing service account to use with this instance. When set, the operator will not automatically create a ServiceAccount for the TargetAllocator.
false diff --git a/tests/e2e/smoke-targetallocator/00-assert.yaml b/tests/e2e/smoke-targetallocator/00-assert.yaml index c180946c0e..355df73c4a 100644 --- a/tests/e2e/smoke-targetallocator/00-assert.yaml +++ b/tests/e2e/smoke-targetallocator/00-assert.yaml @@ -19,11 +19,6 @@ kind: ConfigMap metadata: name: stateful-targetallocator --- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: stateful-targetallocator ---- # Print TA pod logs if test fails apiVersion: kuttl.dev/v1beta1 kind: TestAssert From d48881090093f5021dddbe7a071fc1f797501758 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Mon, 14 Nov 2022 10:46:47 -0600 Subject: [PATCH 4/7] make bundle --- .../opentelemetry-operator.clusterserviceversion.yaml | 6 +++--- .../manifests/opentelemetry.io_opentelemetrycollectors.yaml | 6 ++++-- config/manager/kustomization.yaml | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index 007b9c3b18..e70bca5f41 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -37,7 +37,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: github.com/open-telemetry/opentelemetry-operator support: OpenTelemetry Community - name: opentelemetry-operator.v0.63.1 + name: opentelemetry-operator.v0.63.1-12-ge652e05 namespace: placeholder spec: apiservicedefinitions: {} @@ -294,7 +294,7 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.63.1 + image: ghcr.io/craig.squire/opentelemetry-operator/opentelemetry-operator:0.63.1-12-ge652e05 livenessProbe: httpGet: path: /healthz @@ -401,7 +401,7 @@ spec: maturity: alpha provider: name: OpenTelemetry Community - version: 0.63.1 + version: 0.63.1-12-ge652e05 webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index fcdfed60de..855fbd6e78 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -1685,7 +1685,8 @@ spec: type: object serviceAccount: description: ServiceAccount indicates the name of an existing service - account to use with this instance. + account to use with this instance. When set, the operator will not + automatically create a ServiceAccount for the collector. type: string targetAllocator: description: TargetAllocator indicates a value which determines whether @@ -1733,7 +1734,8 @@ spec: type: integer serviceAccount: description: ServiceAccount indicates the name of an existing - service account to use with this instance. + service account to use with this instance. When set, the operator + will not automatically create a ServiceAccount for the TargetAllocator. type: string type: object tolerations: diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 46c1861f30..cc56cd68cd 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,5 +4,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: local/opentelemetry-operator - newTag: e2e + newName: ghcr.io/craig.squire/opentelemetry-operator/opentelemetry-operator + newTag: 0.63.1-12-ge652e05 From b4bef3e5dbc92285789a2e94f767fdd45a831855 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Tue, 15 Nov 2022 07:12:44 -0600 Subject: [PATCH 5/7] Update bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml Co-authored-by: Ben B. --- .../manifests/opentelemetry-operator.clusterserviceversion.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index e70bca5f41..63fd157bbb 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -294,7 +294,7 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - image: ghcr.io/craig.squire/opentelemetry-operator/opentelemetry-operator:0.63.1-12-ge652e05 + image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.63.1 livenessProbe: httpGet: path: /healthz From 9f0af2ca527552bc5210686cf63fbe62e6fb2d4c Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Tue, 15 Nov 2022 07:13:06 -0600 Subject: [PATCH 6/7] Update bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml Co-authored-by: Ben B. --- .../manifests/opentelemetry-operator.clusterserviceversion.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index 63fd157bbb..38ef806e0c 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -401,7 +401,7 @@ spec: maturity: alpha provider: name: OpenTelemetry Community - version: 0.63.1-12-ge652e05 + version: 0.63.1 webhookdefinitions: - admissionReviewVersions: - v1 From 3a582c24a20e60aae37587696059da71137e05d1 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Tue, 15 Nov 2022 21:52:45 -0600 Subject: [PATCH 7/7] remove other invalid updates from make bundle --- .../opentelemetry-operator.clusterserviceversion.yaml | 2 +- config/manager/kustomization.yaml | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index 38ef806e0c..007b9c3b18 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -37,7 +37,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: github.com/open-telemetry/opentelemetry-operator support: OpenTelemetry Community - name: opentelemetry-operator.v0.63.1-12-ge652e05 + name: opentelemetry-operator.v0.63.1 namespace: placeholder spec: apiservicedefinitions: {} diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index cc56cd68cd..5c5f0b84cb 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,8 +1,2 @@ resources: - manager.yaml -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -images: -- name: controller - newName: ghcr.io/craig.squire/opentelemetry-operator/opentelemetry-operator - newTag: 0.63.1-12-ge652e05