[GitHub] [flink] wangyang0918 commented on a change in pull request #19153: [FLINK-26695][runtime] Evaluation of deleteConfigMap's return value added

[GitBox <gi...@apache.org>]

wangyang0918 commented on a change in pull request #19153:
URL: https://github.com/apache/flink/pull/19153#discussion_r830134099



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClient.java
##########
@@ -350,14 +350,48 @@ public KubernetesLeaderElector createLeaderElector(
     @Override
     public CompletableFuture<Void> deleteConfigMapsByLabels(Map<String, String> labels) {
         return CompletableFuture.runAsync(
-                () -> this.internalClient.configMaps().withLabels(labels).delete(),
+                () -> {
+                    if (!this.internalClient.configMaps().withLabels(labels).delete()) {

Review comment:
       The current implementation assumes that a normal response(`code >= 200 && code < 300`) means the K8s deleted the ConfigMaps successfully. So we also should believe that a `404` code means the ConfigMap does not exist, maybe deleted externally.
   
   Do we really need to check again the existence when the return value of `delete()` is `false`? IIUC, both the `true` and `false` indicate the ConfigMaps does not exist now.




-- 
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@flink.apache.org

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