🐛 dont set negative addon conds if no addon ops were performed (#64)

* fix: dont set neg addon conds if no addon ops were performed

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: dont obscure crd error when addons are requested

Signed-off-by: Artur Shad Nik <[email protected]>

* chore: add logging if addontemplate crd is not found

Signed-off-by: Artur Shad Nik <[email protected]>

---------

Signed-off-by: Artur Shad Nik <[email protected]>

Author: arturshadnik

fleetconfig-controller/internal/controller/v1alpha1/addon.go

@@ -35,22 +35,23 @@ const (
 	hubAddon  = "hub-addon"
 )
 
-func handleAddonConfig(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) error {
+func handleAddonConfig(ctx context.Context, kClient client.Client, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) (bool, error) {
 	logger := log.FromContext(ctx)
 	logger.V(0).Info("handleAddOnConfig", "fleetconfig", fc.Name)
 
+	requestedAddOns := fc.Spec.AddOnConfigs
+
 	// get existing addons
 	createdAddOns, err := addonC.AddonV1alpha1().AddOnTemplates().List(ctx, metav1.ListOptions{LabelSelector: v1alpha1.ManagedBySelector.String()})
 	if err != nil {
-		return err
+		logger.V(1).Info("failed to list AddOnTemplates, ensure CRDs are installed.", "error", err)
+		return len(requestedAddOns) > 0, err
 	}
 
-	requestedAddOns := fc.Spec.AddOnConfigs
-
 	// nothing to do
 	if len(requestedAddOns) == 0 && len(createdAddOns.Items) == 0 {
 		logger.V(5).Info("no addons to reconcile")
-		return nil
+		return false, nil
 	}
 
 	// compare existing to requested
@@ -83,15 +84,15 @@ func handleAddonConfig(ctx context.Context, kClient client.Client, addonC *addon
 	// do deletes first, then creates.
 	err = handleAddonDelete(ctx, addonC, fc, addonsToDelete)
 	if err != nil {
-		return err
+		return true, err
 	}
 
 	err = handleAddonCreate(ctx, kClient, fc, addonsToCreate)
 	if err != nil {
-		return err
+		return true, err
 	}
 
-	return nil
+	return true, nil
 }
 
 func handleAddonCreate(ctx context.Context, kClient client.Client, fc *v1alpha1.FleetConfig, addons []v1alpha1.AddOnConfig) error {
@@ -408,7 +409,7 @@ func isHubAddOnMatching(installed v1alpha1.InstalledHubAddOn, desired v1alpha1.H
 		installed.BundleVersion == bundleVersion
 }
 
-func handleHubAddons(ctx context.Context, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) error {
+func handleHubAddons(ctx context.Context, addonC *addonapi.Clientset, fc *v1alpha1.FleetConfig) (bool, error) {
 	logger := log.FromContext(ctx)
 	logger.V(0).Info("handleHubAddons", "fleetconfig", fc.Name)
 
@@ -419,7 +420,7 @@ func handleHubAddons(ctx context.Context, addonC *addonapi.Clientset, fc *v1alph
 	// nothing to do
 	if len(desiredAddOns) == 0 && len(installedAddOns) == 0 {
 		logger.V(5).Info("no hub addons to reconcile")
-		return nil
+		return false, nil
 	}
 
 	// Find addons that need to be uninstalled (present in installed, missing from desired or version mismatch)
@@ -447,12 +448,12 @@ func handleHubAddons(ctx context.Context, addonC *addonapi.Clientset, fc *v1alph
 	// do uninstalls first, then installs
 	err := handleHubAddonUninstall(ctx, addonsToUninstall, fc)
 	if err != nil {
-		return err
+		return true, err
 	}
 
 	err = handleHubAddonInstall(ctx, addonC, addonsToInstall, bundleVersion, fc)
 	if err != nil {
-		return err
+		return true, err
 	}
 
 	// build the new installed addons list
@@ -465,7 +466,7 @@ func handleHubAddons(ctx context.Context, addonC *addonapi.Clientset, fc *v1alph
 		})
 	}
 	fc.Status.InstalledHubAddOns = newInstalledAddOns
-	return nil
+	return true, nil
 }
 
 func handleHubAddonUninstall(ctx context.Context, addons []v1alpha1.InstalledHubAddOn, fc *v1alpha1.FleetConfig) error {

fleetconfig-controller/internal/controller/v1alpha1/hub.go

@@ -86,27 +86,29 @@ func handleHub(ctx context.Context, kClient client.Client, fc *v1alpha1.FleetCon
 		v1alpha1.FleetConfigHubInitialized, v1alpha1.FleetConfigHubInitialized, metav1.ConditionTrue, metav1.ConditionTrue,
 	))
 
-	err = handleAddonConfig(ctx, kClient, addonC, fc)
-	if err != nil {
+	addonConfigChanged, err := handleAddonConfig(ctx, kClient, addonC, fc)
+	if err != nil && addonConfigChanged {
 		fc.SetConditions(true, v1alpha1.NewCondition(
 			err.Error(), v1alpha1.FleetConfigAddonsConfigured, metav1.ConditionFalse, metav1.ConditionTrue,
 		))
 		return err
 	}
 
-	err = handleHubAddons(ctx, addonC, fc)
-	if err != nil {
+	hubAddonChanged, err := handleHubAddons(ctx, addonC, fc)
+	if err != nil && hubAddonChanged {
 		fc.SetConditions(true, v1alpha1.NewCondition(
 			err.Error(), v1alpha1.FleetConfigAddonsConfigured, metav1.ConditionFalse, metav1.ConditionTrue,
 		))
 		return err
 	}
 
-	if len(fc.Spec.AddOnConfigs)+len(fc.Spec.HubAddOns) > 0 {
+	// only set success condition if we actually managed any addons
+	if addonConfigChanged || hubAddonChanged {
 		fc.SetConditions(true, v1alpha1.NewCondition(
 			v1alpha1.FleetConfigAddonsConfigured, v1alpha1.FleetConfigAddonsConfigured, metav1.ConditionTrue, metav1.ConditionTrue,
 		))
 	}
+
 	// attempt an upgrade whenever the clustermanager's bundleVersion changes
 	upgrade, err := hubNeedsUpgrade(ctx, fc, operatorC)
 	if err != nil {