From 861063effe4d6bd6b342196a4f88ded310dc766d Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Sat, 25 Jul 2026 17:10:15 +0200 Subject: [PATCH] Fix ClassifierReport stuck at WaitingForDelivery in all modes `ClassifierReport.Status.Phase` was staying at `WaitingForDelivery` forever, in every deployment mode, because nothing in `classifier` ever advanced it. This behavior was different than the sibling `HealthCheckReport`/`EventReport` pipelines in healthcheck-manager event-manager, which explicitly mark `Processed` once a report is collected. This PR makes classifier behavior inline with healthcheck-manager and event-manager --- controllers/classifier_report_collection.go | 74 +++++++++++++++---- .../classifier_report_collection_test.go | 70 +++++++++++++++++- controllers/export_test.go | 49 ++++++------ controllers/utils.go | 11 ++- test/fv/utils_test.go | 12 ++- 5 files changed, 173 insertions(+), 43 deletions(-) diff --git a/controllers/classifier_report_collection.go b/controllers/classifier_report_collection.go index 391f145..097e809 100644 --- a/controllers/classifier_report_collection.go +++ b/controllers/classifier_report_collection.go @@ -337,9 +337,15 @@ func processClassifierReportsForClusterInAgentlessMode(ctx context.Context, c cl continue } l := logger.WithValues("classifierReport", cr.Name) - if err := updateClassifierReport(ctx, c, ref, cr, l); err != nil { + mgmtClassifierReport, err := updateClassifierReport(ctx, c, ref, cr, l) + if err != nil { l.V(logs.LogInfo).Error(err, "failed to update ClassifierReport in management cluster") retErr = err + continue + } + + if err := updateClassifierReportStatus(ctx, c, mgmtClassifierReport, l); err != nil { + retErr = err } } @@ -453,30 +459,47 @@ func collectClassifierReportsFromCluster(ctx context.Context, c client.Client, continue } - err = updateClassifierReport(ctx, c, cluster, cr, l) - if err != nil { - l.V(logs.LogInfo).Error(err, "failed to update EventReport in management cluster") - continue - } + processOneClassifierReport(ctx, c, clusterClient, cluster, cr, l) } return nil } +// processOneClassifierReport updates cr's management-cluster copy and marks that copy Processed +// (this is the object mcp-server/dashboard read to determine pipeline health), then also marks cr +// itself Processed in the managed cluster, mirroring healthcheck-manager/event-manager. Split out +// of collectClassifierReportsFromCluster to keep that function's cyclomatic complexity down. +func processOneClassifierReport(ctx context.Context, c, clusterClient client.Client, + cluster *corev1.ObjectReference, cr *libsveltosv1beta1.ClassifierReport, logger logr.Logger) { + + mgmtClassifierReport, err := updateClassifierReport(ctx, c, cluster, cr, logger) + if err != nil { + logger.V(logs.LogInfo).Error(err, "failed to update ClassifierReport in management cluster") + return + } + + if err := updateClassifierReportStatus(ctx, c, mgmtClassifierReport, logger); err != nil { + return + } + + _ = updateClassifierReportStatus(ctx, clusterClient, cr, logger) +} + func updateClassifierReport(ctx context.Context, c client.Client, cluster *corev1.ObjectReference, - classiferReport *libsveltosv1beta1.ClassifierReport, logger logr.Logger) error { + classiferReport *libsveltosv1beta1.ClassifierReport, logger logr.Logger, +) (*libsveltosv1beta1.ClassifierReport, error) { if classiferReport.Labels == nil { msg := "classifierReport is malformed. Labels is empty" logger.V(logs.LogInfo).Info(msg) - return errors.New(msg) + return nil, errors.New(msg) } classifierName, ok := classiferReport.Labels[libsveltosv1beta1.ClassifierlNameLabel] if !ok { msg := "classifierReport is malformed. Label missing" logger.V(logs.LogInfo).Info(msg) - return errors.New(msg) + return nil, errors.New(msg) } // Verify Classifier still exists @@ -484,12 +507,12 @@ func updateClassifierReport(ctx context.Context, c client.Client, cluster *corev err := c.Get(ctx, types.NamespacedName{Name: classifierName}, ¤tClassifier) if err != nil { if apierrors.IsNotFound(err) { - return nil + return classiferReport, nil } - return err + return nil, err } if !currentClassifier.DeletionTimestamp.IsZero() { - return nil + return classiferReport, nil } clusterType := clusterproxy.GetClusterType(cluster) @@ -510,9 +533,12 @@ func updateClassifierReport(ctx context.Context, c client.Client, cluster *corev currentClassifierReport.Spec.ClusterNamespace = cluster.Namespace currentClassifierReport.Spec.ClusterName = cluster.Name currentClassifierReport.Spec.ClusterType = clusterType - return c.Create(ctx, currentClassifierReport) + if err := c.Create(ctx, currentClassifierReport); err != nil { + return nil, err + } + return currentClassifierReport, nil } - return err + return nil, err } logger.V(logs.LogDebug).Info("update ClassifierReport in management cluster") @@ -522,5 +548,23 @@ func updateClassifierReport(ctx context.Context, c client.Client, cluster *corev currentClassifierReport.Spec.ClusterType = clusterType currentClassifierReport.Labels = libsveltosv1beta1.GetClassifierReportLabels( classifierName, cluster.Name, &clusterType) - return c.Update(ctx, currentClassifierReport) + if err := c.Update(ctx, currentClassifierReport); err != nil { + return nil, err + } + return currentClassifierReport, nil +} + +// updateClassifierReportStatus marks report Processed in the management cluster, mirroring +// healthcheck-manager's/event-manager's HealthCheckReport/EventReport collection pattern so +// ClassifierReport doesn't stay stuck at WaitingForDelivery once it has actually been collected. +func updateClassifierReportStatus(ctx context.Context, c client.Client, + report *libsveltosv1beta1.ClassifierReport, logger logr.Logger) error { + + phase := libsveltosv1beta1.ReportProcessed + report.Status.Phase = &phase + if err := c.Status().Update(ctx, report); err != nil { + logger.V(logs.LogInfo).Error(err, "failed to update ClassifierReport status") + return err + } + return nil } diff --git a/controllers/classifier_report_collection_test.go b/controllers/classifier_report_collection_test.go index 6d30267..5b405e2 100644 --- a/controllers/classifier_report_collection_test.go +++ b/controllers/classifier_report_collection_test.go @@ -18,6 +18,8 @@ package controllers_test import ( "context" + "fmt" + "strings" "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" @@ -270,6 +272,67 @@ var _ = Describe("Classifier Deployer", func() { validateClassifierReports(classifierName, cluster, &clusterType) }) + It("processClassifierReportsForClusterInAgentlessMode marks the report Processed", func() { + cluster := prepareCluster() + clusterType := libsveltosv1beta1.ClusterTypeCapi + + classifierName := randomString() + classifier := getClassifierInstance(classifierName) + Expect(testEnv.Create(context.TODO(), classifier)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, classifier)).To(Succeed()) + + // In agentless mode, sveltos-agent-in-mgmt-cluster writes the ClassifierReport directly + // in the management cluster, in the cluster's own namespace, with the cluster labels set. + waitingForDelivery := libsveltosv1beta1.ReportWaitingForDelivery + classifierReport := &libsveltosv1beta1.ClassifierReport{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: libsveltosv1beta1.GetClassifierReportName(classifierName, cluster.Name, &clusterType), + Labels: libsveltosv1beta1.GetClassifierReportLabels(classifierName, cluster.Name, &clusterType), + }, + Spec: libsveltosv1beta1.ClassifierReportSpec{ + ClusterNamespace: cluster.Namespace, + ClusterName: cluster.Name, + ClassifierName: classifierName, + ClusterType: clusterType, + Match: true, + }, + } + Expect(testEnv.Create(context.TODO(), classifierReport)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, classifierReport)).To(Succeed()) + + classifierReport.Status.Phase = &waitingForDelivery + Expect(testEnv.Status().Update(context.TODO(), classifierReport)).To(Succeed()) + + // In agentless mode, the sveltos-agent version ConfigMap lives in the cluster's own + // namespace, named "sa--" (see libsveltos/lib/sveltos_upgrade). + agentVersionConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: fmt.Sprintf("sa-%s-%s", strings.ToLower(string(clusterType)), cluster.Name), + }, + Data: map[string]string{ + classifierLabelVersion: version, + }, + } + Expect(testEnv.Create(context.TODO(), agentVersionConfigMap)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, agentVersionConfigMap)).To(Succeed()) + + Expect(controllers.ProcessClassifierReportsForClusterInAgentlessMode(context.TODO(), testEnv.Client, + getClusterRef(cluster), []*libsveltosv1beta1.ClassifierReport{classifierReport}, version, logger)).To(Succeed()) + + Eventually(func() bool { + currentClassifierReport := &libsveltosv1beta1.ClassifierReport{} + err := testEnv.Get(context.TODO(), + types.NamespacedName{Namespace: classifierReport.Namespace, Name: classifierReport.Name}, + currentClassifierReport) + if err != nil { + return false + } + return currentClassifierReport.Status.Phase != nil && + *currentClassifierReport.Status.Phase == libsveltosv1beta1.ReportProcessed + }, timeout, pollingInterval).Should(BeTrue()) + }) }) func validateClassifierReports(classifierName string, cluster *clusterv1.Cluster, clusterType *libsveltosv1beta1.ClusterType) { @@ -295,6 +358,11 @@ func validateClassifierReports(classifierName string, cluster *clusterv1.Cluster return false } v, ok := currentClassifierReport.Labels[libsveltosv1beta1.ClassifierlNameLabel] - return ok && v == classifierName + if !ok || v != classifierName { + return false + } + + return currentClassifierReport.Status.Phase != nil && + *currentClassifierReport.Status.Phase == libsveltosv1beta1.ReportProcessed }, timeout, pollingInterval).Should(BeTrue()) } diff --git a/controllers/export_test.go b/controllers/export_test.go index 6fcf464..89f3302 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -39,30 +39,31 @@ func GroupClassifierReportsByCluster( } var ( - DeployClassifierCRD = deployClassifierCRD - DeployClassifierReportCRD = deployClassifierReportCRD - DeployHealthCheckCRD = deployHealthCheckCRD - DeployHealthCheckReportCRD = deployHealthCheckReportCRD - DeployEventSourceCRD = deployEventSourceCRD - DeployEventReportCRD = deployEventReportCRD - DeployReloaderCRD = deployReloaderCRD - DeployReloaderReportCRD = deployReloaderReportCRD - DeployDebuggingConfigurationCRD = deployDebuggingConfigurationCRD - DeployClassifierInstance = deployClassifierInstance - DeploySveltosAgentInManagedCluster = deploySveltosAgentInManagedCluster - ClassifierHash = classifierHash - DeployClassifierInCluster = deployClassifierInCluster - UndeployClassifierFromCluster = undeployClassifierFromCluster - RemoveClassifierReports = removeClassifierReports - RemoveClusterClassifierReports = removeClusterClassifierReports - PruneClassifierReportsForDeletedClusters = pruneClassifierReportsForDeletedClusters - CollectClassifierReportsFromCluster = collectClassifierReportsFromCluster - DeploySveltosAgentInManagementCluster = deploySveltosAgentInManagementCluster - RemoveSveltosAgentFromManagementCluster = removeSveltosAgentFromManagementCluster - GetSveltosAgentLabels = getSveltosAgentLabels - GetSveltosAgentNamespace = getSveltosAgentNamespace - GetSveltosAgentPatches = getSveltosAgentPatches - GetSveltosApplierPatches = getSveltosApplierPatches + DeployClassifierCRD = deployClassifierCRD + DeployClassifierReportCRD = deployClassifierReportCRD + DeployHealthCheckCRD = deployHealthCheckCRD + DeployHealthCheckReportCRD = deployHealthCheckReportCRD + DeployEventSourceCRD = deployEventSourceCRD + DeployEventReportCRD = deployEventReportCRD + DeployReloaderCRD = deployReloaderCRD + DeployReloaderReportCRD = deployReloaderReportCRD + DeployDebuggingConfigurationCRD = deployDebuggingConfigurationCRD + DeployClassifierInstance = deployClassifierInstance + DeploySveltosAgentInManagedCluster = deploySveltosAgentInManagedCluster + ClassifierHash = classifierHash + DeployClassifierInCluster = deployClassifierInCluster + UndeployClassifierFromCluster = undeployClassifierFromCluster + RemoveClassifierReports = removeClassifierReports + RemoveClusterClassifierReports = removeClusterClassifierReports + PruneClassifierReportsForDeletedClusters = pruneClassifierReportsForDeletedClusters + CollectClassifierReportsFromCluster = collectClassifierReportsFromCluster + ProcessClassifierReportsForClusterInAgentlessMode = processClassifierReportsForClusterInAgentlessMode + DeploySveltosAgentInManagementCluster = deploySveltosAgentInManagementCluster + RemoveSveltosAgentFromManagementCluster = removeSveltosAgentFromManagementCluster + GetSveltosAgentLabels = getSveltosAgentLabels + GetSveltosAgentNamespace = getSveltosAgentNamespace + GetSveltosAgentPatches = getSveltosAgentPatches + GetSveltosApplierPatches = getSveltosApplierPatches CreateAccessRequest = createAccessRequest GetAccessRequestName = getAccessRequestName diff --git a/controllers/utils.go b/controllers/utils.go index 335d5c4..69d62c8 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -384,7 +384,14 @@ func updateClassifierReportDeploymentStatus(ctx context.Context, c client.Client return c.Status().Patch(ctx, report, patch) } -// updateClassifierReportLabelStatus patches ClassifierReport.Status with label management data. +// updateClassifierReportLabelStatus patches ClassifierReport.Status with label management data, +// and marks the report Processed. This runs for every matching/non-matching cluster on every +// reconcile, regardless of ClassifierReportMode, so it is what actually closes WaitingForDelivery +// -> Processed for AgentSendReportsNoGateway (send-reports) mode: sveltos-agent pushes Spec +// straight into this same report (by canonical name) but never touches Status, and collection's +// own Processed-marking (processOneClassifierReport / processClassifierReportsForClusterInAgentlessMode) +// never runs for that mode since collectClassifierReports is only started when +// ClassifierReportMode == CollectFromManagementCluster. // Pass nil slices to clear the tracking when a cluster stops matching. func updateClassifierReportLabelStatus(ctx context.Context, c client.Client, classifierName string, cluster *corev1.ObjectReference, @@ -401,6 +408,8 @@ func updateClassifierReportLabelStatus(ctx context.Context, c client.Client, patch := client.MergeFrom(report.DeepCopy()) report.Status.ManagedLabels = managed report.Status.UnManagedLabels = unmanaged + processed := libsveltosv1beta1.ReportProcessed + report.Status.Phase = &processed return c.Status().Patch(ctx, report, patch) } diff --git a/test/fv/utils_test.go b/test/fv/utils_test.go index 45ac177..9a64b9e 100644 --- a/test/fv/utils_test.go +++ b/test/fv/utils_test.go @@ -91,8 +91,16 @@ func verifyClassfierIsProvisioned(classifier *libsveltosv1beta1.Classifier) { types.NamespacedName{Namespace: currentCuster.GetNamespace(), Name: reportName}, report); err != nil { return false } - return report.Status.DeploymentStatus != nil && - *report.Status.DeploymentStatus == libsveltosv1beta1.SveltosStatusProvisioned + if report.Status.DeploymentStatus == nil || + *report.Status.DeploymentStatus != libsveltosv1beta1.SveltosStatusProvisioned { + + return false + } + + // Collection must have marked the report Processed; otherwise it is stuck at + // WaitingForDelivery/Delivering and was never actually picked up. + return report.Status.Phase != nil && + *report.Status.Phase == libsveltosv1beta1.ReportProcessed }, timeout, pollingInterval).Should(BeTrue()) }