From 1c7b296150e0e6193e827a47d47e6d0ed9ad223e Mon Sep 17 00:00:00 2001 From: Avadhut Pisal Date: Wed, 2 Aug 2023 20:25:24 +0530 Subject: [PATCH] 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 --- ...nagementstate-field-to-collector-spec.yaml | 16 ++++ apis/v1alpha1/opentelemetrycollector_types.go | 23 +++++ ...ntelemetry.io_opentelemetrycollectors.yaml | 12 +++ ...ntelemetry.io_opentelemetrycollectors.yaml | 12 +++ .../opentelemetrycollector_controller.go | 6 ++ docs/api.md | 10 +++ pkg/collector/upgrade/upgrade.go | 6 ++ pkg/collector/upgrade/upgrade_test.go | 22 +++-- tests/e2e/managed-reconcile/00-assert.yaml | 58 +++++++++++++ tests/e2e/managed-reconcile/00-install.yaml | 22 +++++ tests/e2e/managed-reconcile/01-assert.yaml | 82 ++++++++++++++++++ .../01-disable-reconciliation.yaml | 51 ++++++++++++ tests/e2e/managed-reconcile/02-assert.yaml | 83 +++++++++++++++++++ .../02-enable-reconciliation.yaml | 23 +++++ 14 files changed, 418 insertions(+), 8 deletions(-) create mode 100755 .chloggen/1881-add-managementstate-field-to-collector-spec.yaml create mode 100644 tests/e2e/managed-reconcile/00-assert.yaml create mode 100644 tests/e2e/managed-reconcile/00-install.yaml create mode 100644 tests/e2e/managed-reconcile/01-assert.yaml create mode 100644 tests/e2e/managed-reconcile/01-disable-reconciliation.yaml create mode 100644 tests/e2e/managed-reconcile/02-assert.yaml create mode 100644 tests/e2e/managed-reconcile/02-enable-reconciliation.yaml diff --git a/.chloggen/1881-add-managementstate-field-to-collector-spec.yaml b/.chloggen/1881-add-managementstate-field-to-collector-spec.yaml new file mode 100755 index 0000000000..a54005d2a0 --- /dev/null +++ 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: \ No newline at end of file diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 443dd3f216..2d599eb675 100644 --- 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 index 1a18ddb382..6357ffd4dc 100644 --- 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 index 45c701979a..f1a732b9e9 100644 --- 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 index 75b84a50b7..74691c8b29 100644 --- 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 index d1db878bf4..9a0fe3bbfa 100644 --- 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.
false + + managementState + enum + + ManagementState defines if the CR should be managed by the operator or not. Default is managed.
+
+ Enum: Managed, Unmanaged
+ Default: Managed
+ + false maxReplicas integer diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go index e5523e8d13..1fed413c5a 100644 --- 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 index 9e0191aa8c..500bf3068d 100644 --- 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 new file mode 100644 index 0000000000..09a06184b0 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e3d856a5c4 --- /dev/null +++ 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] \ No newline at end of file diff --git a/tests/e2e/managed-reconcile/01-assert.yaml b/tests/e2e/managed-reconcile/01-assert.yaml new file mode 100644 index 0000000000..17c266852e --- /dev/null +++ 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] \ No newline at end of file diff --git a/tests/e2e/managed-reconcile/01-disable-reconciliation.yaml b/tests/e2e/managed-reconcile/01-disable-reconciliation.yaml new file mode 100644 index 0000000000..ea52b88497 --- /dev/null +++ b/tests/e2e/managed-reconcile/01-disable-reconciliation.yaml @@ -0,0 +1,51 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + managementState: Unmanaged + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [logging] + +--- +# change config map in Unmanaged mode and it should not get overridden as reconciliation is disabled +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] \ No newline at end of file diff --git a/tests/e2e/managed-reconcile/02-assert.yaml b/tests/e2e/managed-reconcile/02-assert.yaml new file mode 100644 index 0000000000..90da4e64f8 --- /dev/null +++ b/tests/e2e/managed-reconcile/02-assert.yaml @@ -0,0 +1,83 @@ +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 + +--- + +apiVersion: v1 +kind: ConfigMap +metadata: + name: simplest-collector +data: + collector.yaml: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [logging] \ No newline at end of file diff --git a/tests/e2e/managed-reconcile/02-enable-reconciliation.yaml b/tests/e2e/managed-reconcile/02-enable-reconciliation.yaml new file mode 100644 index 0000000000..688ab25edb --- /dev/null +++ b/tests/e2e/managed-reconcile/02-enable-reconciliation.yaml @@ -0,0 +1,23 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: simplest +spec: + managementState: Managed + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + logging: + + service: + pipelines: + traces: + receivers: [otlp] + processors: [] + exporters: [logging] \ No newline at end of file