From aff25c44a823e9b995b7fe39071f352046099e25 Mon Sep 17 00:00:00 2001 From: Serhii Leshchenko Date: Tue, 2 Jan 2018 13:55:22 +0200 Subject: [PATCH 1/4] Add workspace information into bootstrapper debug logs --- .../openshift/bootstrapper/OpenShiftBootstrapper.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/bootstrapper/OpenShiftBootstrapper.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/bootstrapper/OpenShiftBootstrapper.java index added05a2f..b19b449f4e 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/bootstrapper/OpenShiftBootstrapper.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/bootstrapper/OpenShiftBootstrapper.java @@ -114,14 +114,16 @@ public class OpenShiftBootstrapper extends AbstractBootstrapper { } private void injectBootstrapper() throws InfrastructureException { - LOG.debug("Creating folder for bootstrapper"); + String machineName = openShiftMachine.getName(); + LOG.debug( + "Bootstrapping {}:{}. Creating folder for bootstrapper", runtimeIdentity, machineName); openShiftMachine.exec("mkdir", "-p", BOOTSTRAPPER_DIR); - LOG.debug("Downloading bootstrapper binary"); + LOG.debug("Bootstrapping {}:{}. Downloading bootstrapper binary", runtimeIdentity, machineName); openShiftMachine.exec( "curl", "-o", BOOTSTRAPPER_DIR + BOOTSTRAPPER_FILE, bootstrapperBinaryUrl); openShiftMachine.exec("chmod", "+x", BOOTSTRAPPER_DIR + BOOTSTRAPPER_FILE); - LOG.debug("Creating bootstrapper config file"); + LOG.debug("Bootstrapping {}:{}. Creating config file", runtimeIdentity, machineName); openShiftMachine.exec( "sh", "-c", From 995b7494a2a6ba907e65fab6abe11db9da488139 Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Thu, 14 Dec 2017 17:22:03 +0200 Subject: [PATCH 2/4] Remove redundant checking Runtime on null Since RuntimeContext **MUST** return non-null value checking Runtime on null is redundant --- .../che/api/workspace/server/WorkspaceRuntimes.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceRuntimes.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceRuntimes.java index 06b17bc617..23746fc932 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceRuntimes.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceRuntimes.java @@ -211,13 +211,8 @@ public class WorkspaceRuntimes { try { InternalEnvironment internalEnv = createInternalEnvironment(environment); RuntimeContext runtimeContext = infrastructure.prepare(runtimeId, internalEnv); - InternalRuntime runtime = runtimeContext.getRuntime(); - if (runtime == null) { - throw new IllegalStateException( - "SPI contract violated. RuntimeInfrastructure.start(...) must not return null: " - + RuntimeInfrastructure.class); - } + RuntimeState state = new RuntimeState(runtime, STARTING); if (isStartRefused.get()) { throw new ConflictException( From 925c17539ce40248fb36485fb317773d011b4af4 Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Thu, 14 Dec 2017 17:23:07 +0200 Subject: [PATCH 3/4] Fix java doc for RuntimeInfrastructure#prepare --- .../workspace/server/spi/RuntimeInfrastructure.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/RuntimeInfrastructure.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/RuntimeInfrastructure.java index 5d20436fda..67b18e0a23 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/RuntimeInfrastructure.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/spi/RuntimeInfrastructure.java @@ -79,9 +79,15 @@ public abstract class RuntimeInfrastructure { } /** - * Making Runtime is a two phase process. On the first phase implementation MUST prepare - * RuntimeContext, this is supposedly "fast" method On the second phase Runtime is created with - * RuntimeContext.start() which is supposedly "long" method. + * Starting the Runtime is a two phase process: + * + *
+   * 
    + *
  • On the first phase implementation MUST prepare RuntimeContext;
  • + *
  • On the second phase the Runtime that can be fetched from RuntimeContext + * should be started with InternalRuntime.start().
  • + *
+ *
* * @param id the RuntimeIdentity * @param environment incoming internal environment From 78630b82c17b7ef059dbcdd7711e2bd8c7cae676 Mon Sep 17 00:00:00 2001 From: Sergii Leshchenko Date: Thu, 4 Jan 2018 14:11:10 +0000 Subject: [PATCH 4/4] CHE-7922 Change OpenShift machines servers resolving approach --- .../infrastructure/openshift/Annotations.java | 15 +- .../openshift/OpenShiftInternalRuntime.java | 5 +- .../openshift/ServerExposer.java | 29 +++- .../openshift/ServerResolver.java | 101 ++++-------- .../openshift/AnnotationsTest.java | 5 + .../openshift/ServerExposerTest.java | 49 ++++-- .../openshift/ServerResolverTest.java | 151 +++++------------- 7 files changed, 159 insertions(+), 196 deletions(-) diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/Annotations.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/Annotations.java index c346d03680..87186cf62e 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/Annotations.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/Annotations.java @@ -22,8 +22,8 @@ import org.eclipse.che.api.core.model.workspace.config.ServerConfig; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; /** - * Helps to convert {@link ServerConfig} objects to OpenShift infrastructure annotations and - * vise-versa. + * Helps to convert servers related entities (like {@link ServerConfig} and machine name) to + * OpenShift annotations and vise-versa. * * @author Sergii Leshchenko */ @@ -37,6 +37,8 @@ public class Annotations { public static final String SERVER_ATTR_ANNOTATION_FMT = ANNOTATION_PREFIX + "server.%s.attributes"; + public static final String MACHINE_NAME_ANNOTATION = ANNOTATION_PREFIX + "machine.name"; + /** Pattern that matches server annotations e.g. "org.eclipse.che.server.exec-agent.port". */ private static final Pattern SERVER_ANNOTATION_PATTERN = Pattern.compile("org\\.eclipse\\.che\\.server\\.(?[\\w-/]+)\\..+"); @@ -86,6 +88,11 @@ public class Annotations { return this; } + public Serializer machineName(String machineName) { + annotations.put(MACHINE_NAME_ANNOTATION, machineName); + return this; + } + public Map annotations() { return annotations; } @@ -121,6 +128,10 @@ public class Annotations { } return servers; } + + public String machineName() { + return annotations.get(MACHINE_NAME_ANNOTATION); + } } private Annotations() {} diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java index 77d3f99c7a..1476a3d4c3 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftInternalRuntime.java @@ -240,12 +240,13 @@ public class OpenShiftInternalRuntime extends InternalRuntimeCreated services and routes will have serialized servers which are exposed by the + * corresponding object and machine name to which these servers belongs to. + * *

Container, service and route are linked in the following way: * *

@@ -144,6 +147,7 @@ public class ServerExposer {
     Service service =
         new ServiceBuilder()
             .withName(generate(SERVER_PREFIX, SERVER_UNIQUE_PART_SIZE) + '-' + machineName)
+            .withMachineName(machineName)
             .withSelectorEntry(CHE_ORIGINAL_NAME_LABEL, pod.getMetadata().getName())
             .withPorts(new ArrayList<>(portToServicePort.values()))
             .withServers(internalServers)
@@ -164,6 +168,7 @@ public class ServerExposer {
       Route route =
           new RouteBuilder()
               .withName(serviceName + '-' + servicePort.getName())
+              .withMachineName(machineName)
               .withTargetPort(servicePort.getName())
               .withServers(routesServers)
               .withTo(serviceName)
@@ -211,6 +216,7 @@ public class ServerExposer {
 
   private static class ServiceBuilder {
     private String name;
+    private String machineName;
     private Map selector = new HashMap<>();
     private List ports = Collections.emptyList();
     private Map serversConfigs = Collections.emptyMap();
@@ -241,7 +247,11 @@ public class ServerExposer {
       return builder
           .withNewMetadata()
           .withName(name.replace("/", "-"))
-          .withAnnotations(Annotations.newSerializer().servers(serversConfigs).annotations())
+          .withAnnotations(
+              Annotations.newSerializer()
+                  .servers(serversConfigs)
+                  .machineName(machineName)
+                  .annotations())
           .endMetadata()
           .withNewSpec()
           .withSelector(selector)
@@ -249,6 +259,11 @@ public class ServerExposer {
           .endSpec()
           .build();
     }
+
+    public ServiceBuilder withMachineName(String machineName) {
+      this.machineName = machineName;
+      return this;
+    }
   }
 
   private static class RouteBuilder {
@@ -256,6 +271,7 @@ public class ServerExposer {
     private String serviceName;
     private IntOrString targetPort;
     private Map serversConfigs;
+    private String machineName;
 
     private RouteBuilder withName(String name) {
       this.name = name;
@@ -277,6 +293,11 @@ public class ServerExposer {
       return this;
     }
 
+    public RouteBuilder withMachineName(String machineName) {
+      this.machineName = machineName;
+      return this;
+    }
+
     private Route build() {
       io.fabric8.openshift.api.model.RouteBuilder builder =
           new io.fabric8.openshift.api.model.RouteBuilder();
@@ -284,7 +305,11 @@ public class ServerExposer {
       return builder
           .withNewMetadata()
           .withName(name.replace("/", "-"))
-          .withAnnotations(Annotations.newSerializer().servers(serversConfigs).annotations())
+          .withAnnotations(
+              Annotations.newSerializer()
+                  .servers(serversConfigs)
+                  .machineName(machineName)
+                  .annotations())
           .endMetadata()
           .withNewSpec()
           .withNewTo()
diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolver.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolver.java
index 9178d69ab9..6ad6f6fbe1 100644
--- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolver.java
+++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolver.java
@@ -10,40 +10,47 @@
  */
 package org.eclipse.che.workspace.infrastructure.openshift;
 
-import io.fabric8.kubernetes.api.model.Container;
-import io.fabric8.kubernetes.api.model.ContainerPort;
-import io.fabric8.kubernetes.api.model.IntOrString;
-import io.fabric8.kubernetes.api.model.Pod;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import io.fabric8.kubernetes.api.model.Service;
-import io.fabric8.kubernetes.api.model.ServicePort;
 import io.fabric8.openshift.api.model.Route;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
 import org.eclipse.che.api.core.model.workspace.config.ServerConfig;
 import org.eclipse.che.api.core.model.workspace.runtime.ServerStatus;
 import org.eclipse.che.api.workspace.server.model.impl.ServerImpl;
 
 /**
- * Helps to resolve {@link ServerImpl servers} by container in pod according to specified {@link
- * Route routes} and {@link Service services}.
+ * Helps to resolve {@link ServerImpl servers} by machine name according to specified {@link Route
+ * routes} and {@link Service services}.
  *
- * 

How {@link Container}, {@link Pod}, {@link Service} and {@link Route} are linked described in - * {@link ServerExposer}. + *

Objects annotations are used to check if {@link Service service} or {@link Route route} + * exposes the specified machine servers. * * @author Sergii Leshchenko * @author Alexander Garagatyi * @see ServerExposer + * @see Annotations */ public class ServerResolver { - private final List services; - private final List routes; + private final Multimap services; + private final Multimap routes; private ServerResolver(List services, List routes) { - this.services = services; - this.routes = routes; + this.services = ArrayListMultimap.create(); + for (Service service : services) { + String machineName = + Annotations.newDeserializer(service.getMetadata().getAnnotations()).machineName(); + this.services.put(machineName, service); + } + + this.routes = ArrayListMultimap.create(); + for (Route route : routes) { + String machineName = + Annotations.newDeserializer(route.getMetadata().getAnnotations()).machineName(); + this.routes.put(machineName, route); + } } public static ServerResolver of(List services, List routes) { @@ -51,27 +58,15 @@ public class ServerResolver { } /** - * Resolves servers by the specified container in the pod. + * Resolves servers by the specified machine name. * - * @param pod pod that should be matched by services - * @param container container that expose ports for services + * @param machineName machine to resolve servers * @return resolved servers */ - public Map resolve(Pod pod, Container container) { - Set matchedServices = - getMatchedServices(pod, container) - .stream() - .map(s -> s.getMetadata().getName()) - .collect(Collectors.toSet()); + public Map resolve(String machineName) { Map servers = new HashMap<>(); - services - .stream() - .filter(service -> matchedServices.contains(service.getMetadata().getName())) - .forEach(service -> fillServiceServers(service, servers)); - routes - .stream() - .filter(route -> matchedServices.contains(route.getSpec().getTo().getName())) - .forEach(route -> fillRouteServers(route, servers)); + services.get(machineName).forEach(service -> fillServiceServers(service, servers)); + routes.get(machineName).forEach(route -> fillRouteServers(route, servers)); return servers; } @@ -134,46 +129,4 @@ public class ServerResolver { private String removeSuffix(String port) { return port.split("/")[0]; } - - private List getMatchedServices(Pod pod, Container container) { - return services - .stream() - .filter(service -> isExposedByService(pod, service)) - .filter(service -> isExposedByService(container, service)) - .collect(Collectors.toList()); - } - - private boolean isExposedByService(Pod pod, Service service) { - Map labels = pod.getMetadata().getLabels(); - Map selectorLabels = service.getSpec().getSelector(); - if (labels == null) { - return false; - } - for (Map.Entry selectorLabelEntry : selectorLabels.entrySet()) { - if (!selectorLabelEntry.getValue().equals(labels.get(selectorLabelEntry.getKey()))) { - return false; - } - } - return true; - } - - private boolean isExposedByService(Container container, Service service) { - for (ServicePort servicePort : service.getSpec().getPorts()) { - IntOrString targetPort = servicePort.getTargetPort(); - if (targetPort.getIntVal() != null) { - for (ContainerPort containerPort : container.getPorts()) { - if (targetPort.getIntVal().equals(containerPort.getContainerPort())) { - return true; - } - } - } else { - for (ContainerPort containerPort : container.getPorts()) { - if (targetPort.getStrVal().equals(containerPort.getName())) { - return true; - } - } - } - } - return false; - } } diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/AnnotationsTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/AnnotationsTest.java index cf80a76eb6..7559a32b7f 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/AnnotationsTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/AnnotationsTest.java @@ -43,6 +43,7 @@ public class AnnotationsTest { .servers( ImmutableMap.of( "my-server2", new ServerConfigImpl("8080/tcp", "ws", "/connect", emptyMap()))) + .machineName("test-machine") .annotations(); Map expected = ImmutableMap.builder() @@ -54,6 +55,7 @@ public class AnnotationsTest { .put("org.eclipse.che.server.my-server2.protocol", "ws") .put("org.eclipse.che.server.my-server2.path", "/connect") .put("org.eclipse.che.server.my-server2.attributes", stringEmptyAttributes) + .put("org.eclipse.che.machine.name", "test-machine") .build(); assertEquals(serialized, expected); @@ -74,12 +76,15 @@ public class AnnotationsTest { .put("org.eclipse.che.server.my-server3.port", "7070/tcp") .put("org.eclipse.che.server.my-server3.protocol", "http") .put("org.eclipse.che.server.my-server3.attributes", stringEmptyAttributes) + .put("org.eclipse.che.machine.name", "test-machine") .build(); Annotations.Deserializer deserializer = Annotations.newDeserializer(annotations); Map servers = deserializer.servers(); + assertEquals(deserializer.machineName(), "test-machine"); + Map expected = new HashMap<>(); expected.put("my-server1/http", new ServerConfigImpl("8000/tcp", "http", "/api/info", null)); expected.put("my-server2", new ServerConfigImpl("8080/tcp", "ws", "/connect", ATTRIBUTES)); diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerExposerTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerExposerTest.java index 92e604bd0f..88289f9f58 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerExposerTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerExposerTest.java @@ -45,12 +45,14 @@ import org.testng.annotations.Test; * @author Sergii Leshchenko */ public class ServerExposerTest { + private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final Map INTERNAL_SERVER_ATTRIBUTE_MAP = singletonMap(ServerConfig.INTERNAL_SERVER_ATTRIBUTE, Boolean.TRUE.toString()); private static final Pattern SERVER_PREFIX_REGEX = Pattern.compile('^' + SERVER_PREFIX + "[A-z0-9]{" + SERVER_UNIQUE_PART_SIZE + "}-pod-main$"); + public static final String MACHINE_NAME = "pod/main"; private ServerExposer serverExposer; private OpenShiftEnvironment openShiftEnvironment; @@ -71,7 +73,7 @@ public class ServerExposerTest { openShiftEnvironment = OpenShiftEnvironment.builder().setPods(ImmutableMap.of("pod", pod)).build(); - this.serverExposer = new ServerExposer("pod/main", pod, container, openShiftEnvironment); + this.serverExposer = new ServerExposer(MACHINE_NAME, pod, container, openShiftEnvironment); } @Test @@ -87,6 +89,7 @@ public class ServerExposerTest { // then assertThatExternalServerIsExposed( + MACHINE_NAME, "http-server", "tcp", 8080, @@ -113,11 +116,13 @@ public class ServerExposerTest { assertEquals(openShiftEnvironment.getServices().size(), 1); assertEquals(openShiftEnvironment.getRoutes().size(), 1); assertThatExternalServerIsExposed( + MACHINE_NAME, "http-server", "tcp", 8080, new ServerConfigImpl(httpServerConfig).withAttributes(ATTRIBUTES_MAP)); assertThatExternalServerIsExposed( + MACHINE_NAME, "ws-server", "tcp", 8080, @@ -144,11 +149,13 @@ public class ServerExposerTest { assertEquals(openShiftEnvironment.getServices().size(), 1); assertEquals(openShiftEnvironment.getRoutes().size(), 2); assertThatExternalServerIsExposed( + MACHINE_NAME, "http-server", "tcp", 8080, new ServerConfigImpl(httpServerConfig).withAttributes(ATTRIBUTES_MAP)); assertThatExternalServerIsExposed( + MACHINE_NAME, "ws-server", "tcp", 8081, @@ -171,6 +178,7 @@ public class ServerExposerTest { assertEquals(openShiftEnvironment.getServices().size(), 1); assertEquals(openShiftEnvironment.getRoutes().size(), 1); assertThatExternalServerIsExposed( + MACHINE_NAME, "http-server", "TCP", 8080, @@ -197,6 +205,7 @@ public class ServerExposerTest { // then assertThatExternalServerIsExposed( + MACHINE_NAME, "http-server", "tcp", 8080, @@ -226,6 +235,7 @@ public class ServerExposerTest { assertEquals(container.getPorts().get(1).getContainerPort(), new Integer(8080)); assertEquals(container.getPorts().get(1).getProtocol(), "UDP"); assertThatExternalServerIsExposed( + MACHINE_NAME, "server", "udp", 8080, @@ -245,6 +255,7 @@ public class ServerExposerTest { // then assertThatInternalServerIsExposed( + MACHINE_NAME, "http-server", "tcp", 8080, @@ -266,11 +277,13 @@ public class ServerExposerTest { // then assertThatInternalServerIsExposed( + MACHINE_NAME, "int-server", "tcp", 8080, new ServerConfigImpl(internalServerConfig).withAttributes(INTERNAL_SERVER_ATTRIBUTE_MAP)); assertThatExternalServerIsExposed( + MACHINE_NAME, "ext-server", "tcp", 9090, @@ -278,7 +291,11 @@ public class ServerExposerTest { } private void assertThatExternalServerIsExposed( - String serverNameRegex, String portProtocol, Integer port, ServerConfigImpl expected) { + String machineName, + String serverNameRegex, + String portProtocol, + Integer port, + ServerConfigImpl expected) { // then assertTrue( container @@ -313,21 +330,31 @@ public class ServerExposerTest { assertEquals(servicePort.getPort(), port); assertEquals(servicePort.getName(), SERVER_PREFIX + "-" + port); + Annotations.Deserializer serviceAnnotations = + Annotations.newDeserializer(service.getMetadata().getAnnotations()); + assertEquals(serviceAnnotations.machineName(), machineName); + // ensure that required route is created Route route = openShiftEnvironment.getRoutes().get(service.getMetadata().getName() + "-server-" + port); assertEquals(route.getSpec().getTo().getName(), service.getMetadata().getName()); assertEquals(route.getSpec().getPort().getTargetPort().getStrVal(), servicePort.getName()); - Annotations.Deserializer annotationsDeserializer = + Annotations.Deserializer routeAnnotations = Annotations.newDeserializer(route.getMetadata().getAnnotations()); - Map servers = annotationsDeserializer.servers(); + Map servers = routeAnnotations.servers(); ServerConfig serverConfig = servers.get(serverNameRegex); assertEquals(serverConfig, expected); + + assertEquals(routeAnnotations.machineName(), machineName); } private void assertThatInternalServerIsExposed( - String serverNameRegex, String portProtocol, Integer port, ServerConfigImpl expected) { + String machineName, + String serverNameRegex, + String portProtocol, + Integer port, + ServerConfigImpl expected) { // then assertTrue( container @@ -362,9 +389,11 @@ public class ServerExposerTest { assertEquals(servicePort.getPort(), port); assertEquals(servicePort.getName(), SERVER_PREFIX + "-" + port); - Annotations.Deserializer annotationsDeserializer = + Annotations.Deserializer serviceAnnotations = Annotations.newDeserializer(service.getMetadata().getAnnotations()); - Map servers = annotationsDeserializer.servers(); + assertEquals(serviceAnnotations.machineName(), machineName); + + Map servers = serviceAnnotations.servers(); ServerConfig serverConfig = servers.get(serverNameRegex); assertEquals(serverConfig, expected); @@ -374,10 +403,12 @@ public class ServerExposerTest { assertEquals(route.getSpec().getTo().getName(), service.getMetadata().getName()); assertEquals(route.getSpec().getPort().getTargetPort().getStrVal(), servicePort.getName()); - Annotations.Deserializer routeAnnotationsDeserializer = + Annotations.Deserializer routeAnnotations = Annotations.newDeserializer(route.getMetadata().getAnnotations()); - Map routeServers = routeAnnotationsDeserializer.servers(); + Map routeServers = routeAnnotations.servers(); ServerConfig routeServerConfig = routeServers.get(serverNameRegex); assertNull(routeServerConfig); + + assertEquals(routeAnnotations.machineName(), machineName); } } diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolverTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolverTest.java index 0a5f7fadbb..ed0b3dce5d 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolverTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/ServerResolverTest.java @@ -10,7 +10,7 @@ */ package org.eclipse.che.workspace.infrastructure.openshift; -import static java.util.Collections.emptyMap; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.eclipse.che.api.core.model.workspace.runtime.ServerStatus.UNKNOWN; @@ -18,11 +18,6 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; import com.google.common.collect.ImmutableMap; -import io.fabric8.kubernetes.api.model.Container; -import io.fabric8.kubernetes.api.model.ContainerBuilder; -import io.fabric8.kubernetes.api.model.ContainerPortBuilder; -import io.fabric8.kubernetes.api.model.Pod; -import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; import io.fabric8.kubernetes.api.model.ServicePortBuilder; @@ -31,6 +26,7 @@ import io.fabric8.openshift.api.model.RouteBuilder; import java.util.Map; import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl; import org.eclipse.che.api.workspace.server.model.impl.ServerImpl; +import org.eclipse.che.workspace.infrastructure.openshift.Annotations.Serializer; import org.testng.annotations.Test; /** @@ -39,20 +35,21 @@ import org.testng.annotations.Test; * @author Sergii Leshchenko */ public class ServerResolverTest { + private static final Map ATTRIBUTES_MAP = singletonMap("key", "value"); private static final int CONTAINER_PORT = 3054; private static final String ROUTE_HOST = "localhost"; @Test - public void testResolvingServersWhenThereIsNoMatchedServiceByPodLabels() { + public void + testResolvingServersWhenThereIsNoTheCorrespondingServiceAndRouteForTheSpecifiedMachine() { // given - Container container = createContainer(); - Pod pod = createPod(ImmutableMap.of("kind", "web-app")); Service nonMatchedByPodService = - createService("nonMatched", CONTAINER_PORT, ImmutableMap.of("kind", "db"), null); + createService("nonMatched", "foreignMachine", CONTAINER_PORT, null); Route route = createRoute( "nonMatched", + "foreignMachine", ImmutableMap.of( "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); @@ -60,83 +57,46 @@ public class ServerResolverTest { ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); // when - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); // then assertTrue(resolved.isEmpty()); } @Test - public void testResolvingServersWhenThereIsNoMatchedServiceByContainerPort() { - Container container = createContainer(); - Pod pod = createPod(ImmutableMap.of("kind", "web-app")); - Service nonMatchedByPodService = - createService("nonMatched", 7777, ImmutableMap.of("kind", "web-app"), null); - Route route = - createRoute( - "nonMatched", - ImmutableMap.of( - "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); - - ServerResolver serverResolver = - ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); - - Map resolved = serverResolver.resolve(pod, container); - - assertTrue(resolved.isEmpty()); - } - - @Test - public void testResolvingServersWhenThereIsMatchedServiceForContainer() { - Container container = createContainer(); - Pod pod = createPod(ImmutableMap.of("kind", "web-app")); - Service nonMatchedByPodService = - createService("matched", CONTAINER_PORT, ImmutableMap.of("kind", "web-app"), null); + public void testResolvingServersWhenThereIsMatchedRouteForTheSpecifiedMachine() { Route route = createRoute( "matched", + "machine", ImmutableMap.of( - "http-server", - new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP), - "ws-server", - new ServerConfigImpl("3054", "ws", "/connect", ATTRIBUTES_MAP))); + "http-server", new ServerConfigImpl("3054", "http", "/api", ATTRIBUTES_MAP))); - ServerResolver serverResolver = - ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); + ServerResolver serverResolver = ServerResolver.of(emptyList(), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); - assertEquals(resolved.size(), 2); + assertEquals(resolved.size(), 1); assertEquals( resolved.get("http-server"), new ServerImpl() .withUrl("http://localhost/api") .withStatus(UNKNOWN) .withAttributes(ATTRIBUTES_MAP)); - assertEquals( - resolved.get("ws-server"), - new ServerImpl() - .withUrl("ws://localhost/connect") - .withStatus(UNKNOWN) - .withAttributes(ATTRIBUTES_MAP)); } @Test - public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsNull() { - Container container = createContainer(); - Pod pod = createPod(singletonMap("kind", "web-app")); - Service nonMatchedByPodService = - createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"), null); + public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIsNull() { Route route = createRoute( "matched", + "machine", singletonMap( "http-server", new ServerConfigImpl("3054", "http", null, ATTRIBUTES_MAP))); - ServerResolver serverResolver = - ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); + ServerResolver serverResolver = ServerResolver.of(emptyList(), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); assertEquals(resolved.size(), 1); assertEquals( @@ -148,20 +108,16 @@ public class ServerResolverTest { } @Test - public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsEmpty() { - Container container = createContainer(); - Pod pod = createPod(singletonMap("kind", "web-app")); - Service nonMatchedByPodService = - createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"), null); + public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIsEmpty() { Route route = createRoute( "matched", + "machine", singletonMap("http-server", new ServerConfigImpl("3054", "http", "", ATTRIBUTES_MAP))); - ServerResolver serverResolver = - ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); + ServerResolver serverResolver = ServerResolver.of(emptyList(), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); assertEquals(resolved.size(), 1); assertEquals( @@ -173,21 +129,17 @@ public class ServerResolverTest { } @Test - public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsRelative() { - Container container = createContainer(); - Pod pod = createPod(singletonMap("kind", "web-app")); - Service nonMatchedByPodService = - createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"), null); + public void testResolvingServersWhenThereIsMatchedRouteForMachineAndServerPathIsRelative() { Route route = createRoute( "matched", + "machine", singletonMap( "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); - ServerResolver serverResolver = - ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route)); + ServerResolver serverResolver = ServerResolver.of(emptyList(), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); assertEquals(resolved.size(), 1); assertEquals( @@ -200,20 +152,18 @@ public class ServerResolverTest { @Test public void testResolvingInternalServers() { - Container container = createContainer(); - Pod pod = createPod(singletonMap("kind", "web-app")); Service service = createService( "service11", + "machine", CONTAINER_PORT, - singletonMap("kind", "web-app"), singletonMap( "http-server", new ServerConfigImpl("3054", "http", "api", ATTRIBUTES_MAP))); - Route route = createRoute("matched", null); + Route route = createRoute("matched", "machine", null); ServerResolver serverResolver = ServerResolver.of(singletonList(service), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); assertEquals(resolved.size(), 1); assertEquals( @@ -226,20 +176,18 @@ public class ServerResolverTest { @Test public void testResolvingInternalServersWithPortWithTransportProtocol() { - Container container = createContainer(); - Pod pod = createPod(singletonMap("kind", "web-app")); Service service = createService( "service11", + "machine", CONTAINER_PORT, - singletonMap("kind", "web-app"), singletonMap( "http-server", new ServerConfigImpl("3054/udp", "xxx", "api", ATTRIBUTES_MAP))); - Route route = createRoute("matched", null); + Route route = createRoute("matched", "machine", null); ServerResolver serverResolver = ServerResolver.of(singletonList(service), singletonList(route)); - Map resolved = serverResolver.resolve(pod, container); + Map resolved = serverResolver.resolve("machine"); assertEquals(resolved.size(), 1); assertEquals( @@ -250,32 +198,20 @@ public class ServerResolverTest { .withAttributes(ATTRIBUTES_MAP)); } - private Pod createPod(Map labels) { - return new PodBuilder().withNewMetadata().withLabels(labels).endMetadata().build(); - } - - private Container createContainer() { - return new ContainerBuilder() - .withPorts(new ContainerPortBuilder().withContainerPort(CONTAINER_PORT).build()) - .build(); - } - private Service createService( - String name, - Integer port, - Map selector, - Map servers) { - Map annotations = emptyMap(); + String name, String machineName, Integer port, Map servers) { + Serializer serializer = Annotations.newSerializer(); + serializer.machineName(machineName); if (servers != null) { - annotations = Annotations.newSerializer().servers(servers).annotations(); + serializer.servers(servers); } + return new ServiceBuilder() .withNewMetadata() .withName(name) - .withAnnotations(annotations) + .withAnnotations(serializer.annotations()) .endMetadata() .withNewSpec() - .withSelector(selector) .withPorts( new ServicePortBuilder() .withPort(port) @@ -287,16 +223,17 @@ public class ServerResolverTest { .build(); } - // TODO Think about common builders - private Route createRoute(String name, Map servers) { - Map annotations = emptyMap(); + private Route createRoute( + String name, String machineName, Map servers) { + Serializer serializer = Annotations.newSerializer(); + serializer.machineName(machineName); if (servers != null) { - annotations = Annotations.newSerializer().servers(servers).annotations(); + serializer.servers(servers); } return new RouteBuilder() .withNewMetadata() .withName(name) - .withAnnotations(annotations) + .withAnnotations(serializer.annotations()) .endMetadata() .withNewSpec() .withHost(ROUTE_HOST)