Skip to content

Commit

Permalink
additional changes for fabric8io#3089 and fabric8io#3066
Browse files Browse the repository at this point in the history
this consolidated the base patch support logic to a single method, adds
an applyStatus, and refines the patch(base, item) behavior when null
  • Loading branch information
shawkins committed May 10, 2021
1 parent abd922c commit f864014
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 160 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public interface Patchable<T> {
* as the comparison for the patch.
*
* @param item item to be patched with patched values
* @param item base to be compared with to generate the patch, use null for the current
* @param base item to be compared with to generate the patch, use null to create a merge patch from item
* @return returns deserialized version of api server response
*/
T patch(T base, T item);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,14 @@ public interface StatusUpdatable<T> {
* @return updated object
*/
T updateStatus(T item);

/**
* When the status subresource is enabled, the /status subresource for the custom resource is exposed.
* It does a PATCH requests to the /status subresource take a resource object and ignore changes
* to anything except the status stanza.
*
* @param item kubernetes object
* @return updated object
*/
T applyStatus(T item);
}
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ public Boolean delete(List<T> items) {
@Override
public T updateStatus(T item) {
try {
return handleStatusUpdate(item, getType());
return handleReplace(item, true);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
throw KubernetesClientException.launderThrowable(forOperationType("statusUpdate"), ie);
Expand All @@ -754,6 +754,11 @@ public T updateStatus(T item) {
}

}

@Override
public T applyStatus(T item) {
throw new KubernetesClientException(READ_ONLY_UPDATE_EXCEPTION_MESSAGE);
}

public BaseOperation<T, L, R> withItem(T item) {
return newInstance(context.withItem(item));
Expand Down Expand Up @@ -883,14 +888,14 @@ protected T handleCreate(T resource) throws ExecutionException, InterruptedExcep
return handleCreate(resource, getType());
}

protected T handleReplace(T updated) throws ExecutionException, InterruptedException, IOException {
protected T handleReplace(T updated, boolean status) throws ExecutionException, InterruptedException, IOException {
updateApiVersion(updated);
return handleReplace(updated, getType());
return handleReplace(updated, getType(), status);
}

protected T handlePatch(T current, T updated) throws ExecutionException, InterruptedException, IOException {
protected T handlePatch(T current, T updated, boolean status) throws ExecutionException, InterruptedException, IOException {
updateApiVersion(updated);
return handlePatch(current, updated, getType());
return handlePatch(current, updated, getType(), status);
}

protected T handlePatch(T current, Map<String, Object> patchedUpdate) throws ExecutionException, InterruptedException, IOException {
Expand All @@ -900,7 +905,7 @@ protected T handlePatch(T current, Map<String, Object> patchedUpdate) throws Exe

protected T sendPatchedObject(T oldObject, T updatedObject) {
try {
return handlePatch(oldObject, updatedObject);
return handlePatch(oldObject, updatedObject, false);
} catch (InterruptedException interruptedException) {
Thread.currentThread().interrupt();
throw KubernetesClientException.launderThrowable(interruptedException);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.Gettable;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.utils.Serialization;

Expand All @@ -48,15 +49,15 @@ public HasMetadataOperation(OperationContext ctx) {
@Override
public T edit(UnaryOperator<T> function) {
T item = getMandatory();
return patch(Serialization.clone(item), function.apply(item));
return patch(Serialization.clone(item), function.apply(item), false);
}

@Override
public T accept(Consumer<T> consumer) {
T item = getMandatory();
T clone = Serialization.clone(item);
consumer.accept(item);
return patch(clone, item);
return patch(clone, item, false);
}

protected VisitableBuilder<T, ?> createVisitableBuilder(T item) {
Expand All @@ -67,7 +68,20 @@ public T accept(Consumer<T> consumer) {
public T edit(Visitor... visitors) {
T item = getMandatory();
T clone = Serialization.clone(item);
return patch(clone, createVisitableBuilder(item).accept(visitors).build());
return patch(clone, createVisitableBuilder(item).accept(visitors).build(), false);
}

/**
* Get the current item from the server, consulting the metadata for the name if needed
*/
protected Gettable<T> fromServer(ObjectMeta metadata) {
if (getName() != null) {
return fromServer();
}
if (metadata != null) {
return withName(metadata.getName()).fromServer();
}
throw new KubernetesClientException("Name not specified. But operation requires name.");
}

@Override
Expand All @@ -81,7 +95,7 @@ public T replace(T item) {
if (fixedResourceVersion != null) {
resourceVersion = fixedResourceVersion;
} else {
T got = fromServer().get();
T got = fromServer(item.getMetadata()).get();
if (got == null) {
return null;
}
Expand All @@ -95,7 +109,7 @@ public T replace(T item) {
final UnaryOperator<T> visitor = resource -> {
try {
resource.getMetadata().setResourceVersion(resourceVersion);
return handleReplace(resource);
return handleReplace(resource, false);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("replace"), e);
}
Expand Down Expand Up @@ -125,35 +139,37 @@ public T replace(T item) {

@Override
public T patch(T item) {
return patch(null, item);
return patch(fromServer(item.getMetadata()).get(), item, false);
}

@Override
public T patch(T base, T item) {
if (base == null) {
// no resource version specified, assume the latest
base = fromServer().get();
if (base == null) {
return null;
}
if (item.getMetadata() == null) {
item.setMetadata(new ObjectMeta());
}
if (base.getMetadata() != null) {
item.getMetadata().setResourceVersion(base.getMetadata().getResourceVersion());
}
}
// else could validate that the base / item are the same resource / resourceVersion
final T baseItem = base;
return patch(base, item, false);
}

public T patch(T base, T item, boolean status) {
final UnaryOperator<T> visitor = resource -> {
try {
return handlePatch(baseItem, resource);
if (base != null) {
if (item.getMetadata() == null) {
item.setMetadata(new ObjectMeta());
}
if (base.getMetadata() != null) {
item.getMetadata().setResourceVersion(base.getMetadata().getResourceVersion());
}
}
return handlePatch(base, resource, status);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType(PATCH_OPERATION), e);
}
};
return visitor.apply(item);
}

@Override
public T applyStatus(T item) {
return patch(null, item, true);
}

@Override
public T patch(PatchContext patchContext, String patch) {
Expand All @@ -162,7 +178,7 @@ public T patch(PatchContext patchContext, String patch) {
if (got == null) {
return null;
}
return handlePatch(patchContext, got, convertToJson(patch), getType());
return handlePatch(patchContext, got, convertToJson(patch), getType(), false);
} catch (InterruptedException interruptedException) {
Thread.currentThread().interrupt();
throw KubernetesClientException.launderThrowable(forOperationType(PATCH_OPERATION), interruptedException);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.fabric8.kubernetes.api.model.DeleteOptions;
import io.fabric8.kubernetes.api.model.DeletionPropagation;
Expand Down Expand Up @@ -155,9 +154,19 @@ public <T> URL getNamespacedUrl(T item) throws MalformedURLException {
}

public URL getResourceUrl(String namespace, String name) throws MalformedURLException {
return getResourceUrl(namespace, name, false);
}

public URL getResourceUrl(String namespace, String name, boolean status) throws MalformedURLException {
if (name == null) {
if (status) {
throw new KubernetesClientException("Name not specified. But operation requires name.");
}
return getNamespacedUrl(namespace);
}
if (status) {
return new URL(URLUtils.join(getNamespacedUrl(namespace).toString(), name, "status"));
}
return new URL(URLUtils.join(getNamespacedUrl(namespace).toString(), name));
}

Expand Down Expand Up @@ -287,15 +296,16 @@ protected <T, I> T handleCreate(I resource, Class<T> outputType) throws Executio
*
* @param updated updated object
* @param type type of the object provided
* @param status if this is only the status subresource
* @param <T> template argument provided
*
* @return returns de-serialized version of api server response
* @throws ExecutionException Execution Exception
* @throws InterruptedException Interrupted Exception
* @throws IOException IOException
*/
protected <T> T handleReplace(T updated, Class<T> type) throws ExecutionException, InterruptedException, IOException {
return handleReplace(updated, type, Collections.<String, String>emptyMap());
protected <T> T handleReplace(T updated, Class<T> type, boolean status) throws ExecutionException, InterruptedException, IOException {
return handleReplace(updated, type, Collections.<String, String>emptyMap(), status);
}

/**
Expand All @@ -304,43 +314,45 @@ protected <T> T handleReplace(T updated, Class<T> type) throws ExecutionExceptio
* @param updated updated object
* @param type type of object provided
* @param parameters a HashMap containing parameters for processing object
* @param status if this is only the status subresource
* @param <T> template argument provided
*
* @return returns de-serialized version of api server response.
* @throws ExecutionException Execution Exception
* @throws InterruptedException Interrupted Exception
* @throws IOException IOException
*/
protected <T> T handleReplace(T updated, Class<T> type, Map<String, String> parameters) throws ExecutionException, InterruptedException, IOException {
protected <T> T handleReplace(T updated, Class<T> type, Map<String, String> parameters, boolean status) throws ExecutionException, InterruptedException, IOException {
RequestBody body = RequestBody.create(JSON, JSON_MAPPER.writeValueAsString(updated));
Request.Builder requestBuilder = new Request.Builder().put(body).url(getResourceURLForWriteOperation(getResourceUrl(checkNamespace(updated), checkName(updated))));
Request.Builder requestBuilder = new Request.Builder().put(body).url(getResourceURLForWriteOperation(getResourceUrl(checkNamespace(updated), checkName(updated), status)));
return handleResponse(requestBuilder, type, parameters);
}

protected <T> T handleStatusUpdate(T updated, Class<T> type) throws ExecutionException, InterruptedException, KubernetesClientException, IOException {
RequestBody body = RequestBody.create(JSON, JSON_MAPPER.writeValueAsString(updated));
Request.Builder requestBuilder = new Request.Builder().put(body).url(getResourceUrl(checkNamespace(updated), checkName(updated)) + "/status");
return handleResponse(requestBuilder, type);
}

/**
* Send an http patch and handle the response.
*
* @param current current object
* @param updated updated object
* @param type type of object
* @param status if this is only the status subresource
* @param <T> template argument provided
*
* @return returns de-serialized version of api server response
* @throws ExecutionException Execution Exception
* @throws InterruptedException Interrupted Exception
* @throws IOException IOException
*/
protected <T> T handlePatch(T current, T updated, Class<T> type) throws ExecutionException, InterruptedException, IOException {
JsonNode diff = JsonDiff.asJson(patchMapper().valueToTree(current), patchMapper().valueToTree(updated));
RequestBody body = RequestBody.create(JSON_PATCH, JSON_MAPPER.writeValueAsString(diff));
Request.Builder requestBuilder = new Request.Builder().patch(body).url(getResourceURLForWriteOperation(getResourceUrl(checkNamespace(updated), checkName(updated))));
return handleResponse(requestBuilder, type, Collections.<String, String>emptyMap());
protected <T> T handlePatch(T current, T updated, Class<T> type, boolean status) throws ExecutionException, InterruptedException, IOException {
String patchForUpdate = null;
PatchType patchType = PatchType.JSON;
if (current != null) {
patchForUpdate = JSON_MAPPER.writeValueAsString(JsonDiff.asJson(patchMapper().valueToTree(current), patchMapper().valueToTree(updated)));
} else {
patchType = PatchType.JSON_MERGE;
patchForUpdate = Serialization.asJson(updated);
current = updated; // use the updated to determine the path
}
return handlePatch(new PatchContext.Builder().withPatchType(patchType).build(), current, patchForUpdate, type, status);
}

/**
Expand All @@ -357,9 +369,8 @@ protected <T> T handlePatch(T current, T updated, Class<T> type) throws Executio
* @throws IOException IOException
*/
protected <T> T handlePatch(T current, Map<String, Object> patchForUpdate, Class<T> type) throws ExecutionException, InterruptedException, IOException {
RequestBody body = RequestBody.create(STRATEGIC_MERGE_JSON_PATCH, JSON_MAPPER.writeValueAsString(patchForUpdate));
Request.Builder requestBuilder = new Request.Builder().patch(body).url(getResourceUrl(checkNamespace(current), checkName(current)));
return handleResponse(requestBuilder, type, Collections.<String, String>emptyMap());
return handlePatch(new PatchContext.Builder().withPatchType(PatchType.STRATEGIC_MERGE).build(), current,
JSON_MAPPER.writeValueAsString(patchForUpdate), type, false);
}

/**
Expand All @@ -369,16 +380,17 @@ protected <T> T handlePatch(T current, Map<String, Object> patchForUpdate, Class
* @param current current object
* @param patchForUpdate Patch string
* @param type type of object
* @param status if this is only the status subresource
* @param <T> template argument provided
* @return returns de-serialized version of api server response
* @throws ExecutionException Execution Exception
* @throws InterruptedException Interrupted Exception
* @throws IOException IOException in case of network errors
*/
protected <T> T handlePatch(PatchContext patchContext, T current, String patchForUpdate, Class<T> type) throws ExecutionException, InterruptedException, IOException {
protected <T> T handlePatch(PatchContext patchContext, T current, String patchForUpdate, Class<T> type, boolean status) throws ExecutionException, InterruptedException, IOException {
MediaType bodyMediaType = getMediaTypeFromPatchContextOrDefault(patchContext);
RequestBody body = RequestBody.create(bodyMediaType, patchForUpdate);
Request.Builder requestBuilder = new Request.Builder().patch(body).url(getResourceURLForPatchOperation(getResourceUrl(checkNamespace(current), checkName(current)), patchContext));
Request.Builder requestBuilder = new Request.Builder().patch(body).url(getResourceURLForPatchOperation(getResourceUrl(checkNamespace(current), checkName(current), status), patchContext));
return handleResponse(requestBuilder, type, Collections.emptyMap());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;

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

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -51,17 +54,29 @@ class DryRunTest {
public void setUp() throws IOException {
this.mockClient = Mockito.mock(OkHttpClient.class, Mockito.RETURNS_DEEP_STUBS);
Config config = new ConfigBuilder().withMasterUrl("https://localhost:8443/").build();
Call mockCall = mock(Call.class);
Response mockResponse = new Response.Builder()
.request(new Request.Builder().url("http://mock").build())
.protocol(Protocol.HTTP_1_1)
.code(HttpURLConnection.HTTP_OK)
.body(ResponseBody.create(MediaType.get("application/json"), "{}"))
.message("mock")
.build();
when(mockCall.execute())
.thenReturn(mockResponse);
when(mockClient.newCall(any())).thenReturn(mockCall);
when(mockClient.newCall(any())).then(new Answer<Call>() {
@Override
public Call answer(InvocationOnMock invocation) throws Throwable {
Call mockCall = mock(Call.class);
Request request = invocation.getArgument(0);
List<String> segments = request.url().encodedPathSegments();
String name = segments.get(segments.size() - 1);
String body = "{}";
if (!name.endsWith("s")) {
body = "{\"metadata\":{\"name\":\""+name+"\"}}";
}
Response mockResponse = new Response.Builder()
.request(new Request.Builder().url("http://mock").build())
.protocol(Protocol.HTTP_1_1)
.code(HttpURLConnection.HTTP_OK)
.body(ResponseBody.create(MediaType.get("application/json"), body))
.message("mock")
.build();
when(mockCall.execute())
.thenReturn(mockResponse);
return mockCall;
}
});
kubernetesClient = new DefaultKubernetesClient(mockClient, config);
}

Expand Down
Loading

0 comments on commit f864014

Please sign in to comment.