Merge pull request #2372 from rohanKanojia/pr/issue2292

CHANGELOG.md

@@ -12,6 +12,7 @@
 * Fix #2360: bump mockito-core from 3.4.0 to 3.4.2
 * Fix #2355: bump jandex from 2.1.3.Final to 2.2.0.Final 
 * Fix #2353: chore: bump workflow action-setup versions + kubernetes to 1.18.6
+* Fix #2292: Update createOrReplace to do replace when create fails with conflict
 
 #### New Features
 * Fix #2287: Add support for V1 and V1Beta1 CustomResourceDefinition

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CreateOrReplaceable.java

@@ -17,7 +17,20 @@
 
 public interface CreateOrReplaceable<I, T, D> {
 
+  /**
+   * Creates a provided resource in a Kubernetes Cluster. If creation
+   * fails with a HTTP_CONFLICT, it tries to replace resource.
+   *
+   * @param item item to create or replace
+   * @return created item returned in kubernetes api response
+   */
   T createOrReplace(I... item);
 
+  /**
+   * Create or replace a resource in a Kubernetes Cluster dynamically with
+   * the help of Kubernetes Model Builders.
+   *
+   * @return created item returned in kubernetes api response
+   */
   D createOrReplaceWithNew();
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java

@@ -15,6 +15,8 @@
  */
 package io.fabric8.kubernetes.client.dsl.base;
 
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
+import io.fabric8.kubernetes.client.utils.ResourceCompare;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -395,24 +397,40 @@ public D createOrReplaceWithNew() throws KubernetesClientException {
 
   @Override
   public T createOrReplace(T... items) {
-    T item = getItem();
+    T itemToCreateOrReplace = getItem();
     if (items.length > 1) {
       throw new IllegalArgumentException("Too many items to create.");
     } else if (items.length == 1) {
-      item = items[0];
+      itemToCreateOrReplace = items[0];
     }
 
-    if (item == null) {
+    if (itemToCreateOrReplace == null) {
       throw new IllegalArgumentException("Nothing to create.");
     }
 
-    if (Utils.isNullOrEmpty(name) && item instanceof HasMetadata) {
-      return withName(((HasMetadata)item).getMetadata().getName()).createOrReplace(item);
+    if (Utils.isNullOrEmpty(name)) {
+
+      return withName(itemToCreateOrReplace.getMetadata().getName()).createOrReplace(itemToCreateOrReplace);
     }
-    if (fromServer().get() == null) {
-      return create(item);
-    } else {
-      return replace(item);
+
+    try {
+      // Create
+      KubernetesResourceUtil.setResourceVersion(itemToCreateOrReplace, null);
+      return create(itemToCreateOrReplace);
+    } catch (KubernetesClientException exception) {
+      if (exception.getCode() != HttpURLConnection.HTTP_CONFLICT) {
+        throw exception;
+      }
+
+      // Conflict; Do Replace
+      T itemFromServer = fromServer().get();
+      if (ResourceCompare.equals(itemFromServer, itemToCreateOrReplace)) {
+        // Nothing changed, ignore
+        return itemToCreateOrReplace;
+      } else {
+        KubernetesResourceUtil.setResourceVersion(itemToCreateOrReplace, KubernetesResourceUtil.getResourceVersion(itemFromServer));
+        return replace(itemToCreateOrReplace);
+      }
     }
   }
 

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java

@@ -17,7 +17,10 @@
 
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.ListOptions;
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
 import io.fabric8.kubernetes.client.utils.Utils;
+
+import java.net.HttpURLConnection;
 import java.util.function.Predicate;
 
 import org.slf4j.Logger;
@@ -137,22 +140,33 @@ public HasMetadata apply() {
   public HasMetadata createOrReplace() {
     HasMetadata meta = acceptVisitors(asHasMetadata(item), visitors);
     ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h = handlerOf(meta);
-    HasMetadata r = h.reload(client, config, meta.getMetadata().getNamespace(), meta);
     String namespaceToUse = meta.getMetadata().getNamespace();
 
-    if (r == null) {
+    String resourceVersion = KubernetesResourceUtil.getResourceVersion(meta);
+    try {
+      // Create
+      KubernetesResourceUtil.setResourceVersion(meta, null);
       return h.create(client, config, namespaceToUse, meta);
-    } else if (deletingExisting) {
-      Boolean deleted = h.delete(client, config, namespaceToUse, propagationPolicy, meta);
-      if (!deleted) {
-        throw new KubernetesClientException("Failed to delete existing item:" + meta);
+    } catch (KubernetesClientException exception) {
+      if (exception.getCode() != HttpURLConnection.HTTP_CONFLICT) {
+        throw exception;
+      }
+
+      // Conflict; check deleteExisting flag otherwise replace
+      HasMetadata r = h.reload(client, config, meta.getMetadata().getNamespace(), meta);
+      if (Boolean.TRUE.equals(deletingExisting)) {
+        Boolean deleted = h.delete(client, config, namespaceToUse, propagationPolicy, meta);
+        if (Boolean.FALSE.equals(deleted)) {
+          throw new KubernetesClientException("Failed to delete existing item:" + meta);
+        }
+        return h.create(client, config, namespaceToUse, meta);
+      } else if (ResourceCompare.equals(r, meta)) {
+        LOGGER.debug("Item has not changed. Skipping");
+        return meta;
+      } else {
+        KubernetesResourceUtil.setResourceVersion(meta, resourceVersion);
+        return h.replace(client, config, namespaceToUse, meta);
       }
-      return h.create(client, config, namespaceToUse, meta);
-    } else if (ResourceCompare.equals(r, meta)) {
-      LOGGER.debug("Item has not changed. Skipping");
-      return meta;
-    } else {
-      return h.replace(client, config, namespaceToUse, meta);
     }
   }
 

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java

@@ -36,12 +36,14 @@
 import io.fabric8.kubernetes.client.dsl.base.OperationSupport;
 import io.fabric8.kubernetes.client.handlers.KubernetesListHandler;
 import io.fabric8.kubernetes.client.internal.readiness.Readiness;
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
 import io.fabric8.kubernetes.client.utils.ResourceCompare;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.client.utils.Utils;
 import io.fabric8.openshift.api.model.Parameter;
 import io.fabric8.openshift.api.model.Template;
 
+import java.net.HttpURLConnection;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.function.Predicate;
 import okhttp3.OkHttpClient;
@@ -261,30 +263,31 @@ public List<HasMetadata> createOrReplace() {
     List<HasMetadata> result = new ArrayList<>();
     for (HasMetadata meta : acceptVisitors(asHasMetadata(item, true), visitors)) {
       ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h = handlerOf(meta);
-      HasMetadata r = h.reload(client, config, meta.getMetadata().getNamespace(), meta);
       String namespaceToUse =  meta.getMetadata().getNamespace();
 
-      if (r == null) {
-        HasMetadata created = h.create(client, config, namespaceToUse, meta);
-        if (created != null) {
-          result.add(created);
-        }
-      } else if(deletingExisting) {
-        Boolean deleted = h.delete(client, config, namespaceToUse, propagationPolicy, meta);
-        if (!deleted) {
-          throw new KubernetesClientException("Failed to delete existing item:" + meta);
+      String resourceVersion = KubernetesResourceUtil.getResourceVersion(meta);
+      try {
+        // Create
+        KubernetesResourceUtil.setResourceVersion(meta, null);
+        result.add(h.create(client, config, namespaceToUse, meta));
+      } catch (KubernetesClientException exception) {
+        if (exception.getCode() != HttpURLConnection.HTTP_CONFLICT) {
+          throw exception;
         }
 
-        HasMetadata created = h.create(client, config, namespaceToUse, meta);
-        if (created != null) {
-          result.add(created);
-        }
-      } else if (ResourceCompare.equals(r, meta)) {
-        LOGGER.debug("Item has not changed. Skipping");
-      } else {
-        HasMetadata replaced = h.replace(client, config, namespaceToUse, meta);
-        if (replaced != null) {
-          result.add(replaced);
+        // Conflict; check deleteExisting flag otherwise replace
+        HasMetadata r = h.reload(client, config, meta.getMetadata().getNamespace(), meta);
+        if (Boolean.TRUE.equals(deletingExisting)) {
+          Boolean deleted = h.delete(client, config, namespaceToUse, propagationPolicy, meta);
+          if (Boolean.FALSE.equals(deleted)) {
+            throw new KubernetesClientException("Failed to delete existing item:" + meta);
+          }
+          result.add(h.create(client, config, namespaceToUse, meta));
+        } else if (ResourceCompare.equals(r, meta)) {
+          LOGGER.debug("Item has not changed. Skipping");
+        } else {
+          KubernetesResourceUtil.setResourceVersion(meta, resourceVersion);
+          result.add(h.replace(client, config, namespaceToUse, meta));
         }
       }
     }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/KubernetesResourceUtil.java

@@ -55,6 +55,21 @@ public static String getResourceVersion(HasMetadata entity) {
     return null;
   }
 
+  /**
+   * Set resource version of a kubernetes resource
+   *
+   * @param entity entity provided
+   * @param resourceVersion updated resource version
+   */
+  public static void setResourceVersion(HasMetadata entity, String resourceVersion) {
+    if (entity != null) {
+      ObjectMeta metadata = entity.getMetadata();
+      if (metadata != null) {
+        metadata.setResourceVersion(resourceVersion);
+      }
+    }
+  }
+
   /**
    * Returns the kind of the entity
    *

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java

@@ -18,45 +18,116 @@
 
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import io.fabric8.kubernetes.api.model.KubernetesList;
 
-import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 public class ResourceCompare {
+  private ResourceCompare() {}
 
-    private static TypeReference<HashMap<String, Object>> TYPE_REF = new TypeReference<HashMap<String, Object>>(){};
+  private static TypeReference<HashMap<String, Object>> TYPE_REF = new TypeReference<HashMap<String, Object>>(){};
 
-    private static final String METADATA = "metadata";
-    private static final String STATUS = "status";
-    private static final String LABELS = "labels";
+  private static final String METADATA = "metadata";
+  private static final String SPEC = "spec";
+  private static final String ITEMS = "items";
 
 
-    public static <T>  boolean equals(T left, T right) {
-        ObjectMapper jsonMapper = Serialization.jsonMapper();
-        Map<String, Object> leftJson = (Map<String, Object>) jsonMapper.convertValue(left, TYPE_REF);
-        Map<String, Object> rightJson = (Map<String, Object>) jsonMapper.convertValue(right, TYPE_REF);
+  /**
+   * This method returns true when left Kubernetes resource contains
+   * all data that's present in right Kubernetes resource, this method
+   * won't consider fields that are missing in right parameters. Values
+   * which are present in right would only be compared.
+   *
+   * @param left kubernetes resource (fetched from cluster)
+   * @param right kubernetes resource (provided as input by user)
+   * @param <T> type for kubernetes resource
+   *
+   * @return boolean value whether both resources are actually equal or not
+   */
+  public static <T>  boolean equals(T left, T right) {
+    ObjectMapper jsonMapper = Serialization.jsonMapper();
+    Map<String, Object> leftJson = jsonMapper.convertValue(left, TYPE_REF);
+    Map<String, Object> rightJson = jsonMapper.convertValue(right, TYPE_REF);
 
-        Map<String, Object> leftLabels = fetchLabels(leftJson);
-        Map<String, Object> rightLabels = fetchLabels(rightJson);
+    if (left instanceof KubernetesList) {
+      return compareKubernetesList(leftJson, rightJson);
+    } else {
+      return compareKubernetesResource(leftJson, rightJson);
+    }
+  }
+
+  public static boolean compareKubernetesList(Map<String, Object> leftJson, Map<String, Object> rightJson) {
+    List<Map<String, Object>> leftItems = (List<Map<String, Object>>)leftJson.get(ITEMS);
+    List<Map<String, Object>> rightItems = (List<Map<String, Object>>)rightJson.get(ITEMS);
+
+    if (leftItems != null && rightItems != null) {
+      if (leftItems.size() != rightItems.size()) {
+        return false;
+      }
+
+      for (int i = 0; i < rightItems.size(); i++) {
+        if (!compareKubernetesResource(leftItems.get(i), rightItems.get(i))) {
+          return false;
+        }
+      }
+    } else return leftItems != null;
+    return true;
+  }
+
+  public static boolean compareKubernetesResource(Map<String, Object> leftJson, Map<String, Object> rightJson) {
+    return isEqualMetadata(leftJson, rightJson) &&
+      isEqualSpec(leftJson, rightJson);
+  }
 
-        HashMap<String, Object> leftMap = trim(leftJson);
-        HashMap<String, Object> rightMap = trim(rightJson);
+  private static boolean isEqualMetadata(Map<String, Object> leftMap, Map<String, Object> rightMap) {
+    Map<String, Object> leftMetadata = (Map<String, Object>) leftMap.get(METADATA);
+    Map<String, Object> rightMetadata = (Map<String, Object>) rightMap.get(METADATA);
 
-        return leftMap.equals(rightMap) && leftLabels.equals(rightLabels);
+    if (leftMetadata == null && rightMetadata == null) {
+      return true;
+    } else if (leftMetadata != null && rightMetadata == null) {
+      return true;
+    } else if (leftMetadata == null) {
+      return false;
     }
 
-    private static HashMap<String, Object> trim(Map<String, Object> map) {
-        HashMap<String, Object> result = new HashMap<>(map);
-        result.remove(STATUS);
-        result.remove(METADATA);
-        return result;
+    return isLeftMapSupersetOfRight(leftMetadata, rightMetadata);
+  }
+
+  private static boolean isEqualSpec(Map<String, Object> leftMap, Map<String, Object> rightMap) {
+    Map<String, Object> leftSpec = (Map<String, Object>) leftMap.get(SPEC);
+    Map<String, Object> rightSpec = (Map<String, Object>) rightMap.get(SPEC);
+
+    if (leftSpec == null && rightSpec == null) {
+      return true;
+    } else if (leftSpec != null && rightSpec == null) {
+      return true;
+    } else if (leftSpec == null) {
+      return false;
     }
 
-    private static Map<String, Object> fetchLabels(Map<String, Object> map){
-        if (!map.containsKey(METADATA) || !((Map<Object, Object>)map.get(METADATA)).containsKey(LABELS)){
-            return Collections.emptyMap();
-        }
-        return (Map<String, Object>) ((Map<Object, Object>)map.get(METADATA)).get(LABELS);
+    return isLeftMapSupersetOfRight(leftSpec, rightSpec);
+  }
+
+  /**
+   * Iterates via keys of right map to see if they are present in left map
+   *
+   * @param leftMap a hashmap of string, object
+   * @param rightMap a hashmap of string, object
+   * @return boolean value indicating whether left contains all keys and values of right map or not
+   */
+  private static boolean isLeftMapSupersetOfRight(Map<String, Object> leftMap, Map<String, Object> rightMap) {
+    for (Map.Entry<String, Object> entry : rightMap.entrySet()) {
+      if (!leftMap.containsKey(entry.getKey())) {
+        return false;
+      }
+
+      if (!leftMap.get(entry.getKey()).equals(entry.getValue())) {
+        return false;
+      }
     }
+    return true;
+  }
 }