adds operations to generalize createOrReplace

also adds / clarifies serverSideApply docs

Closes #5377 #5363

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 #### Improvements
 * Fix #5368: added support for additional ListOptions fields
+* Fix #5377: added a createOr and unlock function to provide a straight-forward replacement for createOrReplace.
 * Fix #5388: [crd-generator] Generate deterministic CRDs
 
 #### Dependency Upgrade

doc/FAQ.md

@@ -151,14 +151,26 @@ When the resourceVersion is null for a patch the server will always attempt to p
 
 ### Alternatives to createOrReplace and replace
 
-createOrReplace was introduced as an alternative to the kubectl apply operation.  Over the years there were quite a few issues highlighting where the behavior was different, and there were only limited workarounds and improvements offered.  Given the additional complexity of matching the kubectl client side apply behavior, that was never offered as an option.  Now that there is first class support for serverSideApply it can be use, but it comes with a couple of caveats:
+createOrReplace was introduced as an alternative to the kubectl apply operation.  Over the years there were quite a few issues highlighting where the behavior was different, and there were only limited workarounds and improvements offered.  Given the additional complexity of matching the kubectl client side apply behavior, that was never offered as an option.  Now that there is first class support for serverSideApply it can be used, but it comes with a couple of caveats:
 
 - you will want to use forceConficts - resource.forceConflicts().serverSideApply() - this is especially true when acting as a controller updating a resource that [manages](https://kubernetes.io/docs/reference/using-api/server-side-apply/#using-server-side-apply-in-a-controller)
 
 - for some resource types serverSideApply may cause vacuous revisions - see https://github.com/kubernetes/kubernetes/issues/118519 - if you are being informed of modifications on those resources you must either filter those out, don't perform the serverSideApply, or use the [java-operator-sdk](https://github.com/java-operator-sdk/java-operator-sdk) that should handle that possibility already.
 
-- a common pattern for createOrReplace was obtaining the current resource from the api server (possibly via an informer cache), making modifications, then calling createOrReplace.  Doing something similar with serverSideApply will fail as the managedFields will be populated.  While you may be tempted to simply clear the managedFields and the resourceVersion, this is generally not what you should be doing unless you want your logic to assume ownership of every field on the resource.  Instead you should re-organize your code to apply only the desired state that you wish to apply.  If you have a more involved situation please read the [upstream server side apply documentation](https://kubernetes.io/docs/reference/using-api/server-side-apply/) to understand topics like transferring ownership and partial patches.
+- Keep in mind that serverSideApply is not the same as client side apply.  In particular serverSideApply does not treat list merging the same, which may lead to unexpected behavior when list item merge keys are modified by actors other than the manager of that field.  See the upstream issue: https://github.com/kubernetes/kubernetes/issues/118725
 
-The alternative to replace is either serverSideApply - with the same caveats as above - or to use update, but with resourceVersion set to null.
+- a common pattern for createOrReplace was obtaining the current resource from the api server (possibly via an informer cache), making modifications, then calling createOrReplace.  Doing something similar with serverSideApply will fail as the managedFields will be populated.  While you may be tempted to simply clear the managedFields and the resourceVersion, this is generally not what you should be doing unless you want your logic to assume ownership of every field on the resource.  Instead you should reorganize your code to apply only the desired state that you wish to apply.  If you have a more involved situation please read the [upstream server side apply documentation](https://kubernetes.io/docs/reference/using-api/server-side-apply/) to understand topics like transferring ownership and partial patches.
 
-**Note:** that when using informers - do not make modifications to the resources obtained from the cache - especially to the resourceVersion.
+If the limitations / changes necessary to use serverSideApply are too much, you may also use the createOr method.  Instead of 
+```
+resource.createOrReplace()
+```
+you would use 
+```
+resource.unlock().createOr(NonDeletingOperation::update)
+```
+Or NonDeletingOperation::patch.  The use of the unlock function is optional and is only needed if you are starting with a item that has the resourceVersion populated.  If you have any concern over replacing concurrent changes you should omit the usage of the unlock function.
+
+The alternative to replace is either serverSideApply - with the same caveats as above - or to use update, but with resourceVersion set to null or usage of the unlock function.
+
+**Note:** that when using informers - do not make modifications to the resources obtained from the cache - especially to the resourceVersion.  If you use the unlock function it will make changes to a copy of your item.

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

@@ -23,7 +23,8 @@ public interface CreateOrReplaceable<T> extends Replaceable<T> {
    *
    * @return created item returned in kubernetes api response
    *
-   * @deprecated please use {@link ServerSideApplicable#serverSideApply()} or attempt a create then edit/patch operation.
+   * @deprecated please use {@link ServerSideApplicable#serverSideApply()}
+   *             or the {@link NonDeletingOperation#createOr(java.util.function.Function)}.
    * @see <a href=
    *      "https://github.com/fabric8io/kubernetes-client/blob/main/doc/FAQ.md#alternatives-to-createOrReplace-and-replace"
    *      >Migration FAQ</a>
@@ -37,4 +38,5 @@ public interface CreateOrReplaceable<T> extends Replaceable<T> {
    * @return the item from the api server
    */
   T create();
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/dsl/NonDeletingOperation.java

@@ -15,10 +15,33 @@
  */
 package io.fabric8.kubernetes.client.dsl;
 
+import java.util.function.Function;
+
 public interface NonDeletingOperation<T> extends
     CreateOrReplaceable<T>,
     EditReplacePatchable<T>,
     Replaceable<T>, ItemReplacable<T>,
     ItemWritableOperation<T>,
     ServerSideApplicable<T> {
+
+  /**
+   * Alternative to {@link CreateOrReplaceable#createOrReplace()}.
+   * <p>
+   * Will attempt a create, and if that fails will perform the conflictAction.
+   * <p>
+   * Most commonly the conflictAction will be NonDeletingOperation::update or NonDeletingOperation::patch,
+   * but you are free to provide whatever Function suits your needs.
+   *
+   * @param conflictAction to be performed it the create fails with a conflict
+   * @return
+   */
+  T createOr(Function<NonDeletingOperation<T>, T> conflictAction);
+
+  /**
+   * Removes the resource version from the current item. If the operation context was
+   * created by name, and without an item, this will fetch the item from the api server first.
+   *
+   * @return NonDeletingOperation that may act on the unlocked item
+   */
+  NonDeletingOperation<T> unlock();
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extension/ExtensibleResource.java

@@ -90,6 +90,9 @@ public interface ExtensibleResource<T> extends Resource<T>, TimeoutableScalable<
   @Override
   ExtensibleResource<T> withTimeout(long timeout, TimeUnit unit);
 
+  @Override
+  ExtensibleResource<T> unlock();
+
   @Override
   default ExtensibleResource<T> withTimeoutInMillis(long timeoutInMillis) {
     return withTimeout(timeoutInMillis, TimeUnit.MILLISECONDS);

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extension/ExtensibleResourceAdapter.java

@@ -127,4 +127,9 @@ public ExtensibleResource<T> withTimeoutInMillis(long timeoutInMillis) {
     return withTimeout(timeoutInMillis, TimeUnit.MILLISECONDS);
   }
 
+  @Override
+  public ExtensibleResource<T> unlock() {
+    return newInstance().init(resource.unlock(), client);
+  }
+
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extension/ResourceAdapter.java

@@ -370,4 +370,14 @@ public Scale scale(Scale scale) {
     return resource.scale(scale);
   }
 
+  @Override
+  public T createOr(Function<NonDeletingOperation<T>, T> conflictAction) {
+    return resource.createOr(conflictAction);
+  }
+
+  @Override
+  public NonDeletingOperation<T> unlock() {
+    return resource.unlock();
+  }
+
 }

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

@@ -44,6 +44,7 @@
 import io.fabric8.kubernetes.client.dsl.FilterNested;
 import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
+import io.fabric8.kubernetes.client.dsl.NonDeletingOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.dsl.Waitable;
 import io.fabric8.kubernetes.client.dsl.base.PatchContext;
@@ -307,6 +308,29 @@ public final T createOrReplace() {
     return createOrReplaceHelper.createOrReplace(item);
   }
 
+  @Override
+  public T createOr(Function<NonDeletingOperation<T>, T> conflictAction) {
+    try {
+      return create();
+    } catch (KubernetesClientException e) {
+      if (e.getCode() == HttpURLConnection.HTTP_CONFLICT) {
+        return conflictAction.apply(this);
+      }
+      throw e;
+    }
+  }
+
+  @Override
+  public ExtensibleResource<T> unlock() {
+    // this could be done lazily and tracked via the context,
+    // but it's easier to just modify the item
+    T current = getItemOrRequireFromServer();
+    if (current.getMetadata() != null) {
+      current.getMetadata().setResourceVersion(null);
+    }
+    return newInstance(context.withItem(current));
+  }
+
   @Override
   public FilterWatchListDeletable<T, L, R> withLabels(Map<String, String> labels) {
     return withNewFilter().withLabels(labels).endFilter();

kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CreateOrTest.java

@@ -0,0 +1,107 @@
+/**
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.fabric8.kubernetes.client.mock;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodBuilder;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.KubernetesClientException;
+import io.fabric8.kubernetes.client.dsl.NamespaceableResource;
+import io.fabric8.kubernetes.client.dsl.NonDeletingOperation;
+import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
+import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import java.net.HttpURLConnection;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+@EnableKubernetesMockClient
+class CreateOrTest {
+
+  KubernetesMockServer server;
+  KubernetesClient client;
+
+  @Test
+  @DisplayName("Should replace an existing resource in Kubernetes Cluster")
+  void testResourceReplace() {
+    server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12345").and().build()).once();
+
+    server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12345").and().build()).times(2);
+
+    server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12345").and().build()).once();
+
+    Pod pod = client.resource(new PodBuilder().withNewMetadata().withName("pod123").and().withNewSpec().and().build())
+        .createOr(NonDeletingOperation::update);
+    assertNotNull(pod);
+    assertEquals("12345", pod.getMetadata().getResourceVersion());
+  }
+
+  @Test
+  @DisplayName("Should create a new resource in Kubernetes Cluster")
+  void testResourceCreate() {
+    server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CREATED, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12345").and().build()).once();
+
+    HasMetadata result = client
+        .resource(new PodBuilder().withNewMetadata().withName("pod123").and().withNewSpec().and().build())
+        .createOr(NonDeletingOperation::update);
+    assertNotNull(result);
+    assertEquals("12345", result.getMetadata().getResourceVersion());
+  }
+
+  @Test
+  @DisplayName("Should throw Exception on failed create")
+  void testResourceCreateFailure() {
+    // Given
+    server.expect().post().withPath("/api/v1/namespaces/test/pods")
+        .andReturn(HttpURLConnection.HTTP_BAD_REQUEST, new PodBuilder()
+            .withNewMetadata().withResourceVersion("12345").endMetadata().build())
+        .once();
+    NamespaceableResource<Pod> podOperation = client
+        .resource(new PodBuilder().withNewMetadata().withName("pod123").endMetadata().build());
+
+    // When
+    assertThrows(KubernetesClientException.class, () -> podOperation.createOr(NonDeletingOperation::update));
+  }
+
+  @Test
+  void testUnlock() {
+    // Given
+    server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, "").once();
+
+    server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12345").and().build()).once();
+
+    server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
+        .withNewMetadata().withResourceVersion("12346").and().build()).once();
+
+    NamespaceableResource<Pod> podOperation = client
+        .resource(new PodBuilder().withNewMetadata().withResourceVersion("12344").withName("pod123").endMetadata().build());
+
+    Pod pod = podOperation.unlock().createOr(NonDeletingOperation::update);
+    assertNotNull(pod);
+    assertEquals("12346", pod.getMetadata().getResourceVersion());
+  }
+
+}