merge master

4 files changed, 55 insertions, 88 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
index 71abdb44fa9..8856c1770d1 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/CreateContainerCommandImpl.java
@@ -139,7 +139,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
         List<String> labelList = labels.entrySet().stream()
                 .map(entry -> entry.getKey() + "=" + entry.getValue()).collect(Collectors.toList());
 
-        return "--name " + containerName + " "
+        return "--name " + containerName.asString() + " "
                 + "--hostname " + hostName + " "
                 + toRepeatedOption("--label", labelList)
                 + toRepeatedOption("--env", environmentAssignments)
@@ -148,7 +148,7 @@ class CreateContainerCommandImpl implements Docker.CreateContainerCommand {
                 + toOptionalOption("--net", networkMode)
                 + toOptionalOption("--ip", ipv4Address)
                 + toOptionalOption("--ip6", ipv6Address)
-                + dockerImage;
+                + dockerImage.asString();
     }
 
     private String generateRandomMACAddress() {
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index e43af970a33..4dbe8a620a1 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -82,9 +82,6 @@ public class DockerImpl implements Docker {
     private CounterWrapper numberOfDockerDaemonFails;
     private final boolean hackAroundPullImageDueToJerseyConflicts;
 
-    private JerseyDockerCmdExecFactory dockerFactory;
-    private RemoteApiVersion remoteApiVersion;
-
     // For testing
     DockerImpl(final DockerClient dockerClient) {
         hackAroundPullImageDueToJerseyConflicts = false;
@@ -566,12 +563,13 @@ public class DockerImpl implements Docker {
         }
     }
 
-    private void initDockerConnection(final DockerConfig config, int connectTimeousMs, boolean fallbackTo123orErrors) {
-        dockerFactory = new JerseyDockerCmdExecFactory()
+    private void initDockerConnection(final DockerConfig config, int connectTimeoutMs, boolean fallbackTo123orErrors) {
+        JerseyDockerCmdExecFactory dockerFactory = new JerseyDockerCmdExecFactory()
                 .withMaxPerRouteConnections(DOCKER_MAX_PER_ROUTE_CONNECTIONS)
                 .withMaxTotalConnections(DOCKER_MAX_TOTAL_CONNECTIONS)
-                .withConnectTimeout(connectTimeousMs)
+                .withConnectTimeout(connectTimeoutMs)
                 .withReadTimeout(DOCKER_READ_TIMEOUT_MILLIS);
+        RemoteApiVersion remoteApiVersion;
         try {
             remoteApiVersion = RemoteApiVersion.parseConfig(DockerClientImpl.getInstance(
                     buildDockerClientConfig(config).build())
@@ -605,7 +603,7 @@ public class DockerImpl implements Docker {
         numberOfRunningContainersGauge = metricReceiver.declareGauge(dimensions, "containers.running");
         numberOfDockerDaemonFails = metricReceiver.declareCounter(dimensions, "daemon.api_fails");
 
-        // Some containers could already be running, count them and intialize to that value
+        // Some containers could already be running, count them and initialize to that value
         numberOfRunningContainersGauge.sample(getAllManagedContainers().size());
     }
 }
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
index a8f37236116..e41074d9354 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java
@@ -15,10 +15,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.Inet6Address;
 import java.net.InetAddress;
-import java.net.NetworkInterface;
-import java.net.SocketException;
 import java.net.URL;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
@@ -39,17 +36,7 @@ import static org.junit.Assume.assumeTrue;
  *   2. For network test, we need to make docker containers visible for Mac: sudo route add 172.0.0.0/8 192.168.99.100
  *   2. Run tests from IDE/mvn.
  *
- * LINUX:
- *  1. Remove Ignore annotations
- *  2. Change ownership of docker.sock
- *      $ sudo chown <your username> /var/run/docker.sock
- *  3. (Temporary) Manually create the docker network used by DockerImpl by running:
- *      $ sudo docker network create --ipv6 --gateway=<your local IPv6 address> --subnet=fe80::1/16 habla
- *  4. (Temporary) Manually build docker test image. Inside src/test/resources/simple-ipv6-server run:
- *      $ sudo docker build -t "simple-ipv6-server:Dockerfile" .
- *  5. (Temporary) Comment out createDockerImage() and shutdown()
- *
- *
+
  * TIPS:
  *   For cleaning up your local docker machine (DON'T DO THIS ON PROD)
  *     docker stop $(docker ps -a -q)
@@ -63,18 +50,18 @@ public class DockerTest {
     public static final String IP_ADDRESS_FOR_CONTAINER = "172.18.7.32";
 
     private DockerImpl docker;
-    private static final boolean isMacOSX = System.getProperty("os.name").equals("Mac OS X");
+    private static final OS operatingSystem = getSystemOS();
     private static final String prefix = "/Users/" + System.getProperty("user.name") + "/.docker/machine/machines/default/";
     private static final DockerConfig dockerConfig = new DockerConfig(new DockerConfig.Builder()
-            .caCertPath(isMacOSX ? prefix + "ca.pem" : "")
-            .clientCertPath(isMacOSX ? prefix + "cert.pem" : "")
-            .clientKeyPath(isMacOSX ? prefix + "key.pem" : "")
-            .uri(isMacOSX ? "tcp://192.168.99.100:2376" : "unix:///var/run/docker.sock"));
+            .caCertPath(operatingSystem == OS.Mac_OS_X ? prefix + "ca.pem" : "")
+            .clientCertPath(operatingSystem == OS.Mac_OS_X ? prefix + "cert.pem" : "")
+            .clientKeyPath(operatingSystem == OS.Mac_OS_X ? prefix + "key.pem" : "")
+            .uri(operatingSystem == OS.Mac_OS_X ? "tcp://192.168.99.100:2376" : "tcp://localhost:2376"));
     private static final DockerImage dockerImage = new DockerImage("simple-ipv6-server:Dockerfile");
 
     @Test
     public void testGetAllManagedContainersNoContainersRunning() {
-        assumeTrue(isMacOSX);
+        assumeTrue(operatingSystem != OS.Unsupported);
         assumeTrue(dockerDaemonIsPresent());
 
         List<Container> containers = docker.getAllManagedContainers();
@@ -174,26 +161,33 @@ public class DockerTest {
         assertThat(success, is(true));
     }
 
-    // TODO: Does not work on Mac OSx
-    @Ignore
+
     @Test
-    public void testDockerIPv6Networking() throws InterruptedException, ExecutionException, IOException {
+    public void testDockerNetworking() throws InterruptedException, ExecutionException, IOException {
+        assumeTrue(dockerDaemonIsPresent());
+        createDockerImage(docker);
+
         String hostName1 = "docker10.test.yahoo.com";
         String hostName2 = "docker11.test.yahoo.com";
         ContainerName containerName1 = new ContainerName("test-container-1");
         ContainerName containerName2 = new ContainerName("test-container-2");
-        InetAddress inetAddress1 = Inet6Address.getByName("fe80::10");
-        InetAddress inetAddress2 = Inet6Address.getByName("fe80::11");
-        DockerImpl docker = new DockerImpl(dockerConfig, new MetricReceiverWrapper(MetricReceiver.nullImplementation));
+        InetAddress inetAddress1 = InetAddress.getByName("172.18.0.10");
+        InetAddress inetAddress2 = InetAddress.getByName("172.18.0.11");
 
-         docker.createContainerCommand(dockerImage, containerName1, hostName1).withIpAddress(inetAddress1).create();
-         docker.createContainerCommand(dockerImage, containerName2, hostName2).withIpAddress(inetAddress2).create();
+        docker.createContainerCommand(dockerImage, containerName1, hostName1)
+                .withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME).withIpAddress(inetAddress1).create();
+
+        docker.createContainerCommand(dockerImage, containerName2, hostName2)
+                .withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME).withIpAddress(inetAddress2).create();
+
+        docker.startContainer(containerName1);
+        docker.startContainer(containerName2);
 
         try {
             testReachabilityFromHost(containerName1, inetAddress1);
             testReachabilityFromHost(containerName2, inetAddress2);
 
-            String[] curlFromNodeToNode = new String[]{"curl", "-g", "http://[" + inetAddress2 + "%eth0]/ping"};
+            String[] curlFromNodeToNode = new String[]{"curl", "-g", "http://" + inetAddress2 + "/ping"};
             while (! docker.executeInContainer(containerName1, curlFromNodeToNode).isSuccess()) {
                 Thread.sleep(20);
             }
@@ -210,15 +204,18 @@ public class DockerTest {
     }
 
     private boolean dockerDaemonIsPresent() {
-        if (!isMacOSX) {
+        if (docker != null) return true;
+        if (operatingSystem == OS.Unsupported) {
             System.out.println("This test does not support " + System.getProperty("os.name") + " yet, ignoring test.");
             return false;
         }
+
         try {
             setDocker();
             return true;
         } catch (Exception e) {
             System.out.println("Please install Docker Toolbox and start Docker Quick Start Terminal once, ignoring test.");
+            e.printStackTrace();
             return false;
         }
     }
@@ -228,33 +225,20 @@ public class DockerTest {
                 dockerConfig,
                 false, /* fallback to 1.23 on errors */
                 false, /* try setup netowork */
-                isMacOSX ? Optional.of(SUBNET_CONTAINER) : Optional.empty(), /* subnetwork */
+                operatingSystem == OS.Mac_OS_X ? Optional.of(SUBNET_CONTAINER) : Optional.empty(), /* subnetwork */
                 false, /* hackAroundPullImageDueToJerseyConflicts */
                 100 /* dockerConnectTimeoutMillis */,
                 new MetricReceiverWrapper(MetricReceiver.nullImplementation));
     }
 
     private void testReachabilityFromHost(ContainerName containerName, InetAddress target) throws IOException, InterruptedException {
-        String[] curlNodeFromHost = {"curl", "-g", "http://[" + target.getHostAddress() + "%" + getInterfaceName() + "]/ping"};
+        String[] curlNodeFromHost = {"curl", "-g", "http://" + target.getHostAddress() + "/ping"};
         while (!exec(curlNodeFromHost).equals("pong\n")) {
             Thread.sleep(20);
         }
         assertTrue("Could not reach " + containerName.asString() + " from host", exec(curlNodeFromHost).equals("pong\n"));
     }
 
-
-    /**
-     * Returns the display name of the bridge used by our custom docker network. This is used for routing in the
-     * network tests. The bridge is assumed to be the only IPv6 interface starting with "br-"
-     */
-    private static String getInterfaceName() throws SocketException {
-        return Collections.list(NetworkInterface.getNetworkInterfaces()).stream()
-                .filter(networkInterface -> networkInterface.getDisplayName().startsWith("br-") &&
-                        networkInterface.getInterfaceAddresses().stream()
-                                .anyMatch(ip -> ip.getAddress() instanceof Inet6Address))
-                .findFirst().orElseThrow(RuntimeException::new).getDisplayName();
-    }
-
     /**
      * Synchronously executes a system process and returns its stdout. Based of {@link com.yahoo.system.ProcessExecuter}
      * but could not be reused because of import errors.
@@ -277,38 +261,11 @@ public class DockerTest {
         return ret.toString();
     }
 
-    /**
-     * Returns IPv6 address of on the "docker0" interface that can be reached by the containers
-     */
-    private static String getLocalIPv6Address() throws SocketException {
-        return Collections.list(NetworkInterface.getNetworkInterfaces()).stream()
-                .filter(networkInterface -> networkInterface.getDisplayName().equals("docker0"))
-                .flatMap(i -> Collections.list(i.getInetAddresses()).stream())
-                .filter(ip -> ip instanceof Inet6Address && ip.isLinkLocalAddress())
-                .findFirst().orElseThrow(RuntimeException::new)
-                .getHostAddress().split("%")[0];
-    }
-
-    private void createSomeNetork() throws SocketException {
-        Network.Ipam ipam = new Network.Ipam().withConfig(new Network.Ipam.Config()
-                .withSubnet("fe80::1/16").withGateway(getLocalIPv6Address()));
-        // TODO: This needs to match the network name in DockerOperations!?
-        docker.dockerClient.createNetworkCmd().withDriver("bridge").withName("habla")
-                .withIpam(ipam).exec();
-    }
-
-    private void remoteNetwork() {
-        // Remove the network we created earlier
-        // TODO: This needs to match the network name in DockerOperations!?
-        docker.dockerClient.removeNetworkCmd("habla").exec();
-    }
-
-    // TODO: Do we need this? Rather use network created by DockerImpl, right?
-    void createDockerImage(DockerImpl docker) throws IOException, ExecutionException, InterruptedException {
+    private void createDockerImage(DockerImpl docker) throws IOException, ExecutionException, InterruptedException {
         try {
             docker.deleteImage(new DockerImage(dockerImage.asString()));
         } catch (Exception e) {
-            assertThat(e.getMessage(), is("Failed to delete docker image simple-ipv6-server:Dockerfile"));
+            assertThat(e.getMessage(), is("Failed to delete docker image " + dockerImage.asString()));
         }
 
         // Build the image locally
@@ -317,4 +274,14 @@ public class DockerTest {
                 .buildImageCmd(dockerFilePath)
                 .withTag(dockerImage.asString()).exec(new BuildImageResultCallback()).awaitCompletion();
     }
+
+    private enum OS {Linux, Mac_OS_X, Unsupported}
+
+    private static OS getSystemOS() {
+        switch (System.getProperty("os.name").toLowerCase()) {
+            case "linux": return OS.Linux;
+            case "mac os x": return OS.Mac_OS_X;
+            default: return OS.Unsupported;
+        }
+    }
 }
diff --git a/docker-api/src/test/resources/simple-ipv6-server/src/server.py b/docker-api/src/test/resources/simple-ipv6-server/src/server.py
index 320f311e0fc..c28c288bbca 100644
--- a/docker-api/src/test/resources/simple-ipv6-server/src/server.py
+++ b/docker-api/src/test/resources/simple-ipv6-server/src/server.py
@@ -28,14 +28,16 @@ class MyHandler(SimpleHTTPRequestHandler):
             return
 
 
-class HTTPServerV6(HTTPServer):
-    address_family = socket.AF_INET6
+class DualHTTPServer(HTTPServer):
+    def __init__(self, address, handler):
+        self.address_family = socket.AF_INET6 if (':' in address[0]) else socket.AF_INET
+        HTTPServer.__init__(self, address, handler)
 
 
-def main():
-    server = HTTPServerV6(('::', 80), MyHandler)
+def main(ipv6):
+    server = DualHTTPServer(('::' if ipv6 else '', 80), MyHandler)
     server.serve_forever()
 
 
 if __name__ == '__main__':
-    main()
+    main(False)