Add managementState field to collector crd spec (#1888)

* adds managementState field to collector crd spec * changes e2e test case * cover new case in e2e test case * fix typo * fix e2e test cases * fix e2e test case * add assertion on config map state
open-telemetry · Aug 2, 2023 · 1c7b296 · 1c7b296
1 parent c8fabb5
commit 1c7b296
Show file tree

Hide file tree

Showing 14 changed files with 418 additions and 8 deletions.
diff --git a/.chloggen/1881-add-managementstate-field-to-collector-spec.yaml b/.chloggen/1881-add-managementstate-field-to-collector-spec.yaml
@@ -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 a new field called `managementState` in the OpenTelemetry Collector CRD.
+
+# One or more tracking issues related to the change
+issues: [1881]
+
+# (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/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go
@@ -21,6 +21,21 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// ManagementStateType defines the type for CR management states.
+//
+// +kubebuilder:validation:Enum=Managed;Unmanaged
+type ManagementStateType string
+
+const (
+	// ManagementStateManaged when the OpenTelemetryCollector custom resource should be
+	// reconciled by the operator.
+	ManagementStateManaged ManagementStateType = "Managed"
+
+	// ManagementStateUnmanaged when the OpenTelemetryCollector custom resource should not be
+	// reconciled by the operator.
+	ManagementStateUnmanaged ManagementStateType = "Unmanaged"
+)
+
 // Ingress is used to specify how OpenTelemetry Collector is exposed. This
 // functionality is only available if one of the valid modes is set.
 // Valid modes are: deployment, daemonset and statefulset.
@@ -69,6 +84,13 @@ type OpenShiftRoute struct {
 
 // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
 type OpenTelemetryCollectorSpec struct {
+	// ManagementState defines if the CR should be managed by the operator or not.
+	// Default is managed.
+	//
+	// +required
+	// +kubebuilder:validation:Required
+	// +kubebuilder:default:=Managed
+	ManagementState ManagementStateType `json:"managementState,omitempty"`
 	// Resources to set on the OpenTelemetry Collector pods.
 	// +optional
 	Resources v1.ResourceRequirements `json:"resources,omitempty"`
@@ -332,6 +354,7 @@ type OpenTelemetryCollectorStatus struct {
 // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.scale.statusReplicas"
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
 // +kubebuilder:printcolumn:name="Image",type="string",JSONPath=".status.image"
+// +kubebuilder:printcolumn:name="Management",type="string",JSONPath=".spec.managementState",description="Management State"
 // +operator-sdk:csv:customresourcedefinitions:displayName="OpenTelemetry Collector"
 // This annotation provides a hint for OLM which resources are managed by OpenTelemetryCollector kind.
 // It's not mandatory to list all resources.

diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml
@@ -38,6 +38,10 @@ spec:
     - jsonPath: .status.image
       name: Image
       type: string
+    - description: Management State
+      jsonPath: .spec.managementState
+      name: Management
+      type: string
     name: v1alpha1
     schema:
       openAPIV3Schema:
@@ -2921,6 +2925,14 @@ spec:
                     format: int32
                     type: integer
                 type: object
+              managementState:
+                default: Managed
+                description: ManagementState defines if the CR should be managed by
+                  the operator or not. Default is managed.
+                enum:
+                - Managed
+                - Unmanaged
+                type: string
               maxReplicas:
                 description: 'MaxReplicas sets an upper bound to the autoscaling feature.
                   If MaxReplicas is set autoscaling is enabled. Deprecated: use "OpenTelemetryCollector.Spec.Autoscaler.MaxReplicas"

diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml
@@ -35,6 +35,10 @@ spec:
     - jsonPath: .status.image
       name: Image
       type: string
+    - description: Management State
+      jsonPath: .spec.managementState
+      name: Management
+      type: string
     name: v1alpha1
     schema:
       openAPIV3Schema:
@@ -2918,6 +2922,14 @@ spec:
                     format: int32
                     type: integer
                 type: object
+              managementState:
+                default: Managed
+                description: ManagementState defines if the CR should be managed by
+                  the operator or not. Default is managed.
+                enum:
+                - Managed
+                - Unmanaged
+                type: string
               maxReplicas:
                 description: 'MaxReplicas sets an upper bound to the autoscaling feature.
                   If MaxReplicas is set autoscaling is enabled. Deprecated: use "OpenTelemetryCollector.Spec.Autoscaler.MaxReplicas"

diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go
@@ -206,6 +206,12 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if instance.Spec.ManagementState != v1alpha1.ManagementStateManaged {
+		log.Info("Skipping reconciliation for unmanaged OpenTelemetryCollector resource", "name", req.String())
+		// Stop requeueing for unmanaged OpenTelemetryCollector custom resources
+		return ctrl.Result{}, nil
+	}
+
 	params := reconcile.Params{
 		Config:   r.config,
 		Client:   r.Client,

diff --git a/docs/api.md b/docs/api.md
@@ -3702,6 +3702,16 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
           Liveness config for the OpenTelemetry Collector except the probe handler which is auto generated from the health extension of the collector. It is only effective when healthcheckextension is configured in the OpenTelemetry Collector pipeline.<br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b>managementState</b></td>
+        <td>enum</td>
+        <td>
+          ManagementState defines if the CR should be managed by the operator or not. Default is managed.<br/>
+          <br/>
+            <i>Enum</i>: Managed, Unmanaged<br/>
+            <i>Default</i>: Managed<br/>
+        </td>
+        <td>false</td>
       </tr><tr>
         <td><b>maxReplicas</b></td>
         <td>integer</td>

diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go
@@ -55,6 +55,12 @@ func (u VersionUpgrade) ManagedInstances(ctx context.Context) error {
 	for i := range list.Items {
 		original := list.Items[i]
 		itemLogger := u.Log.WithValues("name", original.Name, "namespace", original.Namespace)
+
+		if original.Spec.ManagementState == v1alpha1.ManagementStateUnmanaged {
+			itemLogger.Info("skipping upgrade because instance is not managed")
+			continue
+		}
+
 		if original.Spec.UpgradeStrategy == v1alpha1.UpgradeStrategyNone {
 			itemLogger.Info("skipping instance upgrade due to UpgradeStrategy")
 			continue

diff --git a/pkg/collector/upgrade/upgrade_test.go b/pkg/collector/upgrade/upgrade_test.go
@@ -48,7 +48,7 @@ func TestShouldUpgradeAllToLatestBasedOnUpgradeStrategy(t *testing.T) {
 		t.Run("spec.UpgradeStrategy = "+string(tt.strategy), func(t *testing.T) {
 			// prepare
 			nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
-			existing := makeOtelcol(nsn)
+			existing := makeOtelcol(nsn, v1alpha1.ManagementStateManaged)
 			err := k8sClient.Create(context.Background(), &existing)
 			require.NoError(t, err)
 
@@ -95,7 +95,7 @@ func TestUpgradeUpToLatestKnownVersion(t *testing.T) {
 		t.Run(tt.desc, func(t *testing.T) {
 			// prepare
 			nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
-			existing := makeOtelcol(nsn)
+			existing := makeOtelcol(nsn, v1alpha1.ManagementStateManaged)
 			existing.Status.Version = tt.v
 
 			currentV := version.Get()
@@ -117,20 +117,23 @@ func TestUpgradeUpToLatestKnownVersion(t *testing.T) {
 }
 
 func TestVersionsShouldNotBeChanged(t *testing.T) {
+	nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
 	for _, tt := range []struct {
 		desc            string
 		v               string
 		expectedV       string
 		failureExpected bool
+		managementState v1alpha1.ManagementStateType
 	}{
-		{"new-instance", "", "", false},
-		{"newer-than-our-newest", "100.0.0", "100.0.0", false},
-		{"unparseable", "unparseable", "unparseable", true},
+		{"new-instance", "", "", false, v1alpha1.ManagementStateManaged},
+		{"newer-than-our-newest", "100.0.0", "100.0.0", false, v1alpha1.ManagementStateManaged},
+		{"unparseable", "unparseable", "unparseable", true, v1alpha1.ManagementStateManaged},
+		// Ignore unmanaged instances
+		{"unmanaged-instance", "1.0.0", "1.0.0", false, v1alpha1.ManagementStateUnmanaged},
 	} {
 		t.Run(tt.desc, func(t *testing.T) {
 			// prepare
-			nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
-			existing := makeOtelcol(nsn)
+			existing := makeOtelcol(nsn, tt.managementState)
 			existing.Status.Version = tt.v
 
 			currentV := version.Get()
@@ -157,8 +160,11 @@ func TestVersionsShouldNotBeChanged(t *testing.T) {
 	}
 }
 
-func makeOtelcol(nsn types.NamespacedName) v1alpha1.OpenTelemetryCollector {
+func makeOtelcol(nsn types.NamespacedName, managementState v1alpha1.ManagementStateType) v1alpha1.OpenTelemetryCollector {
 	return v1alpha1.OpenTelemetryCollector{
+		Spec: v1alpha1.OpenTelemetryCollectorSpec{
+			ManagementState: managementState,
+		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      nsn.Name,
 			Namespace: nsn.Namespace,

diff --git a/tests/e2e/managed-reconcile/00-assert.yaml b/tests/e2e/managed-reconcile/00-assert.yaml
@@ -0,0 +1,58 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simplest-collector
+status:
+  readyReplicas: 1
+spec:
+  template:
+    spec:
+      serviceAccountName: simplest-collector
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector-headless
+spec:
+  ports:
+  - appProtocol: grpc
+    name: otlp-grpc
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - appProtocol: http
+    name: otlp-http
+    port: 4318
+    protocol: TCP
+    targetPort: 4318
+  - appProtocol: http
+    name: otlp-http-legacy
+    port: 55681
+    protocol: TCP
+    targetPort: 4318
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector
+spec:
+  ports:
+  - appProtocol: grpc
+    name: otlp-grpc
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - appProtocol: http
+    name: otlp-http
+    port: 4318
+    protocol: TCP
+    targetPort: 4318
+  - appProtocol: http
+    name: otlp-http-legacy
+    port: 55681
+    protocol: TCP
+    targetPort: 4318
diff --git a/tests/e2e/managed-reconcile/00-install.yaml b/tests/e2e/managed-reconcile/00-install.yaml
@@ -0,0 +1,22 @@
+apiVersion: opentelemetry.io/v1alpha1
+kind: OpenTelemetryCollector
+metadata:
+  name: simplest
+spec:
+  config: |
+    receivers:
+      otlp:
+        protocols:
+          grpc:
+          http:
+    processors:
+
+    exporters:
+      logging:
+
+    service:
+      pipelines:
+        traces:
+          receivers: [otlp]
+          processors: []
+          exporters: [logging]
diff --git a/tests/e2e/managed-reconcile/01-assert.yaml b/tests/e2e/managed-reconcile/01-assert.yaml
@@ -0,0 +1,82 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: simplest-collector
+status:
+  readyReplicas: 1
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector-headless
+spec:
+  ports:
+  - appProtocol: grpc
+    name: otlp-grpc
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - appProtocol: http
+    name: otlp-http
+    port: 4318
+    protocol: TCP
+    targetPort: 4318
+  - appProtocol: http
+    name: otlp-http-legacy
+    port: 55681
+    protocol: TCP
+    targetPort: 4318
+
+---
+
+apiVersion: v1
+kind: Service
+metadata:
+  name: simplest-collector
+spec:
+  ports:
+  - appProtocol: grpc
+    name: otlp-grpc
+    port: 4317
+    protocol: TCP
+    targetPort: 4317
+  - appProtocol: http
+    name: otlp-http
+    port: 4318
+    protocol: TCP
+    targetPort: 4318
+  - appProtocol: http
+    name: otlp-http-legacy
+    port: 55681
+    protocol: TCP
+    targetPort: 4318
+
+---
+
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: simplest-collector
+data:
+  collector.yaml: |
+    receivers:
+      jaeger:
+        protocols:
+          grpc:
+      otlp:
+        protocols:
+          grpc:
+          http:
+    processors:
+
+    exporters:
+      logging:
+
+    service:
+      pipelines:
+        traces:
+          receivers: [jaeger, otlp]
+          processors: []
+          exporters: [logging]