[I] DeletePodForUpdate reports NotFound as a rolling-update delete error [solr-operator]

["ChaoHuang2018 (via GitHub)" <gi...@apache.org>]

ChaoHuang2018 opened a new issue, #839:
URL: https://github.com/apache/solr-operator/issues/839

   Hi Solr Operator maintainers,
   
   While looking at the managed rolling update path, I noticed that `DeletePodForUpdate` appears to treat an already-deleted pod as an error.
   
   `handleManagedCloudRollingUpdate` calls `DeletePodForUpdate` for pods selected for update:
   
   [`controllers/solr_cluster_ops_util.go:508-L510`](https://github.com/apache/solr-operator/blob/32bb7987a02a3491483563564b99a03ebb4bda77/controllers/solr_cluster_ops_util.go#L508-L510)
   
   Inside `DeletePodForUpdate`, once the pod is ready to be deleted, the helper deletes it with a UID precondition:
   
   [`controllers/solr_pod_lifecycle_util.go:102-L107`](https://github.com/apache/solr-operator/blob/32bb7987a02a3491483563564b99a03ebb4bda77/controllers/solr_pod_lifecycle_util.go#L102-L107)
   
   That UID precondition is useful because it avoids deleting a replacement pod with the same name. But if the target pod is already gone, the helper currently logs the delete error and records a `PodUpdateError` event:
   
   [`controllers/solr_pod_lifecycle_util.go:108-L111`](https://github.com/apache/solr-operator/blob/32bb7987a02a3491483563564b99a03ebb4bda77/controllers/solr_pod_lifecycle_util.go#L108-L111)
   
   I do not see an `apierrors.IsNotFound` case around that delete. A possible sequence is:
   
   ```text
   managed rolling update selects pod P for update
   DeletePodForUpdate prepares P for deletion
   P is already deleted by a previous reconcile attempt or another controller action
   the cached rolling-update state still includes P
   r.Delete(ctx, P, UID precondition) returns NotFound
   DeletePodForUpdate reports PodUpdateError even though P is already absent
   ```
   
   For this update path, absence of the old pod seems to be the desired postcondition: the StatefulSet can recreate it with the updated spec. Would it be reasonable to treat `apierrors.IsNotFound(err)` from this delete as success, while still keeping the UID precondition for the non-NotFound case?
   
   For example, the delete branch could avoid logging/returning a warning for NotFound and only report other delete errors.
   


-- 
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: issues-unsubscribe@solr.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org

Re: [I] DeletePodForUpdate reports NotFound as a rolling-update delete error [solr-operator]

["arpitjain099 (via GitHub)" <gi...@apache.org>]

arpitjain099 commented on issue #839:
URL: https://github.com/apache/solr-operator/issues/839#issuecomment-5494900797

   This looks resolved on main. `DeletePodForUpdate` now checks `apierrors.IsNotFound` around the delete and records a normal `PodUpdate` event instead of `PodUpdateError`, which came in with #840 (ed5c5c7).


-- 
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: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org