From 81eda8223d50e68d5ab0443540b5e52fa1d91629 Mon Sep 17 00:00:00 2001 From: 72004 Date: Sat, 7 Nov 2026 16:01:33 +0500 Subject: [PATCH 1/4] Let a SolrCloud be deleted when its providedConfigMap is gone Reconcile ran the whole reconcile loop for a SolrCloud being deleted, so the providedConfigMap lookup failed with NotFound on every pass and the storage finalizer was never removed. A SolrCloud with a deletionTimestamp now only runs the storage finalizer logic. If the StatefulSet is already gone, the PVCs are found by the selector labels the operator always generates. Fixes #828 --- controllers/solrcloud_controller.go | 34 ++++++++++++ .../solrcloud_controller_storage_test.go | 52 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/controllers/solrcloud_controller.go b/controllers/solrcloud_controller.go index 203d6e10..790fa1a9 100644 --- a/controllers/solrcloud_controller.go +++ b/controllers/solrcloud_controller.go @@ -117,6 +117,12 @@ func (r *SolrCloudReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return reconcile.Result{Requeue: true}, nil } + if !instance.ObjectMeta.DeletionTimestamp.IsZero() { + // Owned resources are garbage collected by Kubernetes, so only the storage finalizer needs handling. + // Skipping the full reconcile keeps a missing dependency, e.g. a deleted providedConfigMap, from blocking deletion. + return r.reconcileDeletion(ctx, instance, logger) + } + // When working with the clouds, some actions outside of kube may need to be retried after a few seconds requeueOrNot := reconcile.Result{} @@ -1206,6 +1212,34 @@ func (r *SolrCloudReconciler) storageClassAllowsExpansion(ctx context.Context, c return allowed, className, nil } +// reconcileDeletion handles a SolrCloud that is being deleted, by running the storage finalizer logic. +func (r *SolrCloudReconciler) reconcileDeletion(ctx context.Context, cloud *solrv1beta1.SolrCloud, logger logr.Logger) (ctrl.Result, error) { + if !util.ContainsString(cloud.ObjectMeta.Finalizers, util.SolrStorageFinalizer) { + return reconcile.Result{}, nil + } + + statefulSet := &appsv1.StatefulSet{} + err := r.Get(ctx, types.NamespacedName{Name: cloud.StatefulSetName(), Namespace: cloud.Namespace}, statefulSet) + if errors.IsNotFound(err) { + // The StatefulSet may already be gone, e.g. with foreground deletion. + // Its PVCs still carry the selector labels the operator always generates. + statefulSet.Spec.Selector = &metav1.LabelSelector{MatchLabels: cloud.SharedLabels()} + } else if err != nil { + return reconcile.Result{}, err + } + + // Do not reconcile the storage finalizer unless we have PVC Labels that we know the Solr data PVCs are using. + // Otherwise it will delete all PVCs possibly + if statefulSet.Spec.Selector == nil || len(statefulSet.Spec.Selector.MatchLabels) == 0 { + return reconcile.Result{}, nil + } + if err = r.reconcileStorageFinalizer(ctx, cloud, statefulSet, logger); err != nil { + logger.Error(err, "Cannot delete PVCs while garbage collecting after deletion.") + return reconcile.Result{RequeueAfter: time.Second * 15}, nil + } + return reconcile.Result{}, nil +} + // Logic derived from: // - https://book.kubebuilder.io/reference/using-finalizers.html // - https://github.com/pravega/zookeeper-operator/blob/v0.2.9/pkg/controller/zookeepercluster/zookeepercluster_controller.go#L629 diff --git a/controllers/solrcloud_controller_storage_test.go b/controllers/solrcloud_controller_storage_test.go index 87a5c357..071c3ed1 100644 --- a/controllers/solrcloud_controller_storage_test.go +++ b/controllers/solrcloud_controller_storage_test.go @@ -169,6 +169,58 @@ var _ = FDescribe("SolrCloud controller - Storage", func() { }) }) + FContext("Persistent Data - PVC Delete on Reclaim with a deleted provided ConfigMap", func() { + var providedConfigMap *corev1.ConfigMap + + BeforeEach(func(ctx context.Context) { + providedConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "provided-config-deleted-first", + Namespace: solrCloud.Namespace, + }, + Data: map[string]string{ + util.SolrXmlFile: "${solr.port.advertise:80}", + }, + } + Expect(k8sClient.Create(ctx, providedConfigMap)).To(Succeed()) + + solrCloud.Spec.CustomSolrKubeOptions.ConfigMapOptions = &solrv1beta1.ConfigMapOptions{ + ProvidedConfigMap: providedConfigMap.Name, + } + solrCloud.Spec.StorageOptions = solrv1beta1.SolrDataStorageOptions{ + PersistentStorage: &solrv1beta1.SolrPersistentDataStorageOptions{ + VolumeReclaimPolicy: solrv1beta1.VolumeReclaimPolicyDelete, + PersistentVolumeClaimTemplate: solrv1beta1.PersistentVolumeClaimTemplate{ + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("5Gi"), + }, + }, + }, + }, + }, + } + }) + FIt("removes the finalizer even though the provided ConfigMap no longer exists", func(ctx context.Context) { + By("waiting for the storage finalizer to be added") + expectSolrCloudWithChecks(ctx, solrCloud, func(g Gomega, found *solrv1beta1.SolrCloud) { + g.Expect(found.GetFinalizers()).To(ContainElement(util.SolrStorageFinalizer), "Storage finalizer was not added") + }) + expectStatefulSet(ctx, solrCloud, solrCloud.StatefulSetName()) + + By("deleting the provided ConfigMap before the SolrCloud, as Helm can do") + Expect(k8sClient.Delete(ctx, providedConfigMap)).To(Succeed()) + + By("deleting the SolrCloud") + Expect(k8sClient.Delete(ctx, solrCloud)).To(Succeed(), "Could not delete solrCloud") + + Eventually(func() error { + return k8sClient.Get(ctx, resourceKey(solrCloud, solrCloud.Name), &solrv1beta1.SolrCloud{}) + }).Should(MatchError("solrclouds.solr.apache.org \""+solrCloud.Name+"\" not found"), "Cloud has not been deleted, thus the finalizers have not been removed from the object.") + }) + }) + FContext("Ephemeral Data - Default", func() { BeforeEach(func() { solrCloud.Spec.StorageOptions = solrv1beta1.SolrDataStorageOptions{ From 860a170740cf7ecd8d3ffd742a99ed11f2d7151f Mon Sep 17 00:00:00 2001 From: 72004 Date: Sat, 7 Nov 2026 16:01:48 +0500 Subject: [PATCH 2/4] Add changelog entry for #828 --- helm/solr-operator/Chart.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml index 9b5e6ffc..b4ea8dce 100644 --- a/helm/solr-operator/Chart.yaml +++ b/helm/solr-operator/Chart.yaml @@ -167,6 +167,13 @@ annotations: url: https://github.com/apache/solr-operator/issues/707 - name: Github PR url: https://github.com/apache/solr-operator/pull/876 + - kind: fixed + description: A SolrCloud that is being deleted no longer gets stuck when its providedConfigMap has already been removed, e.g. by Helm in the same uninstall + links: + - name: Github Issue + url: https://github.com/apache/solr-operator/issues/828 + - name: Github PR + url: https://github.com/apache/solr-operator/pull/888 artifacthub.io/images: | - name: solr-operator image: apache/solr-operator:v0.10.0-prerelease From ebda267bbb9e44426b2e7107ed1120a258217745 Mon Sep 17 00:00:00 2001 From: 72004 Date: Tue, 10 Nov 2026 00:00:47 +0500 Subject: [PATCH 3/4] Test PVC deletion scope when a SolrCloud is deleted --- .../solrcloud_controller_storage_test.go | 80 +++++++++++++++++-- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/controllers/solrcloud_controller_storage_test.go b/controllers/solrcloud_controller_storage_test.go index 071c3ed1..d2f42efb 100644 --- a/controllers/solrcloud_controller_storage_test.go +++ b/controllers/solrcloud_controller_storage_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -202,22 +203,89 @@ var _ = FDescribe("SolrCloud controller - Storage", func() { }, } }) - FIt("removes the finalizer even though the provided ConfigMap no longer exists", func(ctx context.Context) { + // dataClaim carries the StatefulSet selector labels; otherClaim only shares the cloud label and must survive. + createClaims := func(ctx context.Context) (dataClaim, otherClaim *corev1.PersistentVolumeClaim) { + claim := func(name string, labels map[string]string) *corev1.PersistentVolumeClaim { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: solrCloud.Namespace, Labels: labels}, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("5Gi")}, + }, + }, + } + Expect(k8sClient.Create(ctx, pvc)).To(Succeed()) + // envtest has no controller to drop the pvc-protection finalizer, so clear it to really remove the PVC. + DeferCleanup(func(ctx context.Context) { + found := &corev1.PersistentVolumeClaim{} + if err := k8sClient.Get(ctx, resourceKey(solrCloud, name), found); errors.IsNotFound(err) { + return + } + found.Finalizers = nil + Expect(k8sClient.Update(ctx, found)).To(Succeed()) + _ = k8sClient.Delete(ctx, found) + }) + return pvc + } + dataLabels := solrCloud.SharedLabels() + dataLabels["technology"] = solrv1beta1.SolrTechnologyLabel + return claim("data-"+solrCloud.StatefulSetName()+"-0", dataLabels), claim(solrCloud.Name+"-other-claim", solrCloud.SharedLabels()) + } + + deleteCloudAndCheckClaims := func(ctx context.Context, dataClaim, otherClaim *corev1.PersistentVolumeClaim) { + By("deleting the SolrCloud") + Expect(k8sClient.Delete(ctx, solrCloud)).To(Succeed(), "Could not delete solrCloud") + + Eventually(func() error { + return k8sClient.Get(ctx, resourceKey(solrCloud, solrCloud.Name), &solrv1beta1.SolrCloud{}) + }).Should(MatchError("solrclouds.solr.apache.org \""+solrCloud.Name+"\" not found"), "Cloud has not been deleted, thus the finalizers have not been removed from the object.") + + // envtest runs no controller manager, so a deleted PVC may linger with only the pvc-protection finalizer. + found := &corev1.PersistentVolumeClaim{} + err := k8sClient.Get(ctx, resourceKey(solrCloud, dataClaim.Name), found) + if err == nil { + Expect(found.DeletionTimestamp).ToNot(BeNil(), "The Solr data PVC was not deleted") + } else { + Expect(errors.IsNotFound(err)).To(BeTrue(), "Unexpected error getting the Solr data PVC: %v", err) + } + + Expect(k8sClient.Get(ctx, resourceKey(solrCloud, otherClaim.Name), found)).To(Succeed(), "A PVC outside the StatefulSet selector was deleted") + Expect(found.DeletionTimestamp).To(BeNil(), "A PVC outside the StatefulSet selector was deleted") + } + + FIt("removes the finalizer and the data PVCs even though the provided ConfigMap no longer exists", func(ctx context.Context) { By("waiting for the storage finalizer to be added") expectSolrCloudWithChecks(ctx, solrCloud, func(g Gomega, found *solrv1beta1.SolrCloud) { g.Expect(found.GetFinalizers()).To(ContainElement(util.SolrStorageFinalizer), "Storage finalizer was not added") }) expectStatefulSet(ctx, solrCloud, solrCloud.StatefulSetName()) + dataClaim, otherClaim := createClaims(ctx) By("deleting the provided ConfigMap before the SolrCloud, as Helm can do") Expect(k8sClient.Delete(ctx, providedConfigMap)).To(Succeed()) - By("deleting the SolrCloud") - Expect(k8sClient.Delete(ctx, solrCloud)).To(Succeed(), "Could not delete solrCloud") + deleteCloudAndCheckClaims(ctx, dataClaim, otherClaim) + }) - Eventually(func() error { - return k8sClient.Get(ctx, resourceKey(solrCloud, solrCloud.Name), &solrv1beta1.SolrCloud{}) - }).Should(MatchError("solrclouds.solr.apache.org \""+solrCloud.Name+"\" not found"), "Cloud has not been deleted, thus the finalizers have not been removed from the object.") + FIt("removes only the Solr data PVCs when the StatefulSet is already gone", func(ctx context.Context) { + By("waiting for the storage finalizer to be added") + expectSolrCloudWithChecks(ctx, solrCloud, func(g Gomega, found *solrv1beta1.SolrCloud) { + g.Expect(found.GetFinalizers()).To(ContainElement(util.SolrStorageFinalizer), "Storage finalizer was not added") + }) + statefulSet := expectStatefulSet(ctx, solrCloud, solrCloud.StatefulSetName()) + dataClaim, otherClaim := createClaims(ctx) + + By("deleting the provided ConfigMap, so the reconcile cannot recreate the StatefulSet") + Expect(k8sClient.Delete(ctx, providedConfigMap)).To(Succeed()) + + By("deleting the StatefulSet before the SolrCloud, as foreground deletion can do") + Expect(k8sClient.Delete(ctx, statefulSet)).To(Succeed()) + Eventually(func() bool { + return errors.IsNotFound(k8sClient.Get(ctx, resourceKey(solrCloud, statefulSet.Name), &appsv1.StatefulSet{})) + }).Should(BeTrue(), "The StatefulSet was not deleted") + + deleteCloudAndCheckClaims(ctx, dataClaim, otherClaim) }) }) From 4ae5bdfee19f70d6531dc7714ad339ae60dfe568 Mon Sep 17 00:00:00 2001 From: 72004 Date: Tue, 10 Nov 2026 00:06:54 +0500 Subject: [PATCH 4/4] Include the technology label when the StatefulSet is gone Without it, deleting a SolrCloud whose StatefulSet is already gone also deleted PVCs that only carry the solr-cloud label, which the StatefulSet selector would never match. --- controllers/solrcloud_controller.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/controllers/solrcloud_controller.go b/controllers/solrcloud_controller.go index 790fa1a9..ff106695 100644 --- a/controllers/solrcloud_controller.go +++ b/controllers/solrcloud_controller.go @@ -1222,8 +1222,10 @@ func (r *SolrCloudReconciler) reconcileDeletion(ctx context.Context, cloud *solr err := r.Get(ctx, types.NamespacedName{Name: cloud.StatefulSetName(), Namespace: cloud.Namespace}, statefulSet) if errors.IsNotFound(err) { // The StatefulSet may already be gone, e.g. with foreground deletion. - // Its PVCs still carry the selector labels the operator always generates. - statefulSet.Spec.Selector = &metav1.LabelSelector{MatchLabels: cloud.SharedLabels()} + // Its PVCs still carry the selector labels the operator always generates, as in GenerateStatefulSet. + selectorLabels := cloud.SharedLabels() + selectorLabels["technology"] = solrv1beta1.SolrTechnologyLabel + statefulSet.Spec.Selector = &metav1.LabelSelector{MatchLabels: selectorLabels} } else if err != nil { return reconcile.Result{}, err }