fix: reconcile error message not set (#4)

* fix(helm): reconcile error message not set * fix(makefile): increase linter timeout to 5m * add testcase to ensure UI plugin reconciliation
cloudoperators · Mar 15, 2024 · 3179974 · 3179974
1 parent 26f101a
commit 3179974
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 32 deletions.
diff --git a/Makefile b/Makefile
@@ -62,7 +62,7 @@ test: generate-manifests generate envtest ## Run tests.
 fmt: goimports golint
 	GOBIN=$(LOCALBIN) go fmt ./...
 	$(GOIMPORTS) -w -local github.com/cloudoperators/greenhouse .
-	$(GOLINT) run -v
+	$(GOLINT) run -v --timeout 5m
 
 .PHONY: check
 check: fmt test

diff --git a/pkg/controllers/pluginconfig/helm_controller.go b/pkg/controllers/pluginconfig/helm_controller.go
@@ -149,9 +149,9 @@ func (r *HelmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{}, fmt.Errorf("plugin not found: %s", helmReconcileFailedCondition.Message)
 	}
 
-	conditions, success := r.reconcileHelmRelease(ctx, restClientGetter, pluginConfig, plugin, pluginConfigStatus)
-	pluginConfigStatus.StatusConditions.SetConditions(conditions...)
-	if !success {
+	driftDetectedCondition, reconcileFailedCondition := r.reconcileHelmRelease(ctx, restClientGetter, pluginConfig, plugin, pluginConfigStatus)
+	pluginConfigStatus.StatusConditions.SetConditions(driftDetectedCondition, reconcileFailedCondition)
+	if reconcileFailedCondition.IsTrue() {
 		return ctrl.Result{}, fmt.Errorf("helm reconcile failed: %s", helmReconcileFailedCondition.Message)
 	}
 	statusReconcileCompleteCondition := r.reconcileStatus(ctx, restClientGetter, pluginConfig, plugin, &pluginConfigStatus)
@@ -279,59 +279,57 @@ func (r *HelmReconciler) reconcileHelmRelease(
 	pluginConfig *greenhousev1alpha1.PluginConfig,
 	plugin *greenhousev1alpha1.Plugin,
 	pluginConfigStatus greenhousev1alpha1.PluginConfigStatus,
-) (conditions []greenhousev1alpha1.Condition, success bool) {
+) (driftDetectedCondition, reconcileFailedCondition greenhousev1alpha1.Condition) {
 
-	helmReconcileFailedCondition := *pluginConfigStatus.GetConditionByType(greenhousev1alpha1.HelmReconcileFailedCondition)
+	driftDetectedCondition = *pluginConfigStatus.GetConditionByType(greenhousev1alpha1.HelmDriftDetectedCondition)
+	reconcileFailedCondition = *pluginConfigStatus.GetConditionByType(greenhousev1alpha1.HelmReconcileFailedCondition)
 
 	// Not a HelmChart plugin. Ignore it.
 	if plugin.Spec.HelmChart == nil {
-		helmReconcileFailedCondition.Status = metav1.ConditionFalse
-		helmReconcileFailedCondition.Message = "Plugin is not backed by HelmChart"
-		return append(conditions, helmReconcileFailedCondition), false
+		reconcileFailedCondition.Status = metav1.ConditionFalse
+		reconcileFailedCondition.Message = "Plugin is not backed by HelmChart"
+		return driftDetectedCondition, reconcileFailedCondition
 	}
 
 	// Validate before attempting the installation/upgrade.
 	// Any error is reflected in the status of the PluginConfig.
 	if _, err := helm.TemplateHelmChartFromPlugin(ctx, r.Client, restClientGetter, plugin, pluginConfig); err != nil {
-		helmReconcileFailedCondition.Status = metav1.ConditionTrue
-		helmReconcileFailedCondition.Message = fmt.Sprintf("Helm template failed: %s", err.Error())
-		return append(conditions, helmReconcileFailedCondition), false
+		reconcileFailedCondition.Status = metav1.ConditionTrue
+		reconcileFailedCondition.Message = fmt.Sprintf("Helm template failed: %s", err.Error())
+		return driftDetectedCondition, reconcileFailedCondition
 	}
 
 	// Check whether the deployed resources match the ones we expect.
 	diffObjects, isHelmDrift, err := helm.DiffChartToDeployedResources(ctx, r.Client, restClientGetter, plugin, pluginConfig)
 	if err != nil {
-		helmReconcileFailedCondition.Status = metav1.ConditionTrue
-		helmReconcileFailedCondition.Message = fmt.Sprintf("Helm diff failed: %s", err.Error())
-		return append(conditions, helmReconcileFailedCondition), false
+		reconcileFailedCondition.Status = metav1.ConditionTrue
+		reconcileFailedCondition.Message = fmt.Sprintf("Helm diff failed: %s", err.Error())
+		return driftDetectedCondition, reconcileFailedCondition
 	}
-	helmDriftDetectedCondition := pluginConfigStatus.StatusConditions.GetConditionByType(greenhousev1alpha1.HelmDriftDetectedCondition)
+
 	switch isHelmDrift {
 	case true:
-		helmDriftDetectedCondition.Status = metav1.ConditionTrue
-		helmDriftDetectedCondition.LastTransitionTime = metav1.Now()
-		conditions = append(conditions, *helmDriftDetectedCondition)
+		driftDetectedCondition.Status = metav1.ConditionTrue
+		driftDetectedCondition.LastTransitionTime = metav1.Now()
 	case false:
-		helmDriftDetectedCondition.Status = metav1.ConditionFalse
-		helmDriftDetectedCondition.LastTransitionTime = metav1.Now()
-		conditions = append(conditions, *helmDriftDetectedCondition)
+		driftDetectedCondition.Status = metav1.ConditionFalse
+		driftDetectedCondition.LastTransitionTime = metav1.Now()
 
-		helmReconcileFailedCondition.Status = metav1.ConditionFalse
-		helmReconcileFailedCondition.Message = "Release for pluginconfig is up-to-date"
+		reconcileFailedCondition.Status = metav1.ConditionFalse
+		reconcileFailedCondition.Message = "Release for pluginconfig is up-to-date"
 		// TODO: remove unnecessary log?
 		log.FromContext(ctx).Info("release for pluginconfig is up-to-date")
-		return append(conditions, helmReconcileFailedCondition, *helmDriftDetectedCondition), true
+		return driftDetectedCondition, reconcileFailedCondition
 	}
 	log.FromContext(ctx).Info("drift between deployed resources and manifest detected", "resources", diffObjects.String())
 	if err := helm.InstallOrUpgradeHelmChartFromPlugin(ctx, r.Client, restClientGetter, plugin, pluginConfig); err != nil {
-		helmReconcileFailedCondition.Status = metav1.ConditionTrue
-		helmReconcileFailedCondition.Message = fmt.Sprintf("Helm install/upgrade failed: %s", err.Error())
-		return append(conditions, helmReconcileFailedCondition), false
+		reconcileFailedCondition.Status = metav1.ConditionTrue
+		reconcileFailedCondition.Message = fmt.Sprintf("Helm install/upgrade failed: %s", err.Error())
+		return driftDetectedCondition, reconcileFailedCondition
 	}
-	helmReconcileFailedCondition.Status = metav1.ConditionFalse
-	helmReconcileFailedCondition.Message = "Helm install/upgrade successful"
-
-	return append(conditions, helmReconcileFailedCondition), true
+	reconcileFailedCondition.Status = metav1.ConditionFalse
+	reconcileFailedCondition.Message = "Helm install/upgrade successful"
+	return driftDetectedCondition, reconcileFailedCondition
 }
 
 func (r *HelmReconciler) reconcileStatus(ctx context.Context,

diff --git a/pkg/controllers/pluginconfig/suite_test.go b/pkg/controllers/pluginconfig/suite_test.go
@@ -630,6 +630,67 @@ var _ = Describe("HelmControllerTest", Serial, func() {
 
 })
 
+var _ = When("the plugin is UI only", func() {
+	var uiPlugin *greenhousev1alpha1.Plugin
+	var uiPluginConfig *greenhousev1alpha1.PluginConfig
+	BeforeEach(func() {
+		uiPlugin = &greenhousev1alpha1.Plugin{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Plugin",
+				APIVersion: greenhousev1alpha1.GroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "myuiplugin",
+			},
+			Spec: greenhousev1alpha1.PluginSpec{
+				Description: "Testplugin with UI only",
+				Version:     "1.0.0",
+				UIApplication: &greenhousev1alpha1.UIApplicationReference{
+					Name:    "myapp",
+					Version: "1.0.0",
+					URL:     "http://myapp.com",
+				},
+			},
+		}
+		uiPluginConfig = &greenhousev1alpha1.PluginConfig{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "PluginConfig",
+				APIVersion: greenhousev1alpha1.GroupVersion.String(),
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "uipluginconfig",
+				Namespace: "default",
+			},
+			Spec: greenhousev1alpha1.PluginConfigSpec{
+				Plugin: "myuiplugin",
+			},
+		}
+
+		Expect(test.K8sClient.Create(test.Ctx, uiPlugin)).Should(Succeed())
+		Expect(test.K8sClient.Create(test.Ctx, uiPluginConfig)).Should(Succeed())
+	})
+
+	AfterEach(func() {
+		Expect(test.K8sClient.Delete(test.Ctx, uiPlugin)).Should(Succeed())
+		Expect(test.K8sClient.Delete(test.Ctx, uiPluginConfig)).Should(Succeed())
+	})
+
+	It("should skip the helm reconciliation without errors", func() {
+		pluginConfigID := types.NamespacedName{Name: "uipluginconfig", Namespace: "default"}
+		Eventually(func(g Gomega) bool {
+			err := test.K8sClient.Get(test.Ctx, pluginConfigID, uiPluginConfig)
+			if err != nil {
+				return false
+			}
+			g.Expect(uiPluginConfig.Status.GetConditionByType(greenhousev1alpha1.ReadyCondition)).ToNot(BeNil())
+			g.Expect(uiPluginConfig.Status.GetConditionByType(greenhousev1alpha1.ReadyCondition).Status).To(Equal(metav1.ConditionTrue))
+			g.Expect(uiPluginConfig.Status.GetConditionByType(greenhousev1alpha1.HelmReconcileFailedCondition).Status).To(Equal(metav1.ConditionFalse))
+			g.Expect(uiPluginConfig.Status.GetConditionByType(greenhousev1alpha1.HelmReconcileFailedCondition).Message).To(Equal("Plugin is not backed by HelmChart"))
+			return true
+		}).Should(BeTrue())
+	})
+})
+
 var _ = AfterSuite(func() {
 	By("tearing down the test environment")
 	test.TestAfterSuite()