diff --git a/controllers/classifier_controller.go b/controllers/classifier_controller.go index 804aba0..f76ae7e 100644 --- a/controllers/classifier_controller.go +++ b/controllers/classifier_controller.go @@ -383,6 +383,8 @@ func (r *ClassifierReconciler) SetupWithManager(ctx context.Context, go removeStaleClassifierResources(ctx, logger) } + go removeStaleClassifierReports(ctx, logger) + return c, nil } diff --git a/controllers/classifier_deployer.go b/controllers/classifier_deployer.go index 171abcf..5e7079c 100644 --- a/controllers/classifier_deployer.go +++ b/controllers/classifier_deployer.go @@ -155,6 +155,19 @@ func (r *ClassifierReconciler) deployClassifier(ctx context.Context, classifierS l := logger.WithValues("cluster", fmt.Sprintf("%s:%s/%s", cluster.Kind, cluster.Namespace, cluster.Name)) + // Cluster may have been deleted since this ClassifierReport was created. Skip it rather + // than treating it as a failure; removeStaleClassifierReports removes the stale report + // separately, regardless of the report's Spec.Match value. + if _, err := clusterproxy.GetCluster(ctx, r.Client, cluster.Namespace, cluster.Name, + clusterproxy.GetClusterType(cluster)); err != nil { + if apierrors.IsNotFound(err) { + l.V(logs.LogDebug).Info("cluster no longer exists, skipping") + continue + } + errorSeen = err + continue + } + clusterInfo, err := r.processClassifier(ctx, classifierScope, r.ControlPlaneEndpoint, cluster, f, l) if err != nil { errorSeen = err diff --git a/controllers/classifier_report_collection_test.go b/controllers/classifier_report_collection_test.go index 4fe9d9c..6d30267 100644 --- a/controllers/classifier_report_collection_test.go +++ b/controllers/classifier_report_collection_test.go @@ -181,6 +181,54 @@ var _ = Describe("Classifier Deployer", func() { Expect(len(classifierReportList.Items)).To(BeZero()) }) + It("pruneClassifierReportsForDeletedClusters removes reports only for clusters that no longer exist", func() { + clusterType := libsveltosv1beta1.ClusterTypeCapi + + // existingCluster still has a Cluster instance: its ClassifierReport must survive + existingClusterNamespace := randomString() + existingClusterName := randomString() + existingCluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: existingClusterNamespace, + Name: existingClusterName, + }, + } + keepClassifierName := randomString() + keepReport := getClassifierReport(keepClassifierName, existingClusterNamespace, existingClusterName) + keepReport.Labels = libsveltosv1beta1.GetClassifierReportLabels( + keepClassifierName, existingClusterName, &clusterType) + + // deletedClusterNamespace/Name has no matching Cluster instance: both its ClassifierReports + // (from two different Classifier instances) must be removed, Spec.Match is irrelevant. + deletedClusterNamespace := randomString() + deletedClusterName := randomString() + staleClassifierName1 := randomString() + staleReport1 := getClassifierReport(staleClassifierName1, deletedClusterNamespace, deletedClusterName) + staleReport1.Labels = libsveltosv1beta1.GetClassifierReportLabels( + staleClassifierName1, deletedClusterName, &clusterType) + staleClassifierName2 := randomString() + staleReport2 := getClassifierReport(staleClassifierName2, deletedClusterNamespace, deletedClusterName) + staleReport2.Labels = libsveltosv1beta1.GetClassifierReportLabels( + staleClassifierName2, deletedClusterName, &clusterType) + + initObjects := []client.Object{ + existingCluster, + keepReport, + staleReport1, + staleReport2, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(keepReport, staleReport1, staleReport2). + WithObjects(initObjects...).Build() + + Expect(controllers.PruneClassifierReportsForDeletedClusters(context.TODO(), c, logger)).To(Succeed()) + + classifierReportList := &libsveltosv1beta1.ClassifierReportList{} + Expect(c.List(context.TODO(), classifierReportList)).To(Succeed()) + Expect(len(classifierReportList.Items)).To(Equal(1)) + Expect(classifierReportList.Items[0].Name).To(Equal(keepReport.Name)) + }) + It("collectClassifierReports collects ClassifierReports from clusters", func() { cluster := prepareCluster() diff --git a/controllers/export_test.go b/controllers/export_test.go index 773342d..6fcf464 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -39,29 +39,30 @@ 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 - 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 + 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 30e6233..335d5c4 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -179,6 +179,75 @@ func removeStaleClassifierResources(ctx context.Context, logger logr.Logger) { } } +// removeStaleClassifierReports periodically removes ClassifierReports whose referenced cluster +// no longer exists. This complements the cleanup done in syncAndGetMatchingClusters, which only +// catches reports with Spec.Match set to true; a report left over with Spec.Match false (or unset) +// for a since-deleted cluster is otherwise never removed and gets retried by every Classifier +// reconcile forever. +func removeStaleClassifierReports(ctx context.Context, logger logr.Logger) { + const interval = 5 * time.Minute + ticker := time.NewTicker(interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + logger.Info("stopping detection and removal of stale ClassifierReports") + return + case <-ticker.C: + if err := pruneClassifierReportsForDeletedClusters(ctx, getManagementClusterClient(), logger); err != nil { + logger.V(logs.LogInfo).Info(fmt.Sprintf("failed to prune stale ClassifierReports: %v", err)) + } + } + } +} + +// pruneClassifierReportsForDeletedClusters removes every ClassifierReport referencing a cluster +// that no longer exists, regardless of the report's Spec.Match value. +func pruneClassifierReportsForDeletedClusters(ctx context.Context, c client.Client, logger logr.Logger) error { + classifierReportList := &libsveltosv1beta1.ClassifierReportList{} + if err := c.List(ctx, classifierReportList); err != nil { + return fmt.Errorf("failed to list ClassifierReports: %w", err) + } + + // Multiple ClassifierReports (from different Classifier instances) can reference the same + // cluster; check each distinct cluster only once per sweep. + checked := make(map[corev1.ObjectReference]bool) + + for i := range classifierReportList.Items { + report := &classifierReportList.Items[i] + if report.Spec.ClusterNamespace == "" { + continue + } + + cluster := getClusterRefFromClassifierReport(report) + if checked[*cluster] { + continue + } + checked[*cluster] = true + + _, err := clusterproxy.GetCluster(ctx, c, cluster.Namespace, cluster.Name, clusterproxy.GetClusterType(cluster)) + if err == nil { + continue + } + if !apierrors.IsNotFound(err) { + logger.V(logs.LogInfo).Info(fmt.Sprintf("failed to get cluster %s:%s/%s: %v", + cluster.Kind, cluster.Namespace, cluster.Name, err)) + continue + } + + logger.V(logs.LogInfo).Info(fmt.Sprintf( + "cluster %s:%s/%s no longer exists, removing its ClassifierReports", + cluster.Kind, cluster.Namespace, cluster.Name)) + if err := removeClusterClassifierReports(ctx, c, cluster.Namespace, cluster.Name, + clusterproxy.GetClusterType(cluster), logger); err != nil { + logger.V(logs.LogInfo).Info(fmt.Sprintf("failed to remove stale ClassifierReports: %v", err)) + } + } + + return nil +} + func deplAssociatedClusterExist(ctx context.Context, c client.Client, depl *appsv1.Deployment, logger logr.Logger) (exist bool, clusterName, clusterNamespace string, clusterType libsveltosv1beta1.ClusterType) {