diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a37aeca523..7719e9dbe99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 4.11-SNAPSHOT #### Bugs +* Fix #2445: ConfigMap and other resources are replaced #### Improvements diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java index e860e0b0f41..92d5e6a5ab8 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java @@ -16,7 +16,6 @@ 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; @@ -131,26 +130,8 @@ protected BaseOperation(OperationContext ctx) { this.watchRetryBackoffMultiplier = ctx.getWatchRetryBackoffMultiplier(); } - /** - * Returns the name and falls back to the item name. - * @param item The item. - * @param name The name to check. - * @param - * @return - */ - private static String name(T item, String name) { - if (name != null && !name.isEmpty()) { - return name; - } else if (item instanceof HasMetadata) { - HasMetadata h = (HasMetadata) item; - return h.getMetadata() != null ? h.getMetadata().getName() : null; - } - return null; - } - - public BaseOperation newInstance(OperationContext context) { - return new BaseOperation(context); + return new BaseOperation<>(context); } /** @@ -188,14 +169,8 @@ private void addQueryStringParam(HttpUrl.Builder requestUrlBuilder, String name, @Override public T get() { try { - T answer = getMandatory(); - if (answer instanceof HasMetadata) { - HasMetadata hasMetadata = (HasMetadata) answer; - updateApiVersion(hasMetadata); - } else if (answer instanceof KubernetesResourceList) { - KubernetesResourceList list = (KubernetesResourceList) answer; - updateApiVersion(list); - } + final T answer = getMandatory(); + updateApiVersion(answer); return answer; } catch (KubernetesClientException e) { if (e.getCode() != HttpURLConnection.HTTP_NOT_FOUND) { @@ -206,19 +181,13 @@ public T get() { } @Override - public T require() throws ResourceNotFoundException { + public T require() { try { T answer = getMandatory(); if (answer == null) { throw new ResourceNotFoundException("The resource you request doesn't exist or couldn't be fetched."); } - if (answer instanceof HasMetadata) { - HasMetadata hasMetadata = (HasMetadata) answer; - updateApiVersion(hasMetadata); - } else if (answer instanceof KubernetesResourceList) { - KubernetesResourceList list = (KubernetesResourceList) answer; - updateApiVersion(list); - } + updateApiVersion(answer); return answer; } catch (KubernetesClientException e) { if (e.getCode() != HttpURLConnection.HTTP_NOT_FOUND) { @@ -265,7 +234,7 @@ public RootPaths getRootPaths() { } @Override - public D edit() throws KubernetesClientException { + public D edit() { throw new KubernetesClientException("Cannot edit read-only resources"); } @@ -332,8 +301,9 @@ public Gettable fromServer() { return newInstance(context.withReloadingFromServer(true)); } + @SafeVarargs @Override - public T create(T... resources) throws KubernetesClientException { + public final T create(T... resources) { try { if (resources.length > 1) { throw new IllegalArgumentException("Too many items to create."); @@ -363,7 +333,7 @@ public T create(T resource) { } @Override - public D createNew() throws KubernetesClientException { + public D createNew() { final Function visitor = resource -> { try { return create(resource); @@ -381,7 +351,7 @@ public D createNew() throws KubernetesClientException { @Override - public D createOrReplaceWithNew() throws KubernetesClientException { + public D createOrReplaceWithNew() { final Function visitor = resource -> { try { return createOrReplace(resource); @@ -397,8 +367,9 @@ public D createOrReplaceWithNew() throws KubernetesClientException { } } + @SafeVarargs @Override - public T createOrReplace(T... items) { + public final T createOrReplace(T... items) { T itemToCreateOrReplace = getItem(); if (items.length > 1) { throw new IllegalArgumentException("Too many items to create."); @@ -423,16 +394,10 @@ public T createOrReplace(T... items) { 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); - } + final T itemFromServer = fromServer().get(); + KubernetesResourceUtil.setResourceVersion(itemToCreateOrReplace, KubernetesResourceUtil.getResourceVersion(itemFromServer)); + return replace(itemToCreateOrReplace); } } @@ -474,29 +439,28 @@ public FilterWatchListDeletable> withLabelSelec return this; } - // Deprecated as the underlying implementation does not align with the arguments anymore. - // It is possible to negate multiple values with the same key, e.g.: - // foo != bar , foo != baz - // To support this a multi-value map is needed, as a regular map would override the key with the new value. + /** + * @deprecated as the underlying implementation does not align with the arguments anymore. + * It is possible to negate multiple values with the same key, e.g.: + * foo != bar , foo != baz + * To support this a multi-value map is needed, as a regular map would override the key with the new value. + */ @Override @Deprecated - public FilterWatchListDeletable> withoutLabels(Map labels) throws - KubernetesClientException { + public FilterWatchListDeletable> withoutLabels(Map labels) { // Re-use "withoutLabel" to convert values from String to String[] labels.forEach(this::withoutLabel); return this; } @Override - public FilterWatchListDeletable> withLabelIn(String key, String... values) throws - KubernetesClientException { + public FilterWatchListDeletable> withLabelIn(String key, String... values) { labelsIn.put(key, values); return this; } @Override - public FilterWatchListDeletable> withLabelNotIn(String key, String... values) throws - KubernetesClientException { + public FilterWatchListDeletable> withLabelNotIn(String key, String... values) { labelsNotIn.put(key, values); return this; } @@ -540,16 +504,17 @@ public FilterWatchListDeletable> withField(Stri return this; } - // Deprecated as the underlying implementation does not align with the arguments fully. - // Method is created to have a similar API as `withoutLabels`, but should eventually be replaced with something - // better for the same reasons. - // It is possible to negate multiple values with the same key, e.g.: - // foo != bar , foo != baz - // To support this a multi-value map is needed, as a regular map would override the key with the new value. + /** + * @deprecated as the underlying implementation does not align with the arguments fully. + * Method is created to have a similar API as `withoutLabels`, but should eventually be replaced + * with something better for the same reasons. + * It is possible to negate multiple values with the same key, e.g.: + * foo != bar , foo != baz + * To support this a multi-value map is needed, as a regular map would override the key with the new value. + */ @Override @Deprecated - public FilterWatchListDeletable> withoutFields(Map fields) throws - KubernetesClientException { + public FilterWatchListDeletable> withoutFields(Map fields) { // Re-use "withoutField" to convert values from String to String[] labels.forEach(this::withoutField); return this; @@ -655,7 +620,7 @@ public String getFieldQueryParam() { return sb.toString(); } - public L list() throws KubernetesClientException { + public L list() { try { return listRequestHelper(getResourceUrl(namespace, name)); } catch (IOException e) { @@ -700,9 +665,9 @@ public Boolean delete() { } } - + @SafeVarargs @Override - public Boolean delete(T... items) { + public final Boolean delete(T... items) { return delete(Arrays.asList(items)); } @@ -710,25 +675,20 @@ public Boolean delete(T... items) { public Boolean delete(List items) { boolean deleted = true; if (items != null) { - for (T item : items) { - if (item == null) { + for (T toDelete : items) { + if (toDelete == null) { continue; } - updateApiVersionResource(item); + updateApiVersion(toDelete); try { - R op; - - if (item instanceof HasMetadata - && ((HasMetadata) item).getMetadata() != null - && ((HasMetadata) item).getMetadata().getName() != null - && !((HasMetadata) item).getMetadata().getName().isEmpty()) { - op = (R) inNamespace(checkNamespace(item)).withName(((HasMetadata) item).getMetadata().getName()); + if (toDelete.getMetadata() != null + && toDelete.getMetadata().getName() != null + && !toDelete.getMetadata().getName().isEmpty()) { + deleted &= inNamespace(checkNamespace(toDelete)).withName(toDelete.getMetadata().getName()).delete(); } else { - op = (R) withItem(item); + deleted &= withItem(toDelete).delete(); } - - deleted &= op.delete(); } catch (KubernetesClientException e) { if (e.getCode() != HttpURLConnection.HTTP_NOT_FOUND) { throw e; @@ -756,7 +716,7 @@ public BaseOperation withItem(T item) { void deleteThis() { try { if (item != null) { - updateApiVersionResource(item); + updateApiVersion(item); handleDelete(item, gracePeriodSeconds, propagationPolicy, cascading); } else { handleDelete(getResourceUrl(), gracePeriodSeconds, propagationPolicy, cascading); @@ -858,27 +818,27 @@ public boolean isResourceNamespaced() { return Utils.isResourceNamespaced(getType()); } - protected T handleResponse(Request.Builder requestBuilder) throws ExecutionException, InterruptedException, KubernetesClientException, IOException { + protected T handleResponse(Request.Builder requestBuilder) throws ExecutionException, InterruptedException, IOException { return handleResponse(requestBuilder, getType()); } - protected T handleCreate(T resource) throws ExecutionException, InterruptedException, KubernetesClientException, IOException { - updateApiVersionResource(resource); + protected T handleCreate(T resource) throws ExecutionException, InterruptedException, IOException { + updateApiVersion(resource); return handleCreate(resource, getType()); } - protected T handleReplace(T updated) throws ExecutionException, InterruptedException, KubernetesClientException, IOException { - updateApiVersionResource(updated); + protected T handleReplace(T updated) throws ExecutionException, InterruptedException, IOException { + updateApiVersion(updated); return handleReplace(updated, getType()); } - protected T handlePatch(T current, T updated) throws ExecutionException, InterruptedException, KubernetesClientException, IOException { - updateApiVersionResource(updated); + protected T handlePatch(T current, T updated) throws ExecutionException, InterruptedException, IOException { + updateApiVersion(updated); return handlePatch(current, updated, getType()); } protected T handlePatch(T current, Map patchedUpdate) throws ExecutionException, InterruptedException, IOException { - updateApiVersionResource(current); + updateApiVersion(current); return handlePatch(current, patchedUpdate, getType()); } @@ -911,7 +871,7 @@ protected Status handleDeploymentRollback(DeploymentRollback deploymentRollback) protected T handleGet(URL resourceUrl) throws InterruptedException, ExecutionException, IOException { T answer = handleGet(resourceUrl, getType()); - updateApiVersionResource(answer); + updateApiVersion(answer); return answer; } @@ -1033,42 +993,19 @@ protected Class getConfigType() { return Config.class; } - /** - * Updates the list or single item if it has a missing or incorrect apiGroupVersion - * - * @param resource resource object - */ - protected void updateApiVersionResource(Object resource) { - if (resource instanceof HasMetadata) { - HasMetadata hasMetadata = (HasMetadata) resource; - updateApiVersion(hasMetadata); - } else if (resource instanceof KubernetesResourceList) { - KubernetesResourceList list = (KubernetesResourceList) resource; - updateApiVersion(list); - } - } - /** * Updates the list items if they have missing or default apiGroupVersion values and the resource is currently * using API Groups with custom version strings * * @param list Kubernetes resource list */ - protected void updateApiVersion(KubernetesResourceList list) { + protected void updateApiVersion(KubernetesResourceList list) { String version = getApiVersion(); - if (list != null && version != null && version.length() > 0) { - List items = list.getItems(); - if (items != null) { - for (Object item : items) { - if (item instanceof HasMetadata) { - updateApiVersion((HasMetadata) item); - } - } - } + if (list != null && version != null && version.length() > 0 && list.getItems() != null) { + list.getItems().forEach(this::updateApiVersion); } } - /** * Updates the resource if it has missing or default apiGroupVersion values and the resource is currently * using API Groups with custom version strings @@ -1102,8 +1039,7 @@ public boolean isApiGroup() { @Override public Boolean isReady() { - T i = get(); - return i instanceof HasMetadata && Readiness.isReady((HasMetadata) i); + return Readiness.isReady(get()); } @Override diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java index 6aca6b72a38..2012bfef257 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java @@ -23,9 +23,6 @@ import java.net.HttpURLConnection; import java.util.function.Predicate; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -58,7 +55,6 @@ 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.ResourceCompare; import okhttp3.OkHttpClient; public class NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl extends OperationSupport implements @@ -66,8 +62,6 @@ public class NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl ex Waitable, Readiable { - private static final Logger LOGGER = LoggerFactory.getLogger(NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.class); - private final String fallbackNamespace; private final String explicitNamespace; @@ -153,16 +147,12 @@ public HasMetadata createOrReplace() { } // 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); diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java index 5b5bc951e7e..51406545368 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java @@ -37,14 +37,12 @@ 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; import org.slf4j.Logger; @@ -276,15 +274,12 @@ public List createOrReplace() { } // 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)); @@ -299,7 +294,6 @@ public Waitable, HasMetadata> createOrReplaceAnd() { return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, deletingExisting, visitors, createOrReplace(), inputStream, null, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier); } - @Override public Boolean delete() { //First pass check before deleting diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PersistentVolumeClaimTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PersistentVolumeClaimTest.java index ed55179b4c6..1b063229d81 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PersistentVolumeClaimTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PersistentVolumeClaimTest.java @@ -30,12 +30,15 @@ import org.junit.Rule; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport; import java.net.HttpURLConnection; import java.util.Collections; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -44,9 +47,17 @@ @EnableRuleMigrationSupport class PersistentVolumeClaimTest { + @Rule public KubernetesServer server = new KubernetesServer(); + private KubernetesClient client; + + @BeforeEach + void setUp() { + client = server.getClient(); + } + @Test void testList() { server.expect().withPath("/api/v1/namespaces/test/persistentvolumeclaims").andReturn(200, new PersistentVolumeClaimListBuilder().build()).once(); @@ -56,7 +67,6 @@ void testList() { .addNewItem().and() .build()).once(); - KubernetesClient client = server.getClient(); PersistentVolumeClaimList persistentVolumeClaimList = client.persistentVolumeClaims().inNamespace("test").list(); assertNotNull(persistentVolumeClaimList); assertEquals(0, persistentVolumeClaimList.getItems().size()); @@ -76,7 +86,6 @@ void testListWithlabels() { .addNewItem().and() .build()).once(); - KubernetesClient client = server.getClient(); PersistentVolumeClaimList persistentVolumeClaimList = client.persistentVolumeClaims().inNamespace("test") .withLabel("key1", "value1") .withLabel("key2","value2") @@ -98,7 +107,6 @@ void testGet() { server.expect().withPath("/api/v1/namespaces/test/persistentvolumeclaims/persistentvolumeclaim1").andReturn(200, new PersistentVolumeClaimBuilder().build()).once(); server.expect().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/persistentvolumeclaim2").andReturn(200, new PersistentVolumeClaimBuilder().build()).once(); - KubernetesClient client = server.getClient(); PersistentVolumeClaim persistentVolumeClaim = client.persistentVolumeClaims().inNamespace("test").withName("persistentvolumeclaim1").get(); assertNotNull(persistentVolumeClaim); @@ -122,7 +130,6 @@ void testDelete() { server.expect().withPath("/api/v1/namespaces/test/persistentvolumeclaims/persistentvolumeclaim1").andReturn(200, new PersistentVolumeClaimBuilder().build()).once(); server.expect().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/persistentvolumeclaim2").andReturn(200, new PersistentVolumeClaimBuilder().build()).once(); - KubernetesClient client = server.getClient(); Boolean deleted = client.persistentVolumeClaims().inNamespace("test").withName("persistentvolumeclaim1").delete(); assertTrue(deleted); @@ -142,7 +149,6 @@ void testDeleteMulti() { server.expect().withPath("/api/v1/namespaces/test/persistentvolumeclaims/persistentvolumeclaim1").andReturn(200, persistentVolumeClaim1).once(); server.expect().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/persistentvolumeclaim2").andReturn(200, persistentVolumeClaim2).once(); - KubernetesClient client = server.getClient(); Boolean deleted = client.persistentVolumeClaims().inAnyNamespace().delete(persistentVolumeClaim1, persistentVolumeClaim2); assertTrue(deleted); @@ -152,7 +158,6 @@ void testDeleteMulti() { @Test void testLoadFromFile() { - KubernetesClient client = server.getClient(); PersistentVolumeClaim persistentVolumeClaim = client.persistentVolumeClaims().load(getClass().getResourceAsStream("/test-persistentvolumeclaim.yml")).get(); assertEquals("task-pv-claim", persistentVolumeClaim.getMetadata().getName()); } @@ -172,54 +177,88 @@ void testBuild() { server.expect().withPath("/api/v1/namespaces/test/persistentvolumeclaims/test-pv-claim").andReturn(200, persistentVolumeClaim).once(); - KubernetesClient client = server.getClient(); persistentVolumeClaim = client.persistentVolumeClaims().inNamespace("test").withName("test-pv-claim").get(); assertNotNull(persistentVolumeClaim); } @Test - void testCreateOrReplaceIgnoreWhenNoChange() { + @DisplayName("createOrReplace, resource is new, should successfully POST to server") + void testCreateOrReplaceWhenNotExists() { // Given - PersistentVolumeClaim pvcOrignal = new PersistentVolumeClaimBuilder() - .withNewMetadata().withName("my-pvc").endMetadata() - .withNewSpec() - .withAccessModes("ReadWriteOnce") - .withNewResources() - .withRequests(Collections.singletonMap("storage", new Quantity("20Gi"))) - .endResources() - .endSpec() - .build(); - PersistentVolumeClaim pvcFromServer = new PersistentVolumeClaimBuilder() - .withNewMetadata() - .withName("my-pvc") - .withCreationTimestamp("2020-07-27T11:02:15Z") - .addToFinalizers("kubernetes.io/pvc-protection") - .withNamespace("default") - .withResourceVersion("203697") - .withSelfLink("/api/v1/namespaces/default/persistentvolumeclaims/my-pvc") - .withUid("60817eaa-19d8-41ba-adb4-3ea75860e1f8") + server.expect().post().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims") + .andReturn(HttpURLConnection.HTTP_CREATED, mockPVCBuilder().build()).once(); + // @formatter:off + final PersistentVolumeClaim toCreate = mockPVCBuilder() + .editMetadata() + .withResourceVersion(null) + .withCreationTimestamp(null) .endMetadata() - .withNewSpec() - .withAccessModes("ReadWriteOnce") - .withNewResources() - .withRequests(Collections.singletonMap("storage", new Quantity("20Gi"))) - .endResources() - .withStorageClassName("standard") - .withVolumeMode("Filesystem") - .withVolumeName("pvc-60817eaa-19d8-41ba-adb4-3ea75860e1f8") - .endSpec() .build(); + // @formatter:on + // When + final PersistentVolumeClaim result = client.persistentVolumeClaims().inNamespace("ns1") + .createOrReplace(toCreate); + // Then + assertThat(result) + .isNotNull() + .extracting("metadata") + .hasFieldOrPropertyWithValue("resourceVersion", "1") + .extracting("creationTimestamp").asString().isNotBlank(); + } + + @Test + @DisplayName("createOrReplace, resource exists on server, should PUT updated resource") + void testCreateOrReplaceWhenExists() { + // Given + final PersistentVolumeClaim existent = mockPVCBuilder().build(); server.expect().post().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims") - .andReturn(HttpURLConnection.HTTP_CONFLICT, pvcFromServer).once(); + .andReturn(HttpURLConnection.HTTP_CONFLICT, "").once(); server.expect().get().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/my-pvc") - .andReturn(HttpURLConnection.HTTP_OK, pvcFromServer).once(); - KubernetesClient client = server.getClient(); - + .andReturn(HttpURLConnection.HTTP_OK, existent).times(2); + server.expect().put().withPath("/api/v1/namespaces/ns1/persistentvolumeclaims/my-pvc") + .andReturn(HttpURLConnection.HTTP_OK, mockPVCBuilder() + .editMetadata().withResourceVersion("2").endMetadata().build()).once(); + // @formatter:off + final PersistentVolumeClaim toReplace = mockPVCBuilder() + .editMetadata() + .withResourceVersion(null) + .withCreationTimestamp(null) + .endMetadata() + .build(); + // @formatter:on // When - PersistentVolumeClaim pvcResult = client.persistentVolumeClaims().inNamespace("ns1").createOrReplace(pvcOrignal); - + final PersistentVolumeClaim result = client.persistentVolumeClaims().inNamespace("ns1") + .createOrReplace(toReplace); // Then - assertNotNull(pvcResult); + assertThat(result) + .isNotNull() + .extracting("metadata") + .hasFieldOrPropertyWithValue("resourceVersion", "2") + .hasFieldOrPropertyWithValue("creationTimestamp", existent.getMetadata().getCreationTimestamp()); + } + + private static PersistentVolumeClaimBuilder mockPVCBuilder() { + // @formatter:off + return new PersistentVolumeClaimBuilder() + .withNewMetadata() + .withName("my-pvc") + .withCreationTimestamp("2020-07-27T11:02:15Z") + .addToFinalizers("kubernetes.io/pvc-protection") + .withNamespace("ns1") + .withResourceVersion("1") + .withSelfLink("/api/v1/namespaces/default/persistentvolumeclaims/my-pvc") + .withUid("60817eaa-19d8-41ba-adb4-3ea75860e1f8") + .endMetadata() + .withNewSpec() + .withAccessModes("ReadWriteOnce") + .withNewResources() + .withRequests(Collections.singletonMap("storage", new Quantity("20Gi"))) + .endResources() + .withStorageClassName("standard") + .withVolumeMode("Filesystem") + .withVolumeName("pvc-60817eaa-19d8-41ba-adb4-3ea75860e1f8") + .endSpec(); + // @formatter:on } } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java index c11a55845a1..820be4ef498 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java @@ -34,6 +34,7 @@ import okhttp3.mockwebserver.RecordedRequest; import org.junit.Rule; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport; @@ -52,29 +53,25 @@ public class ResourceListTest { @Rule public KubernetesServer server = new KubernetesServer(); - Service service = new ServiceBuilder() - .withNewMetadata().withName("my-service").endMetadata() - .withNewSpec() - .addToSelector("app", "Myapp") - .addNewPort().withProtocol("TCP").withPort(80).withTargetPort(new IntOrString(9376)).endPort() - .endSpec() - .build(); - Service updatedService = new ServiceBuilder(service) - .withNewSpec() - .addToSelector("app", "Myapp") - .addNewPort().withProtocol("TCP").withPort(80).withTargetPort(new IntOrString(9999)).endPort() - .endSpec() - .build(); - - - ConfigMap configMap = new ConfigMapBuilder() - .withNewMetadata().withName("my-configmap").endMetadata() - .addToData(Collections.singletonMap("app", "Myapp")) - .build(); - ConfigMap updatedConfigMap = new ConfigMapBuilder(configMap) - .addToData(Collections.singletonMap("foo", "bar")) - .build(); - + private KubernetesClient client; + private Service service; + private Service updatedService; + private ConfigMap configMap; + private ConfigMap updatedConfigMap; + private KubernetesList resourcesToUpdate; + + @BeforeEach + void setUp() { + client = server.getClient(); + service = mockService().build(); + configMap = mockConfigMap().build(); + updatedService = mockService().editSpec().editFirstPort() + .withTargetPort(new IntOrString(9999)).endPort().endSpec() + .build(); + updatedConfigMap = mockConfigMap().addToData(Collections.singletonMap("foo", "bar")).build(); + resourcesToUpdate = new KubernetesListBuilder() + .withItems(updatedService, updatedConfigMap).build(); + } @Test void testCreateOrReplace() { @@ -82,7 +79,6 @@ void testCreateOrReplace() { server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); - KubernetesClient client = server.getClient(); List response = client.resourceList(new PodListBuilder().addToItems(pod1).build()).createOrReplace(); assertTrue(response.contains(pod1)); } @@ -92,7 +88,6 @@ void testCreateOrReplaceFailedCreate() { // Given Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_UNAVAILABLE, pod1).once(); - KubernetesClient client = server.getClient(); NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable listOp = client.resourceList(new PodListBuilder().addToItems(pod1).build()); // When @@ -105,7 +100,6 @@ void testCreateWithExplicitNamespace() { server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); - KubernetesClient client = server.getClient(); List response = client.resourceList(new PodListBuilder().addToItems(pod1).build()).inNamespace("ns1").apply(); assertTrue(response.contains(pod1)); } @@ -121,8 +115,6 @@ void testDelete() { server.expect().withPath("/api/v1/namespaces/ns1/pods/pod2").andReturn(HttpURLConnection.HTTP_OK, pod2).times(2); server.expect().withPath("/api/v1/namespaces/any/pods/pod3").andReturn(HttpURLConnection.HTTP_OK, pod3).times(1); - KubernetesClient client = server.getClient(); - //First time all items should be deleted. Boolean deleted = client.resourceList(new PodListBuilder().withItems(pod1, pod2, pod3).build()).delete(); assertTrue(deleted); @@ -136,16 +128,14 @@ void testDelete() { void testCreateOrReplaceWithoutDeleteExisting() throws Exception { server.expect().post().withPath("/api/v1/namespaces/ns1/services").andReturn(HttpURLConnection.HTTP_CONFLICT, service).once(); server.expect().post().withPath("/api/v1/namespaces/ns1/configmaps").andReturn(HttpURLConnection.HTTP_CONFLICT, configMap).once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HttpURLConnection.HTTP_OK , service).times(2); - server.expect().get().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HttpURLConnection.HTTP_OK, configMap).times(2); + server.expect().get().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HttpURLConnection.HTTP_OK , service).once(); + server.expect().get().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HttpURLConnection.HTTP_OK, configMap).once(); server.expect().put().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HttpURLConnection.HTTP_OK, updatedService).once(); server.expect().put().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HttpURLConnection.HTTP_OK, updatedConfigMap).once(); - KubernetesClient client = server.getClient(); - KubernetesList list = new KubernetesListBuilder().withItems(updatedService, updatedConfigMap).build(); - client.resourceList(list).inNamespace("ns1").createOrReplace(); + client.resourceList(resourcesToUpdate).inNamespace("ns1").createOrReplace(); - assertEquals(8, server.getMockServer().getRequestCount()); + assertEquals(6, server.getMockServer().getRequestCount()); RecordedRequest request = server.getLastRequest(); assertEquals("/api/v1/namespaces/ns1/configmaps/my-configmap", request.getPath()); assertEquals("PUT", request.getMethod()); @@ -155,21 +145,31 @@ void testCreateOrReplaceWithoutDeleteExisting() throws Exception { void testCreateOrReplaceWithDeleteExisting() throws Exception { server.expect().post().withPath("/api/v1/namespaces/ns1/services").andReturn(HttpURLConnection.HTTP_CONFLICT, service).once(); server.expect().post().withPath("/api/v1/namespaces/ns1/configmaps").andReturn(HttpURLConnection.HTTP_CONFLICT, configMap).once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HttpURLConnection.HTTP_OK , service).once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HttpURLConnection.HTTP_OK, configMap).once(); server.expect().delete().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(HttpURLConnection.HTTP_OK , service).once(); server.expect().delete().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(HttpURLConnection.HTTP_OK, configMap).once(); server.expect().post().withPath("/api/v1/namespaces/ns1/services").andReturn(HttpURLConnection.HTTP_OK, updatedService).once(); server.expect().post().withPath("/api/v1/namespaces/ns1/configmaps").andReturn(HttpURLConnection.HTTP_OK, updatedConfigMap).once(); - KubernetesClient client = server.getClient(); - KubernetesList list = new KubernetesListBuilder().withItems(updatedService, updatedConfigMap).build(); - client.resourceList(list).inNamespace("ns1").deletingExisting().createOrReplace(); - + client.resourceList(resourcesToUpdate).inNamespace("ns1").deletingExisting().createOrReplace(); - assertEquals(8, server.getMockServer().getRequestCount()); + assertEquals(6, server.getMockServer().getRequestCount()); RecordedRequest request = server.getLastRequest(); assertEquals("/api/v1/namespaces/ns1/configmaps", request.getPath()); assertEquals("POST", request.getMethod()); } + + private static ServiceBuilder mockService() { + return new ServiceBuilder() + .withNewMetadata().withName("my-service").endMetadata() + .withNewSpec() + .addToSelector("app", "my-app") + .addNewPort().withProtocol("TCP").withPort(80).withTargetPort(new IntOrString(9376)).endPort() + .endSpec(); + } + + private static ConfigMapBuilder mockConfigMap() { + return new ConfigMapBuilder() + .withNewMetadata().withName("my-configmap").endMetadata() + .addToData(Collections.singletonMap("app", "my-app")); + } } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java index 3996d628d98..b3944881bac 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java @@ -62,7 +62,6 @@ public class ResourceTest { @Rule public KubernetesServer server = new KubernetesServer(); - @Test void testCreateOrReplace() { // Given @@ -91,13 +90,13 @@ void testCreateOrReplaceWhenCreateFails() { @Test void testCreateWithExplicitNamespace() { - Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); + Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); - server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); + server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); - KubernetesClient client = server.getClient(); - HasMetadata response = client.resource(pod1).inNamespace("ns1").createOrReplace(); - assertEquals(pod1, response); + KubernetesClient client = server.getClient(); + HasMetadata response = client.resource(pod1).inNamespace("ns1").createOrReplace(); + assertEquals(pod1, response); } @Test @@ -105,7 +104,6 @@ void testCreateOrReplaceWithDeleteExisting() throws Exception { Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, pod1).once(); - server.expect().get().withPath("/api/v1/namespaces/ns1/pods/pod1").andReturn(HttpURLConnection.HTTP_OK, pod1).once(); server.expect().delete().withPath("/api/v1/namespaces/ns1/pods/pod1").andReturn(HttpURLConnection.HTTP_OK, pod1).once(); server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); @@ -114,7 +112,7 @@ void testCreateOrReplaceWithDeleteExisting() throws Exception { assertEquals(pod1, response); RecordedRequest request = server.getLastRequest(); - assertEquals(4, server.getMockServer().getRequestCount()); + assertEquals(3, server.getMockServer().getRequestCount()); assertEquals("/api/v1/namespaces/ns1/pods", request.getPath()); assertEquals("POST", request.getMethod()); }