diff --git a/controllers/solrcloud_controller.go b/controllers/solrcloud_controller.go index 203d6e10..ff106695 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,36 @@ 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, 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 + } + + // 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..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" ) @@ -169,6 +170,125 @@ 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"), + }, + }, + }, + }, + }, + } + }) + // 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()) + + deleteCloudAndCheckClaims(ctx, dataClaim, otherClaim) + }) + + 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) + }) + }) + FContext("Ephemeral Data - Default", func() { BeforeEach(func() { solrCloud.Spec.StorageOptions = solrv1beta1.SolrDataStorageOptions{ 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