Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only create ServiceAccounts if existing ServiceAccount is not specified #1246

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions apis/v1alpha1/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1843,7 +1843,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
<td><b>serviceAccount</b></td>
<td>string</td>
<td>
ServiceAccount indicates the name of an existing service account to use with this instance.<br/>
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.<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -4576,7 +4576,7 @@ TargetAllocator indicates a value which determines whether to spawn a target all
<td><b>serviceAccount</b></td>
<td>string</td>
<td>
ServiceAccount indicates the name of an existing service account to use with this instance.<br/>
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.<br/>
</td>
<td>false</td>
</tr></tbody>
Expand Down
19 changes: 12 additions & 7 deletions pkg/collector/reconcile/serviceaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
27 changes: 27 additions & 0 deletions pkg/collector/reconcile/serviceaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
})
}
5 changes: 0 additions & 5 deletions tests/e2e/smoke-targetallocator/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down