diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java index f8552487ea..391bee61e9 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/ApplyService.java @@ -79,7 +79,6 @@ import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getOrCreateLabels; import static org.eclipse.jkube.kit.common.util.KubernetesHelper.getOrCreateMetadata; - /** * Applies DTOs to the current Kubernetes master */ @@ -244,9 +243,8 @@ public void applyOAuthClient(OAuthClient entity, String sourceName) { protected void doCreateOAuthClient(OAuthClient entity, String sourceName) { OpenShiftClient openShiftClient = getOpenShiftClient(); if (openShiftClient != null) { - Object result = null; try { - result = openShiftClient.oAuthClients().create(entity); + openShiftClient.oAuthClients().create(entity); } catch (Exception e) { onApplyError("Failed to create OAuthClient from " + sourceName + ". " + e + ". " + entity, e); } @@ -402,13 +400,7 @@ public void applyPersistentVolumeClaim(PersistentVolumeClaim entity, String sour doCreatePersistentVolumeClaim(entity, namespace, sourceName); } } else { - log.info("Updating a PersistentVolumeClaim from " + sourceName); - try { - HasMetadata answer = patchService.compareAndPatchEntity(namespace, entity, old); - logGeneratedEntity("Updated PersistentVolumeClaim: ", namespace, entity, answer); - } catch (Exception e) { - onApplyError("Failed to update PersistentVolumeClaim from " + sourceName + ". " + e + ". " + entity, e); - } + doPatchEntity(old, entity, namespace, sourceName); } } } else { @@ -438,13 +430,7 @@ public void applyCustomResourceDefinition(CustomResourceDefinition entity, Strin kubernetesClient.customResourceDefinitions().withName(id).delete(); doCreateCustomResourceDefinition(entity, sourceName); } else { - log.info("Updating a Custom Resource Definition from " + sourceName + " name " + getName(entity)); - try { - HasMetadata answer = patchService.compareAndPatchEntity(namespace, entity, old); - log.info("Updated Custom Resource Definition result: " + getName(answer)); - } catch (Exception e) { - onApplyError("Failed to update Custom Resource Definition from " + sourceName + ". " + e + ". " + entity, e); - } + doPatchEntity(old, entity, namespace, sourceName); } } } else { @@ -459,8 +445,8 @@ public void applyCustomResourceDefinition(CustomResourceDefinition entity, Strin private void doCreateCustomResourceDefinition(CustomResourceDefinition entity, String sourceName) { log.info("Creating a Custom Resource Definition from " + sourceName + " name " + getName(entity)); try { - Object answer = kubernetesClient.customResourceDefinitions().create(entity); - log.info("Created Custom Resource Definition result: " + ((CustomResourceDefinition) answer).getMetadata().getName()); + CustomResourceDefinition answer = kubernetesClient.customResourceDefinitions().create(entity); + log.info("Created Custom Resource Definition result: " + answer.getMetadata().getName()); } catch (Exception e) { onApplyError("Failed to create Custom Resource Definition from " + sourceName + ". " + e + ". " + entity, e); } @@ -535,19 +521,12 @@ public void applySecret(Secret secret, String sourceName) throws Exception { // if the secret already exists and is the same, then do nothing if (UserConfigurationCompare.configEqual(secret, old)) { log.info("Secret has not changed so not doing anything"); - return; } else { if (isRecreateMode()) { kubernetesClient.secrets().inNamespace(namespace).withName(id).delete(); doCreateSecret(secret, namespace, sourceName); } else { - log.info("Updating a Secret from " + sourceName); - try { - Object answer = patchService.compareAndPatchEntity(namespace, secret, old); - logGeneratedEntity("Updated Secret:", namespace, secret, answer); - } catch (Exception e) { - onApplyError("Failed to update secret from " + sourceName + ". " + e + ". " + secret, e); - } + doPatchEntity(old, secret, namespace, sourceName); } } } else { @@ -559,7 +538,6 @@ public void applySecret(Secret secret, String sourceName) throws Exception { } } - protected void doCreateSecret(Secret secret, String namespace, String sourceName) { log.info("Creating a Secret from " + sourceName + " namespace " + namespace + " name " + getName(secret)); try { @@ -642,7 +620,6 @@ public Object processTemplate(Template entity, String sourceName) { } } - public void applyRoute(Route entity, String sourceName) { OpenShiftClient openShiftClient = getOpenShiftClient(); if (openShiftClient != null) { @@ -652,21 +629,47 @@ public void applyRoute(Route entity, String sourceName) { if (StringUtils.isBlank(namespace)) { namespace = getNamespace(); } + if (isServicesOnlyMode()) { + log.debug("Ignoring Route: " + namespace + ":" + id); + return; + } Route route = openShiftClient.routes().inNamespace(namespace).withName(id).get(); - if (route == null) { - try { - log.info("Creating Route " + namespace + ":" + id + " " + - (entity.getSpec() != null ? - "host: " + entity.getSpec().getHost() : - "No Spec !")); - openShiftClient.routes().inNamespace(namespace).create(entity); - } catch (Exception e) { - onApplyError("Failed to create Route from " + sourceName + ". " + e + ". " + entity, e); + if (isRunning(route)) { + if (UserConfigurationCompare.configEqual(entity, route)) { + log.info("Route has not changed so not doing anything"); + } else { + if (isRecreateMode()) { + log.info("Deleting Route: " + id); + openShiftClient.routes().inNamespace(namespace).withName(id).delete(); + doCreateRoute(entity, namespace, sourceName); + } else { + doPatchEntity(route, entity, namespace, sourceName); + } + } + } else { + if (!isAllowCreate()) { + log.warn("Creation disabled so not creating a Route from " + sourceName + " namespace " + namespace + " name " + id); + } else { + doCreateRoute(entity, namespace, sourceName); } } } } + private void doCreateRoute(Route entity, String namespace, String sourceName) { + OpenShiftClient openShiftClient = getOpenShiftClient(); + String id = getName(entity); + try { + log.info("Creating Route " + namespace + ":" + id + " " + + (entity.getSpec() != null ? + "host: " + entity.getSpec().getHost() : + "No Spec !")); + openShiftClient.routes().inNamespace(namespace).create(entity); + } catch (Exception e) { + onApplyError("Failed to create Route from " + sourceName + ". " + e + ". " + entity, e); + } + } + public void applyBuildConfig(BuildConfig entity, String sourceName) { OpenShiftClient openShiftClient = getOpenShiftClient(); if (openShiftClient != null) { @@ -688,17 +691,7 @@ public void applyBuildConfig(BuildConfig entity, String sourceName) { openShiftClient.buildConfigs().inNamespace(namespace).withName(id).delete(); doCreateBuildConfig(entity, namespace, sourceName); } else { - log.info("Updating BuildConfig from " + sourceName); - try { - String resourceVersion = KubernetesHelper.getResourceVersion(old); - ObjectMeta metadata = getOrCreateMetadata(entity); - metadata.setNamespace(namespace); - metadata.setResourceVersion(resourceVersion); - Object answer = patchService.compareAndPatchEntity(namespace, entity, old); - logGeneratedEntity("Updated BuildConfig: ", namespace, entity, answer); - } catch (Exception e) { - onApplyError("Failed to update BuildConfig from " + sourceName + ". " + e + ". " + entity, e); - } + doPatchEntity(old, entity, namespace, sourceName); } } } else { @@ -868,13 +861,7 @@ public void applyService(Service service, String sourceName) throws Exception { kubernetesClient.services().inNamespace(namespace).withName(id).delete(); doCreateService(service, namespace, sourceName); } else { - log.info("Updating a Service from " + sourceName); - try { - Object answer = patchService.compareAndPatchEntity(namespace, service, old); - logGeneratedEntity("Updated Service: ", namespace, service, answer); - } catch (Exception e) { - onApplyError("Failed to update Service from " + sourceName + ". " + e + ". " + service, e); - } + doPatchEntity(old, service, namespace, sourceName); } } } else { @@ -939,6 +926,17 @@ protected void doCreateResource(T resource, String n } } + private void doPatchEntity(T oldEntity, T newEntity, String namespace, String sourceName) { + String kind = newEntity.getKind(); + log.info("Updating {} from {}", kind, sourceName); + try { + Object answer = patchService.compareAndPatchEntity(namespace, newEntity, oldEntity); + logGeneratedEntity("Updated " + kind + ": ", namespace, newEntity, answer); + } catch (Exception e) { + onApplyError("Failed to update " + kind + " from " + sourceName + ". " + e + ". " + newEntity, e); + } + } + protected void doCreateService(Service service, String namespace, String sourceName) { log.info("Creating a Service from " + sourceName + " namespace " + namespace + " name " + getName(service)); try { @@ -1185,13 +1183,7 @@ public void applyPod(Pod pod, String sourceName) throws Exception { kubernetesClient.pods().inNamespace(namespace).withName(id).delete(); doCreatePod(pod, namespace, sourceName); } else { - log.info("Updating a Pod from " + sourceName + " namespace " + namespace + " name " + getName(pod)); - try { - Object answer = patchService.compareAndPatchEntity(namespace, pod, old); - log.info("Updated Pod result: " + answer); - } catch (Exception e) { - onApplyError("Failed to update Pod from " + sourceName + ". " + e + ". " + pod, e); - } + doPatchEntity(old, pod, namespace, sourceName); } } } else { diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PatchService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PatchService.java index 64be3bee29..ea9b329986 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PatchService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/PatchService.java @@ -32,7 +32,9 @@ import io.fabric8.openshift.api.model.BuildConfig; import io.fabric8.openshift.api.model.DoneableBuildConfig; import io.fabric8.openshift.api.model.DoneableImageStream; +import io.fabric8.openshift.api.model.DoneableRoute; import io.fabric8.openshift.api.model.ImageStream; +import io.fabric8.openshift.api.model.Route; import io.fabric8.openshift.client.OpenShiftClient; import org.eclipse.jkube.kit.common.KitLogger; import org.eclipse.jkube.kit.common.util.OpenshiftHelper; @@ -47,7 +49,6 @@ public class PatchService { private static Map> patchers; - // Interface for patching entities interface EntityPatcher { @@ -63,7 +64,6 @@ interface EntityPatcher { T patch(KubernetesClient client, String namespace, T newEntity, T oldEntity); } - static { patchers = new HashMap<>(); patchers.put("Pod", podPatcher()); @@ -75,15 +75,14 @@ interface EntityPatcher { patchers.put("PersistentVolumeClaim", pvcPatcher()); patchers.put("CustomResourceDefinition", crdPatcher()); patchers.put("Job", jobPatcher()); + patchers.put("Route", routePatcher()); } - public PatchService(KubernetesClient client, KitLogger log) { this.kubernetesClient = client; this.log = log; } - public T compareAndPatchEntity(String namespace, T newDto, T oldDto) { EntityPatcher dispatcher = (EntityPatcher) patchers.get(newDto.getKind()); if (dispatcher == null) { @@ -113,7 +112,7 @@ private static EntityPatcher podPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { - entity.withSpec(newObj.getSpec()); + entity.withSpec(newObj.getSpec()); } return entity.done(); }; @@ -136,7 +135,7 @@ private static EntityPatcher rcPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { - entity.withSpec(newObj.getSpec()); + entity.withSpec(newObj.getSpec()); } return entity.done(); }; @@ -181,7 +180,7 @@ private static EntityPatcher secretPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getData(), oldObj.getData())) { - entity.withData(newObj.getData()); + entity.withData(newObj.getData()); } if(!UserConfigurationCompare.configEqual(newObj.getStringData(), oldObj.getStringData())) { entity.withStringData(newObj.getStringData()); @@ -206,7 +205,7 @@ private static EntityPatcher pvcPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { - entity.withSpec(newObj.getSpec()); + entity.withSpec(newObj.getSpec()); } return entity.done(); }; @@ -282,7 +281,7 @@ private static EntityPatcher bcPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { - entity.withSpec(newObj.getSpec()); + entity.withSpec(newObj.getSpec()); } return entity.done(); }; @@ -308,10 +307,37 @@ private static EntityPatcher isPatcher() { } if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { - entity.withSpec(newObj.getSpec()); + entity.withSpec(newObj.getSpec()); } return entity.done(); }; } + private static EntityPatcher routePatcher() { + return (client, namespace, newObj, oldObj) -> { + if (UserConfigurationCompare.configEqual(newObj, oldObj)) { + return oldObj; + } + OpenShiftClient openShiftClient = OpenshiftHelper.asOpenShiftClient(client); + if (openShiftClient == null) { + throw new IllegalArgumentException("Route can only be patched when connected to an OpenShift cluster"); + } + + DoneableRoute entity = openShiftClient.routes() + .inNamespace(namespace) + .withName(oldObj.getMetadata().getName()) + .edit(); + + if (!UserConfigurationCompare.configEqual(newObj.getMetadata(), oldObj.getMetadata())) { + entity.withMetadata(newObj.getMetadata()); + } + + if(!UserConfigurationCompare.configEqual(newObj.getSpec(), oldObj.getSpec())) { + entity.withSpec(newObj.getSpec()); + } + + return entity.done(); + }; + } + } diff --git a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/ApplyServiceTest.java b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/ApplyServiceTest.java new file mode 100644 index 0000000000..9c420d3e3c --- /dev/null +++ b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/ApplyServiceTest.java @@ -0,0 +1,96 @@ +package org.eclipse.jkube.kit.config.service; + +import io.fabric8.openshift.api.model.Route; +import io.fabric8.openshift.api.model.RouteBuilder; +import io.fabric8.openshift.client.server.mock.OpenShiftMockServer; +import mockit.Mocked; +import org.eclipse.jkube.kit.common.KitLogger; +import org.eclipse.jkube.kit.config.service.openshift.WebServerEventCollector; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class ApplyServiceTest { + + @Mocked + private KitLogger log; + + private OpenShiftMockServer mockServer = new OpenShiftMockServer(false); + + private ApplyService applyService; + + @Before + public void setUp() { + applyService = new ApplyService(mockServer.createOpenShiftClient(), log); + } + + @Test + public void testCreateRoute() throws Exception { + Route route = buildRoute(); + + WebServerEventCollector collector = new WebServerEventCollector<>(mockServer); + mockServer.expect().get() + .withPath("/apis/route.openshift.io/v1/namespaces/default/routes/route") + .andReply(collector.record("get-route").andReturn(404, "")) + .always(); + mockServer.expect().post() + .withPath("/apis/route.openshift.io/v1/namespaces/default/routes") + .andReply(collector.record("new-route").andReturn(201, route)) + .once(); + + applyService.apply(route, "route.yml"); + + collector.assertEventsRecordedInOrder("get-route", "new-route"); + } + + @Test + public void testCreateRouteInServiceOnlyMode() throws Exception { + Route route = buildRoute(); + + WebServerEventCollector collector = new WebServerEventCollector<>(mockServer); + mockServer.expect().get() + .withPath("/apis/route.openshift.io/v1/namespaces/default/routes/route") + .andReply(collector.record("get-route").andReturn(404, "")) + .always(); + + applyService.setServicesOnlyMode(true); + applyService.apply(route, "route.yml"); + + collector.assertEventsNotRecorded("get-route"); + assertEquals(1, mockServer.getRequestCount()); + } + + @Test + public void testCreateRouteNotAllowed() throws Exception { + Route route = buildRoute(); + + WebServerEventCollector collector = new WebServerEventCollector<>(mockServer); + mockServer.expect().get() + .withPath("/apis/route.openshift.io/v1/namespaces/default/routes/route") + .andReply(collector.record("get-route").andReturn(404, "")) + .always(); + + applyService.setAllowCreate(false); + applyService.apply(route, "route.yml"); + + collector.assertEventsRecordedInOrder("get-route"); + assertEquals(2, mockServer.getRequestCount()); + } + + private Route buildRoute() { + return new RouteBuilder() + .withNewMetadata() + .withName("route") + .endMetadata() + .withNewSpec() + .withHost("www.example.com") + .withNewTo() + .withKind("Service") + .withName("frontend") + .endTo() + .endSpec() + .build(); + } + +} \ No newline at end of file diff --git a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/PatchServiceTest.java b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/PatchServiceTest.java index 6d9a80adbc..be795eaabd 100644 --- a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/PatchServiceTest.java +++ b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/PatchServiceTest.java @@ -20,12 +20,14 @@ import io.fabric8.kubernetes.api.model.SecretBuilder; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; -import io.fabric8.openshift.client.OpenShiftClient; +import io.fabric8.openshift.api.model.Route; +import io.fabric8.openshift.api.model.RouteBuilder; import io.fabric8.openshift.client.server.mock.OpenShiftMockServer; import org.eclipse.jkube.kit.common.KitLogger; import org.eclipse.jkube.kit.common.util.UserConfigurationCompare; import org.eclipse.jkube.kit.config.service.PatchService; import mockit.Mocked; +import org.junit.Before; import org.junit.Test; import java.util.Collections; @@ -35,9 +37,16 @@ public class PatchServiceTest { @Mocked - KitLogger log; + private KitLogger log; - OpenShiftMockServer mockServer = new OpenShiftMockServer(false); + private OpenShiftMockServer mockServer = new OpenShiftMockServer(false); + + private PatchService patchService; + + @Before + public void setUp() { + patchService = new PatchService(mockServer.createOpenShiftClient(), log); + } @Test public void testResourcePatching() { @@ -61,9 +70,6 @@ public void testResourcePatching() { mockServer.expect().get().withPath("/api/v1/namespaces/test/services/service1").andReturn(200, oldService).always(); mockServer.expect().patch().withPath("/api/v1/namespaces/test/services/service1").andReturn(200, new ServiceBuilder().withMetadata(newService.getMetadata()).withSpec(oldService.getSpec()).build()).once(); - OpenShiftClient client = mockServer.createOpenShiftClient(); - PatchService patchService = new PatchService(client, log); - Service patchedService = patchService.compareAndPatchEntity("test", newService, oldService); assertTrue(UserConfigurationCompare.configEqual(patchedService.getMetadata(), newService.getMetadata())); @@ -87,16 +93,50 @@ public void testSecretPatching() { .andReturn(200, new SecretBuilder().withMetadata(newSecret.getMetadata()) .addToStringData(oldSecret.getData()).build())).once(); - OpenShiftClient client = mockServer.createOpenShiftClient(); - - PatchService patchService = new PatchService(client, log); - patchService.compareAndPatchEntity("test", newSecret, oldSecret); collector.assertEventsRecordedInOrder("get-secret", "get-secret", "patch-secret"); assertEquals("[{\"op\":\"remove\",\"path\":\"/data\"},{\"op\":\"add\",\"path\":\"/stringData\",\"value\":{\"test\":\"test\"}}]", collector.getBodies().get(2)); } + @Test + public void testRoutePatching() { + Route oldRoute = new RouteBuilder() + .withNewMetadata() + .withName("route") + .endMetadata() + .withNewSpec() + .withHost("www.example.com") + .withNewTo() + .withKind("Service") + .withName("frontend") + .endTo() + .endSpec() + .build(); + Route newRoute = new RouteBuilder() + .withNewMetadata() + .withName("route") + .addToAnnotations("haproxy.router.openshift.io/balance", "roundrobin") + .endMetadata() + .withSpec(oldRoute.getSpec()) + .build(); + + mockServer.expect().get() + .withPath("/apis/route.openshift.io/v1/namespaces/test/routes/route") + .andReturn(200, oldRoute) + .always(); + mockServer.expect().patch() + .withPath("/apis/route.openshift.io/v1/namespaces/test/routes/route") + .andReturn(200, new RouteBuilder() + .withMetadata(newRoute.getMetadata()) + .withSpec(oldRoute.getSpec()) + .build()) + .once(); + + Route patchedRoute = patchService.compareAndPatchEntity("test", newRoute, oldRoute); + assertTrue(UserConfigurationCompare.configEqual(patchedRoute, newRoute)); + } + @Test(expected = IllegalArgumentException.class) public void testInvalidPatcherKind() { ConfigMap oldResource = new ConfigMapBuilder() @@ -108,9 +148,7 @@ public void testInvalidPatcherKind() { .addToData(Collections.singletonMap("FOO", "BAR")) .build(); - OpenShiftClient client = mockServer.createOpenShiftClient(); - PatchService patchService = new PatchService(client, log); - patchService.compareAndPatchEntity("test", newResource, oldResource); } + }