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

Adding Additional Containers to the Collector spec #1980

Merged
merged 6 commits into from
Aug 10, 2023
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
16 changes: 16 additions & 0 deletions .chloggen/1987-adding-support-for-sidecar-containers.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# 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: Add `AdditionalContainers` to the collector spec allowing to configure sidecar containers. This only applies to Deployment/StatefulSet/DeamonSet deployment modes of the collector.

# One or more tracking issues related to the change
issues: [1987]

# (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:
16 changes: 16 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,22 @@ type OpenTelemetryCollectorSpec struct {
// +optional
InitContainers []v1.Container `json:"initContainers,omitempty"`

// AdditionalContainers allows injecting additional containers into the Collector's pod definition.
// These sidecar containers can be used for authentication proxies, log shipping sidecars, agents for shipping
// metrics to their cloud, or in general sidecars that do not support automatic injection. This option only
// applies to Deployment, DaemonSet, and StatefulSet deployment modes of the collector. It does not apply to the sidecar
// deployment mode. More info about sidecars:
// https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there is this link?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though it would be helpful to link to more information about sidecars. I can remove it if we don't feel its useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth keeping, I appreciate doclinks

//
// Container names managed by the operator:
// * `otc-container`
//
// Overriding containers managed by the operator is outside the scope of what the maintainers will support and by
// doing so, you wil accept the risk of it breaking things.
//
// +optional
AdditionalContainers []v1.Container `json:"additionalContainers,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we proceed with this PR we should clarify how this is supported e.g. https://github.com/prometheus-operator/prometheus-operator/blob/main/pkg/apis/monitoring/v1/prometheus_types.go#L337

We also should clarify if it is supported for all deployment modes (e.g. deployment, sidecar...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the godoc comment, I added in an explanation, link to additional information about sidecars, and I added a warning about overriding the container that the operator generates. Let me know if this is good enough or if I need to improve it more.


// ObservabilitySpec defines how telemetry data gets handled.
//
// +optional
Expand Down
4 changes: 4 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'affinity'", r.Spec.Mode)
}

if r.Spec.Mode == ModeSidecar && len(r.Spec.AdditionalContainers) > 0 {
return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'AdditionalContainers'", r.Spec.Mode)
}

// validate target allocation
if r.Spec.TargetAllocator.Enabled && r.Spec.Mode != ModeStatefulSet {
return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode)
Expand Down
14 changes: 14 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,20 @@ func TestOTELColValidatingWebhook(t *testing.T) {
},
expectedErr: "the OpenTelemetry Spec LivenessProbe TerminationGracePeriodSeconds configuration is incorrect",
},
{
name: "invalid AdditionalContainers",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeSidecar,
AdditionalContainers: []v1.Container{
{
Name: "test",
},
},
},
},
expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'AdditionalContainers'",
},
}

for _, test := range tests {
Expand Down
7 changes: 7 additions & 0 deletions apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ metadata:
categories: Logging & Tracing
certified: "false"
containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
createdAt: "2023-08-08T13:26:23Z"
createdAt: "2023-08-09T22:53:11Z"
description: Provides the OpenTelemetry components, including the Collector
operators.operatorframework.io/builder: operator-sdk-v1.29.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
Expand Down
1,280 changes: 1,280 additions & 0 deletions bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml

Large diffs are not rendered by default.

1,280 changes: 1,280 additions & 0 deletions config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
resources:
- manager.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: ghcr.io/matt/opentelemetry-operator/opentelemetry-operator
newTag: 0.82.0-9-g6f4246c
Loading
Loading