syahra712 commented on code in PR #888:
URL: https://github.com/apache/solr-operator/pull/888#discussion_r4516824058


##########
controllers/solrcloud_controller.go:
##########
@@ -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()}

Review Comment:
   Good catch, fixed in 4ae5bdf: the fallback selector now adds 
`technology=solr-cloud`, the same as `GenerateStatefulSet`. A PVC that only has 
the `solr-cloud` label is kept.



##########
controllers/solrcloud_controller.go:
##########
@@ -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 {

Review Comment:
   Added in ebda267: both specs create a data PVC and a PVC with only the 
`solr-cloud` label, one with the StatefulSet present and one with it deleted. 
They check that the data PVC is deleted and the other one is kept. The 
StatefulSet-deleted spec fails without 4ae5bdf.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to