Skip to content

Commit

Permalink
Fix fabric8io#2292: Update BaseOperation#createOrReplace()
Browse files Browse the repository at this point in the history
  • Loading branch information
@rohanKanojia
rohanKanojia committed Jul 24, 2020
1 parent 5db2453 commit a6e7352
Show file tree
Hide file tree
Showing 13 changed files with 119 additions and 76 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Expand Up @@ -4,10 +4,11 @@
#### Bugs

#### Improvements
* Fix #2292: Update createOrReplace to do replace when create fails with conflict

#### Dependency Upgrade
* Fix #2355: bump jandex from 2.1.3.Final to 2.2.0.Final
* Fix #2353: bump workflow action-setup-* versions + kubernetes to 1.18.6
* Fix #2353: bump workflow action-setup- versions + kubernetes to 1.18.6

#### New Features
* Fix #2287: Add support for V1 and V1Beta1 CustomResourceDefinition
Expand Down
13 changes: 13 additions & 0 deletions kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CreateOrReplaceable.java
View file
Expand Up @@ -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();
}
26 changes: 17 additions & 9 deletions kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java
View file
Expand Up @@ -395,24 +395,32 @@ 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
return create(itemToCreateOrReplace);
} catch (KubernetesClientException exception) {
if (exception.getCode() != HttpURLConnection.HTTP_CONFLICT) {
throw exception;
}

// Conflict; Do Replace
return replace(itemToCreateOrReplace);
}
}

Expand Down
31 changes: 19 additions & 12 deletions ...client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java
View file
Expand Up @@ -17,7 +17,10 @@

import io.fabric8.kubernetes.api.model.DeletionPropagation;
import io.fabric8.kubernetes.api.model.ListOptions;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.utils.Utils;

import java.net.HttpURLConnection;
import java.util.function.Predicate;

import org.slf4j.Logger;
Expand Down Expand Up @@ -137,22 +140,26 @@ 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) {
try {
// Create
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
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 {
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);
}
}

