Adding webhook validation and improving comments based on PR feedback

open-telemetry · Aug 9, 2023 · a19f9a5 · a19f9a5
1 parent 8de5619
commit a19f9a5
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 24 deletions.
diff --git a/.chloggen/1987-adding-support-for-sidecar-containers.yaml b/.chloggen/1987-adding-support-for-sidecar-containers.yaml
@@ -5,7 +5,7 @@ change_type: enhancement
 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 deployments of the collector
+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]

diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go
@@ -214,10 +214,10 @@ type OpenTelemetryCollectorSpec struct {
 	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, logging shipping sidecars, agents for shipping
+	// 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 deployments of the collector. It does not apply to the sidecar
-	// method. More info:
+	// 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/
 	//
 	// Container names managed by the operator:

diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go
@@ -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)

diff --git a/apis/v1alpha1/opentelemetrycollector_webhook_test.go b/apis/v1alpha1/opentelemetrycollector_webhook_test.go
@@ -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 {

diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml
@@ -31,19 +31,7 @@ metadata:
     categories: Logging & Tracing
     certified: "false"
     containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
-<<<<<<< HEAD
-<<<<<<< HEAD
-<<<<<<< HEAD
-    createdAt: "2023-08-08T13:26:23Z"
-=======
-    createdAt: "2023-08-04T17:33:49Z"
->>>>>>> 24a9586 (Adding additional container specs to support sidecars and other containers in the collector pod)
-=======
-    createdAt: "2023-08-04T20:52:54Z"
->>>>>>> f4f0bf8 (Addressing code review comments: Improving the godoc comment, adding changelog file)
-=======
-    createdAt: "2023-08-08T21:23:14Z"
->>>>>>> 960c0ca (Adding in an assertion that the additional container equals input)
+    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

diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml
@@ -66,11 +66,12 @@ spec:
               additionalContainers:
                 description: "AdditionalContainers allows injecting additional containers
                   into the Collector's pod definition. These sidecar containers can
-                  be used for authentication proxies, logging shipping sidecars, agents
+                  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 deployments of the collector.
-                  It does not apply to the sidecar method. More info: https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/
+                  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/
                   \n Container names managed by the operator: * `otc-container` \n
                   Overriding containers managed by the operator is outside the scope
                   of what the maintainers will support and by doing so, you wil accept

diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml
@@ -63,11 +63,12 @@ spec:
               additionalContainers:
                 description: "AdditionalContainers allows injecting additional containers
                   into the Collector's pod definition. These sidecar containers can
-                  be used for authentication proxies, logging shipping sidecars, agents
+                  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 deployments of the collector.
-                  It does not apply to the sidecar method. More info: https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/
+                  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/
                   \n Container names managed by the operator: * `otc-container` \n
                   Overriding containers managed by the operator is outside the scope
                   of what the maintainers will support and by doing so, you wil accept

diff --git 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: ghcr.io/matt/opentelemetry-operator/opentelemetry-operator
+  newTag: 0.82.0-9-g6f4246c
diff --git a/docs/api.md b/docs/api.md
@@ -3615,7 +3615,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
         <td><b><a href="#opentelemetrycollectorspecadditionalcontainersindex">additionalContainers</a></b></td>
         <td>[]object</td>
         <td>
-          AdditionalContainers allows injecting additional containers into the Collector's pod definition. These sidecar containers can be used for authentication proxies, logging 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 deployments of the collector. It does not apply to the sidecar method. More info: https://kubernetes.io/docs/tasks/configure-pod-container/share-process-namespace/ 
+          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/ 
  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.<br/>
         </td>