From 7f8067cfb97f710182385aa9f17b1b66b0c66d20 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Sat, 31 Oct 2026 07:53:52 +0100 Subject: [PATCH] fix: Do not overwrite ClassifierReport spec when collecting reports in agentless mode In agentless mode sveltos-agent writes the ClassifierReport directly in the management cluster, so the report listed by the collection and the one in the management cluster are the same object. updateClassifierReport copied the listed Spec over the current one, with a fresh resourceVersion obtained by reading it again. When the agent had written Spec.Match since the report was listed, the stale value was written back. Nothing evaluates the Classifier again after that, so the cluster never got its labels. In agentless mode collection now only marks the current report Processed. --- controllers/classifier_report_collection.go | 30 ++++++-- .../classifier_report_collection_test.go | 68 +++++++++++++++++++ 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/controllers/classifier_report_collection.go b/controllers/classifier_report_collection.go index 02f04a7..6a5d59b 100644 --- a/controllers/classifier_report_collection.go +++ b/controllers/classifier_report_collection.go @@ -337,19 +337,35 @@ func processClassifierReportsForClusterInAgentlessMode(ctx context.Context, c cl continue } l := logger.WithValues("classifierReport", cr.Name) - mgmtClassifierReport, err := updateClassifierReport(ctx, c, ref, cr, l) - if err != nil { - l.V(logs.LogInfo).Error(err, "failed to update ClassifierReport in management cluster") + if err := markClassifierReportProcessed(ctx, c, cr, l); err != nil { retErr = err - continue } + } - if err := updateClassifierReportStatus(ctx, c, mgmtClassifierReport, l); err != nil { - retErr = err + return retErr +} + +// markClassifierReportProcessed marks as Processed a ClassifierReport written directly in the management cluster +// by sveltos-agent. +// +// Unlike in a managed cluster, there is no copy to refresh: the listed ClassifierReport and the one in the management +// cluster are the same object. Spec must not be copied over it, because the listed one can be older than what +// sveltos-agent wrote since it was listed. Copying it would revert Spec.Match, and nothing would evaluate the +// Classifier again to set it back. +func markClassifierReportProcessed(ctx context.Context, c client.Client, + listedReport *libsveltosv1beta1.ClassifierReport, logger logr.Logger) error { + + currentReport := &libsveltosv1beta1.ClassifierReport{} + err := c.Get(ctx, client.ObjectKeyFromObject(listedReport), currentReport) + if err != nil { + if apierrors.IsNotFound(err) { + return nil } + logger.V(logs.LogInfo).Error(err, "failed to get ClassifierReport") + return err } - return retErr + return updateClassifierReportStatus(ctx, c, currentReport, logger) } func skipCollecting(ctx context.Context, c client.Client, cluster *corev1.ObjectReference, diff --git a/controllers/classifier_report_collection_test.go b/controllers/classifier_report_collection_test.go index 6b21a6c..1e1dcae 100644 --- a/controllers/classifier_report_collection_test.go +++ b/controllers/classifier_report_collection_test.go @@ -335,6 +335,74 @@ var _ = Describe("Classifier Deployer", func() { }, timeout, pollingInterval).Should(BeTrue()) }) + It("processClassifierReportsForClusterInAgentlessMode does not revert Spec.Match written after the report was listed", + func() { + cluster := prepareCluster() + clusterType := libsveltosv1beta1.ClusterTypeCapi + + classifierName := randomString() + 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: false, + }, + } + Expect(testEnv.Create(context.TODO(), classifierReport)).To(Succeed()) + Expect(waitForObject(context.TODO(), testEnv.Client, classifierReport)).To(Succeed()) + + 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()) + + // The copy listed by the collection still has Match false + listedReport := classifierReport.DeepCopy() + + // sveltos-agent finds the cluster is a match after the report was listed + currentReport := &libsveltosv1beta1.ClassifierReport{} + Expect(testEnv.Get(context.TODO(), client.ObjectKeyFromObject(classifierReport), currentReport)).To(Succeed()) + currentReport.Spec.Match = true + Expect(testEnv.Update(context.TODO(), currentReport)).To(Succeed()) + + // Eventual loop so testEnv Cache is synced and processing reads the Match just written + Eventually(func() bool { + r := &libsveltosv1beta1.ClassifierReport{} + err := testEnv.Get(context.TODO(), client.ObjectKeyFromObject(classifierReport), r) + return err == nil && r.Spec.Match + }, timeout, pollingInterval).Should(BeTrue()) + + Expect(controllers.ProcessClassifierReportsForClusterInAgentlessMode(context.TODO(), testEnv.Client, + getClusterRef(cluster), []*libsveltosv1beta1.ClassifierReport{listedReport}, version, logger)).To(Succeed()) + + // Eventual loop so testEnv Cache is synced + Eventually(func() bool { + r := &libsveltosv1beta1.ClassifierReport{} + err := testEnv.Get(context.TODO(), client.ObjectKeyFromObject(classifierReport), r) + return err == nil && r.Status.Phase != nil && *r.Status.Phase == libsveltosv1beta1.ReportProcessed + }, timeout, pollingInterval).Should(BeTrue()) + + Consistently(func() bool { + r := &libsveltosv1beta1.ClassifierReport{} + err := testEnv.Get(context.TODO(), client.ObjectKeyFromObject(classifierReport), r) + return err == nil && r.Spec.Match + }, 3*time.Second, time.Second).Should(BeTrue()) + }) + It("processClassifierReportsForClusterInAgentlessMode skips a cluster in pull mode without an agent version ConfigMap", func() { // The agent of a cluster in pull mode runs in the managed cluster: no agent version ConfigMap