Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions controllers/classifier_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ func (r *ClassifierReconciler) SetupWithManager(ctx context.Context,
go removeStaleClassifierResources(ctx, logger)
}

go removeStaleClassifierReports(ctx, logger)

return c, nil
}

Expand Down
13 changes: 13 additions & 0 deletions controllers/classifier_deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions controllers/classifier_report_collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
47 changes: 24 additions & 23 deletions controllers/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 69 additions & 0 deletions controllers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down