fix: Report failureMessage when deleting a ClusterSummary fails

controllers/clustersummary_deployer.go

@@ -512,12 +512,12 @@ func (r *ClusterSummaryReconciler) processUndeployResult(ctx context.Context, cl
 			return fmt.Errorf("feature is still being removed")
 		}
 
-		// Failure to undeploy because of missing permission is ignored.
+		// Failure to undeploy because of missing permission is not treated as terminal: this Forbidden
+		// error also covers admission webhooks denying the delete, so the reason is still surfaced via
+		// FailureMessage even though the feature keeps retrying.
 		if apierrors.IsForbidden(result.Err) {
 			logger.V(logs.LogInfo).Info("undeploying failing because of missing permission.")
-			tmpStatus := libsveltosv1beta1.FeatureStatusRemoving
-			status = &tmpStatus
-			r.updateFeatureStatus(clusterSummaryScope, f.id, status, nil, result.Err, logger)
+			r.markUndeployFailureRetriable(clusterSummaryScope, f.id, result.Err, logger)
 			return nil
 		}
 
@@ -535,14 +535,12 @@ func (r *ClusterSummaryReconciler) processUndeployResult(ctx context.Context, cl
 
 		var nonRetriableError *configv1beta1.NonRetriableError
 		if errors.As(result.Err, &nonRetriableError) {
-			removing := libsveltosv1beta1.FeatureStatusRemoving
-			r.updateFeatureStatus(clusterSummaryScope, f.id, &removing, nil, result.Err, logger)
+			r.markUndeployFailureRetriable(clusterSummaryScope, f.id, result.Err, logger)
 			return result.Err
 		}
 		var templateError *configv1beta1.TemplateInstantiationError
 		if errors.As(result.Err, &templateError) {
-			removing := libsveltosv1beta1.FeatureStatusRemoving
-			r.updateFeatureStatus(clusterSummaryScope, f.id, &removing, nil, result.Err, logger)
+			r.markUndeployFailureRetriable(clusterSummaryScope, f.id, result.Err, logger)
 			return result.Err
 		}
 
@@ -587,6 +585,17 @@ func (r *ClusterSummaryReconciler) processUndeployResult(ctx context.Context, cl
 	return fmt.Errorf("cleanup request is queued")
 }
 
+// markUndeployFailureRetriable sets the feature status back to Removing (undeploy will be retried)
+// while still recording failureErr as the FailureMessage, so the reason is not silently dropped.
+func (r *ClusterSummaryReconciler) markUndeployFailureRetriable(clusterSummaryScope *scope.ClusterSummaryScope,
+	featureID libsveltosv1beta1.FeatureID, failureErr error, logger logr.Logger) {
+
+	removing := libsveltosv1beta1.FeatureStatusRemoving
+	r.updateFeatureStatus(clusterSummaryScope, featureID, &removing, nil, failureErr, logger)
+	errorMsg := failureErr.Error()
+	clusterSummaryScope.SetFailureMessage(featureID, &errorMsg)
+}
+
 func cleanPreviousActiveTracking(ctx context.Context, clusterSummary *configv1beta1.ClusterSummary, f feature,
 	logger logr.Logger) error {
 

controllers/clustersummary_deployer_test.go

@@ -27,7 +27,9 @@ import (
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2/textlogger"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
@@ -529,6 +531,58 @@ var _ = Describe("ClustersummaryDeployer", func() {
 		Expect(dep.IsKeyInProgress(key)).To(BeTrue())
 	})
 
+	It("undeployFeature sets FailureMessage when undeploy fails with a Forbidden error", func() {
+		clusterSummary.Spec.ClusterProfileSpec.PolicyRefs = []configv1beta1.PolicyRef{
+			{
+				Namespace: randomString(),
+				Name:      randomString(),
+				Kind:      string(libsveltosv1beta1.ConfigMapReferencedResourceKind),
+			},
+		}
+		clusterSummary.Status.FeatureSummaries = []configv1beta1.FeatureSummary{
+			{FeatureID: libsveltosv1beta1.FeatureResources, Status: libsveltosv1beta1.FeatureStatusRemoving},
+		}
+
+		initObjects := []client.Object{
+			clusterSummary,
+			clusterProfile,
+			cluster,
+		}
+
+		c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(initObjects...).WithObjects(initObjects...).Build()
+
+		clusterSummaryScope := getClusterSummaryScope(c, logger, clusterProfile, clusterSummary)
+
+		// An admission webhook denying the delete request surfaces to the client as a Forbidden
+		// error, same as a genuine RBAC permission error. Verify the underlying message is not lost.
+		webhookDenialErr := apierrors.NewForbidden(schema.GroupResource{Group: "network.kmetal.io", Resource: "eipclaims"},
+			"external", fmt.Errorf("admission webhook \"veipclaim.kb.io\" denied the request: cannot delete EipClaim"))
+
+		dep := fakedeployer.GetClient(context.TODO(), textlogger.NewLogger(textlogger.NewConfig()), c)
+		dep.StoreResult(clusterSummary.Spec.ClusterNamespace, clusterSummary.Spec.ClusterName,
+			clusterSummary.Name, string(libsveltosv1beta1.FeatureResources), libsveltosv1beta1.ClusterTypeCapi, true,
+			webhookDenialErr)
+
+		reconciler := getClusterSummaryReconciler(c, dep)
+
+		f := controllers.GetHandlersForFeature(libsveltosv1beta1.FeatureResources)
+
+		err := controllers.UndeployFeature(reconciler, context.TODO(), clusterSummaryScope, f, textlogger.NewLogger(textlogger.NewConfig()))
+		Expect(err).To(BeNil())
+
+		var featureSummary *configv1beta1.FeatureSummary
+		for i := range clusterSummaryScope.ClusterSummary.Status.FeatureSummaries {
+			fs := &clusterSummaryScope.ClusterSummary.Status.FeatureSummaries[i]
+			if fs.FeatureID == libsveltosv1beta1.FeatureResources {
+				featureSummary = fs
+			}
+		}
+		Expect(featureSummary).ToNot(BeNil())
+		Expect(featureSummary.Status).To(Equal(libsveltosv1beta1.FeatureStatusRemoving))
+		Expect(featureSummary.FailureMessage).ToNot(BeNil())
+		Expect(*featureSummary.FailureMessage).To(Equal(webhookDenialErr.Error()))
+	})
+
 	//nolint: dupl // better readibility of test
 	It("deployFeature return an error if cleaning up is in progress", func() {
 		clusterSummary.Spec.ClusterProfileSpec.PolicyRefs = []configv1beta1.PolicyRef{