Expand Down
36 changes: 16 additions & 20 deletions ...nt/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java
View file
Expand Up @@ -42,6 +42,7 @@
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;
Expand Down Expand Up @@ -261,30 +262,25 @@ 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);
try {
// Create
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
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 {
result.add(h.replace(client, config, namespaceToUse, meta));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java
View file
Expand Up @@ -34,8 +34,8 @@ public class ResourceCompare {

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);
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);
Expand Down
38 changes: 22 additions & 16 deletions ...es-tests/src/test/java/io/fabric8/kubernetes/client/mock/CreateOrReplaceResourceTest.java
View file
Expand Up @@ -26,11 +26,11 @@
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.server.mock.KubernetesServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

import java.net.HttpURLConnection;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -43,11 +43,14 @@ public class CreateOrReplaceResourceTest {
public KubernetesServer server = new KubernetesServer();

@Test
public void testResourceReplace() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(200, new PodBuilder()
public void testResourceReplace() {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(200, new PodBuilder()
server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).once();

KubernetesClient client = server.getClient();
Expand All @@ -57,7 +60,7 @@ public void testResourceReplace() throws Exception {
}

@Test
public void testResourceCreate() throws Exception {
public void testResourceCreate() {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(404, new StatusBuilder().build()).always();

server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(201, new PodBuilder()
Expand All @@ -70,7 +73,7 @@ public void testResourceCreate() throws Exception {
}

@Test
public void testCreate() throws Exception {
public void testCreate() {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(404, new StatusBuilder().build()).always();

server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(201, new PodBuilder()
Expand All @@ -83,10 +86,12 @@ public void testCreate() throws Exception {
}

@Test
public void testReplace() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(200, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();
public void testReplace() {
server.expect().get().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(200, new PodBuilder()
server.expect().put().withPath("/api/v1/namespaces/test/pods/pod123").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).once();

KubernetesClient client = server.getClient();
Expand All @@ -110,18 +115,18 @@ public void testResourceCreateFromLoad() throws Exception {
assertEquals("12345", pod.getMetadata().getResourceVersion());

RecordedRequest request = server.getMockServer().takeRequest();
assertEquals("/api/v1/namespaces/test/pods/nginx", request.getPath());

request = server.getMockServer().takeRequest();
assertEquals("/api/v1/namespaces/test/pods", request.getPath());
Pod requestPod = new ObjectMapper().readerFor(Pod.class).readValue(request.getBody().inputStream());
assertEquals("nginx", requestPod.getMetadata().getName());
}

@Test
public void testResourceReplaceFromLoad() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(200, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();
server.expect().get().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, new PodBuilder().withNewMetadata().withResourceVersion("12345").and().build()).always();

server.expect().put().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(200, new PodBuilder()
server.expect().put().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).once();

KubernetesClient client = server.getClient();
Expand Down Expand Up @@ -156,9 +161,10 @@ public void testCreateFromLoad() throws Exception {

@Test
public void testReplaceFromLoad() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(200, new PodBuilder().build()).always();
server.expect().get().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder().build()).always();
server.expect().post().withPath("/api/v1/namespaces/test/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, new PodBuilder().build()).always();

server.expect().put().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(200, new PodBuilder()
server.expect().put().withPath("/api/v1/namespaces/test/pods/nginx").andReturn(HttpURLConnection.HTTP_OK, new PodBuilder()
.withNewMetadata().withResourceVersion("12345").and().build()).once();

KubernetesClient client = server.getClient();
Expand Down
8 changes: 4 additions & 4 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/JobTest.java
View file
Expand Up @@ -16,7 +16,6 @@

package io.fabric8.kubernetes.client.mock;

import io.fabric8.kubernetes.api.model.OwnerReference;
import io.fabric8.kubernetes.api.model.OwnerReferenceBuilder;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodBuilder;
Expand Down Expand Up @@ -242,7 +241,6 @@ public void testDeleteWithNamespaceMismatch() {
public void testCreateWithNameMismatch() {
Assertions.assertThrows(KubernetesClientException.class, () -> {
Job job1 = new JobBuilder().withNewMetadata().withName("job1").withNamespace("test").and().build();
Job job2 = new JobBuilder().withNewMetadata().withName("job2").withNamespace("ns1").and().build();
KubernetesClient client = server.getClient();

client.batch().jobs().inNamespace("test1").withName("myjob1").create(job1);
Expand All @@ -265,9 +263,11 @@ public void testCreateOrReplaceWithExistingJob() {
.build();

server.expect().get().withPath("/apis/batch/v1/namespaces/test/jobs/job1")
.andReturn(200, jobExistingInServer).always();
.andReturn(HttpURLConnection.HTTP_OK, jobExistingInServer).always();
server.expect().post().withPath("/apis/batch/v1/namespaces/test/jobs")
.andReturn(HttpURLConnection.HTTP_CONFLICT, jobExistingInServer).once();
server.expect().put().withPath("/apis/batch/v1/namespaces/test/jobs/job1")
.andReturn(200, getJobBuilder()
.andReturn(HttpURLConnection.HTTP_OK, getJobBuilder()
.editOrNewMetadata().addToLabels("foo", "bar").addToLabels("foo1", "bar1").endMetadata()
.editSpec()
.editOrNewTemplate().editOrNewMetadata()
Expand Down
9 changes: 6 additions & 3 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceListTest.java
View file
Expand Up @@ -35,6 +35,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

import java.net.HttpURLConnection;
import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -122,23 +123,25 @@ public void testDelete() {
public void testCreateOrReplaceWithoutDeleteExisting() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(200 , service).times(2);
server.expect().get().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(200, configMap).times(2);
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().put().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(200, updatedService).once();
server.expect().put().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(200, updatedConfigMap).once();

KubernetesClient client = server.getClient();
KubernetesList list = new KubernetesListBuilder().withItems(updatedService, updatedConfigMap).build();
client.resourceList(list).inNamespace("ns1").createOrReplace();

assertEquals(6, server.getMockServer().getRequestCount());
assertEquals(7, server.getMockServer().getRequestCount());
RecordedRequest request = server.getLastRequest();
assertEquals("/api/v1/namespaces/ns1/configmaps/my-configmap", request.getPath());
assertEquals("PUT", request.getMethod());
}

@Test
public void testCreateOrReplaceWithDeleteExisting() throws Exception {
server.expect().get().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(200, service).once();
server.expect().get().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(200, configMap).once();
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().delete().withPath("/api/v1/namespaces/ns1/services/my-service").andReturn(200 , service).once();
server.expect().delete().withPath("/api/v1/namespaces/ns1/configmaps/my-configmap").andReturn(200, configMap).once();
server.expect().post().withPath("/api/v1/namespaces/ns1/services").andReturn(200, updatedService).once();
Expand Down
6 changes: 3 additions & 3 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java
View file
Expand Up @@ -86,9 +86,9 @@ public void testCreateWithExplicitNamespace() {
public void testCreateOrReplaceWithDeleteExisting() throws Exception {
Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build();

server.expect().get().withPath("/api/v1/namespaces/ns1/pods/pod1").andReturn(200, pod1).once();
server.expect().delete().withPath("/api/v1/namespaces/ns1/pods/pod1").andReturn(200, pod1).once();
server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(201, pod1).once();
server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CONFLICT, 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();

KubernetesClient client = server.getClient();
HasMetadata response = client.resource(pod1).inNamespace("ns1").deletingExisting().createOrReplace();
Expand Down
9 changes: 7 additions & 2 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ServiceTest.java
View file
Expand Up @@ -87,12 +87,17 @@ void testReplace() throws InterruptedException {

server.expect().get()
.withPath("/api/v1/namespaces/test/services/httpbin")
.andReturn(200, serviceFromServer)
.andReturn(HttpURLConnection.HTTP_OK, serviceFromServer)
.times(3);

server.expect().post()
.withPath("/api/v1/namespaces/test/services")
.andReturn(HttpURLConnection.HTTP_CONFLICT, serviceFromServer)
.once();

server.expect().put()
.withPath("/api/v1/namespaces/test/services/httpbin")
.andReturn(200, serviceFromServer)
.andReturn(HttpURLConnection.HTTP_OK, serviceFromServer)
.once();

KubernetesClient client = server.getClient();
Expand Down

0 comments on commit a6e7352

Please sign in to comment.