Ensures that we only skip reconciliation for unmanaged resources (open-telemetry#2197)

* ensures that we only skip reconciliation for unmanaged resources

* update tests

Author: jaronoff97

.chloggen/default-managed.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# 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: fixes scenario where an old CRD would cause the operator to default to an unmanaged state
+
+# One or more tracking issues related to the change
+issues: [2039]
+
+# (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:

apis/v1alpha1/opentelemetrycollector_webhook.go

@@ -112,6 +112,11 @@ func (r *OpenTelemetryCollector) Default() {
 	if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Ingress.RuleType == "" {
 		r.Spec.Ingress.RuleType = IngressRuleTypePath
 	}
+	// If someone upgrades to a later version without upgrading their CRD they will not have a management state set.
+	// This results in a default state of unmanaged preventing reconciliation from continuing.
+	if len(r.Spec.ManagementState) == 0 {
+		r.Spec.ManagementState = ManagementStateManaged
+	}
 }
 
 // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1

apis/v1alpha1/opentelemetrycollector_webhook_test.go

@@ -49,6 +49,7 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					ManagementState: ManagementStateManaged,
 					PodDisruptionBudget: &PodDisruptionBudgetSpec{
 						MaxUnavailable: &intstr.IntOrString{
 							Type:   intstr.Int,
@@ -77,6 +78,37 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeSidecar,
 					Replicas:        &five,
 					UpgradeStrategy: "adhoc",
+					ManagementState: ManagementStateManaged,
+					PodDisruptionBudget: &PodDisruptionBudgetSpec{
+						MaxUnavailable: &intstr.IntOrString{
+							Type:   intstr.Int,
+							IntVal: 1,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "doesn't override unmanaged",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					ManagementState: ManagementStateUnmanaged,
+					Mode:            ModeSidecar,
+					Replicas:        &five,
+					UpgradeStrategy: "adhoc",
+				},
+			},
+			expected: OpenTelemetryCollector{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"app.kubernetes.io/managed-by": "opentelemetry-operator",
+					},
+				},
+				Spec: OpenTelemetryCollectorSpec{
+					Mode:            ModeSidecar,
+					Replicas:        &five,
+					UpgradeStrategy: "adhoc",
+					ManagementState: ManagementStateUnmanaged,
 					PodDisruptionBudget: &PodDisruptionBudgetSpec{
 						MaxUnavailable: &intstr.IntOrString{
 							Type:   intstr.Int,
@@ -106,6 +138,7 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					ManagementState: ManagementStateManaged,
 					Autoscaler: &AutoscalerSpec{
 						TargetCPUUtilization: &defaultCPUTarget,
 						MaxReplicas:          &five,
@@ -137,6 +170,7 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					ManagementState: ManagementStateManaged,
 					Autoscaler: &AutoscalerSpec{
 						TargetCPUUtilization: &defaultCPUTarget,
 						// webhook Default adds MaxReplicas to Autoscaler because
@@ -171,7 +205,8 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					},
 				},
 				Spec: OpenTelemetryCollectorSpec{
-					Mode: ModeDeployment,
+					Mode:            ModeDeployment,
+					ManagementState: ManagementStateManaged,
 					Ingress: Ingress{
 						Type: IngressTypeRoute,
 						Route: OpenShiftRoute{
@@ -212,6 +247,7 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					Replicas:        &one,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
+					ManagementState: ManagementStateManaged,
 					PodDisruptionBudget: &PodDisruptionBudgetSpec{
 						MinAvailable: &intstr.IntOrString{
 							Type:   intstr.String,

controllers/opentelemetrycollector_controller.go

@@ -174,7 +174,7 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if instance.Spec.ManagementState != v1alpha1.ManagementStateManaged {
+	if instance.Spec.ManagementState == v1alpha1.ManagementStateUnmanaged {
 		log.Info("Skipping reconciliation for unmanaged OpenTelemetryCollector resource", "name", req.String())
 		// Stop requeueing for unmanaged OpenTelemetryCollector custom resources
 		return ctrl.Result{}, nil