Use DockerNetworking

9 files changed, 69 insertions, 47 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
index f0c0a9c31d3..dc0ac0df05d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/component/Environment.java
@@ -15,16 +15,14 @@ import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesImpl;
 import java.net.URI;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
 import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.Date;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.TimeZone;
 
 /**
  * Various utilities for getting values from node-admin's environment. Immutable.
@@ -33,7 +31,8 @@ import java.util.TimeZone;
  * @author hmusum
  */
 public class Environment {
-    private static final DateFormat filenameFormatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS");
+    private static final DateTimeFormatter filenameFormatter = DateTimeFormatter
+            .ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSS").withZone(ZoneOffset.UTC);
     public static final String APPLICATION_STORAGE_CLEANUP_PATH_PREFIX = "cleanup_";
 
     private static final String ENVIRONMENT = "ENVIRONMENT";
@@ -52,13 +51,13 @@ public class Environment {
     private final String environment;
     private final String region;
     private final String system;
+    private final String cloud;
     private final String parentHostHostname;
     private final IPAddresses ipAddresses;
     private final PathResolver pathResolver;
     private final List<String> logstashNodes;
     private final Optional<String> coredumpFeedEndpoint;
     private final NodeType nodeType;
-    private final String cloud;
     private final ContainerEnvironmentResolver containerEnvironmentResolver;
     private final String certificateDnsSuffix;
     private final URI ztsUri;
@@ -66,10 +65,7 @@ public class Environment {
     private final boolean nodeAgentCertEnabled;
     private final boolean isRunningOnHost;
     private final Path trustStorePath;
-
-    static {
-        filenameFormatter.setTimeZone(TimeZone.getTimeZone("UTC"));
-    }
+    private final DockerNetworking dockerNetworking;
 
     public Environment(ConfigServerConfig configServerConfig) {
         this(configServerConfig,
@@ -77,19 +73,20 @@ public class Environment {
              getEnvironmentVariable(ENVIRONMENT),
              getEnvironmentVariable(REGION),
              getEnvironmentVariable(SYSTEM),
+             getEnvironmentVariable(CLOUD),
              Defaults.getDefaults().vespaHostname(),
              new IPAddressesImpl(),
              new PathResolver(),
              getLogstashNodesFromEnvironment(),
              Optional.of(getEnvironmentVariable(COREDUMP_FEED_ENDPOINT)),
              NodeType.host,
-             getEnvironmentVariable(CLOUD),
              new DefaultContainerEnvironmentResolver(),
              getEnvironmentVariable(CERTIFICATE_DNS_SUFFIX),
              URI.create(getEnvironmentVariable(ZTS_URI)),
              (AthenzService)AthenzIdentities.from(getEnvironmentVariable(NODE_ATHENZ_IDENTITY)),
              Boolean.valueOf(getEnvironmentVariable(ENABLE_NODE_AGENT_CERT)),
-             false);
+             false,
+             DockerNetworking.MACVLAN);
     }
 
     private Environment(ConfigServerConfig configServerConfig,
@@ -97,36 +94,33 @@ public class Environment {
                         String environment,
                         String region,
                         String system,
+                        String cloud,
                         String parentHostHostname,
                         IPAddresses ipAddresses,
                         PathResolver pathResolver,
                         List<String> logstashNodes,
                         Optional<String> coreDumpFeedEndpoint,
                         NodeType nodeType,
-                        String cloud,
                         ContainerEnvironmentResolver containerEnvironmentResolver,
                         String certificateDnsSuffix,
                         URI ztsUri,
                         AthenzService nodeAthenzIdentity,
                         boolean nodeAgentCertEnabled,
-                        boolean isRunningOnHost) {
+                        boolean isRunningOnHost,
+                        DockerNetworking dockerNetworking) {
         Objects.requireNonNull(configServerConfig, "configServerConfig cannot be null");
-        Objects.requireNonNull(environment, "environment cannot be null");
-        Objects.requireNonNull(region, "region cannot be null");
-        Objects.requireNonNull(system, "system cannot be null");
-        Objects.requireNonNull(cloud, "cloud cannot be null");
 
         this.configServerInfo = new ConfigServerInfo(configServerConfig);
-        this.environment = environment;
-        this.region = region;
-        this.system = system;
+        this.environment = Objects.requireNonNull(environment, "environment cannot be null");;
+        this.region = Objects.requireNonNull(region, "region cannot be null");;
+        this.system = Objects.requireNonNull(system, "system cannot be null");;
+        this.cloud = Objects.requireNonNull(cloud, "cloud cannot be null");
         this.parentHostHostname = parentHostHostname;
         this.ipAddresses = ipAddresses;
         this.pathResolver = pathResolver;
         this.logstashNodes = logstashNodes;
         this.coredumpFeedEndpoint = coreDumpFeedEndpoint;
         this.nodeType = nodeType;
-        this.cloud = cloud;
         this.containerEnvironmentResolver = containerEnvironmentResolver;
         this.certificateDnsSuffix = certificateDnsSuffix;
         this.ztsUri = ztsUri;
@@ -134,6 +128,7 @@ public class Environment {
         this.nodeAgentCertEnabled = nodeAgentCertEnabled;
         this.isRunningOnHost = isRunningOnHost;
         this.trustStorePath = trustStorePath;
+        this.dockerNetworking = Objects.requireNonNull(dockerNetworking, "dockerNetworking cannot be null");
     }
 
     public List<String> getConfigServerHostNames() { return configServerInfo.getConfigServerHostNames(); }
@@ -148,6 +143,8 @@ public class Environment {
         return system;
     }
 
+    public String getCloud() { return cloud; }
+
     public String getParentHostHostname() {
         return parentHostHostname;
     }
@@ -197,7 +194,7 @@ public class Environment {
     public Path pathInNodeAdminToNodeCleanup(ContainerName containerName) {
         return pathResolver.getApplicationStoragePathForNodeAdmin()
                 .resolve(APPLICATION_STORAGE_CLEANUP_PATH_PREFIX + containerName.asString() +
-                        "_" + filenameFormatter.format(Date.from(Instant.now())));
+                        "_" + filenameFormatter.format(Instant.now()));
     }
 
     /**
@@ -243,8 +240,6 @@ public class Environment {
 
     public NodeType getNodeType() { return nodeType; }
 
-    public String getCloud() { return cloud; }
-
     public ContainerEnvironmentResolver getContainerEnvironmentResolver() {
         return containerEnvironmentResolver;
     }
@@ -282,7 +277,7 @@ public class Environment {
     }
 
     public DockerNetworking getDockerNetworking() {
-        return DockerNetworking.from(cloud, nodeType, isRunningOnHost);
+        return dockerNetworking;
     }
 
     public static class Builder {
@@ -290,13 +285,13 @@ public class Environment {
         private String environment;
         private String region;
         private String system;
+        private String cloud;
         private String parentHostHostname;
         private IPAddresses ipAddresses;
         private PathResolver pathResolver;
         private List<String> logstashNodes = Collections.emptyList();
         private Optional<String> coredumpFeedEndpoint = Optional.empty();
         private NodeType nodeType = NodeType.tenant;
-        private String cloud;
         private ContainerEnvironmentResolver containerEnvironmentResolver;
         private String certificateDnsSuffix;
         private URI ztsUri;
@@ -304,6 +299,7 @@ public class Environment {
         private boolean nodeAgentCertEnabled;
         private boolean isRunningOnHost;
         private Path trustStorePath;
+        private DockerNetworking dockerNetworking;
 
         public Builder configServerConfig(ConfigServerConfig configServerConfig) {
             this.configServerConfig = configServerConfig;
@@ -325,6 +321,11 @@ public class Environment {
             return this;
         }
 
+        public Builder cloud(String cloud) {
+            this.cloud = cloud;
+            return this;
+        }
+
         public Builder parentHostHostname(String parentHostHostname) {
             this.parentHostHostname = parentHostHostname;
             return this;
@@ -360,11 +361,6 @@ public class Environment {
             return this;
         }
 
-        public Builder cloud(String cloud) {
-            this.cloud = cloud;
-            return this;
-        }
-
         public Builder certificateDnsSuffix(String certificateDnsSuffix) {
             this.certificateDnsSuffix = certificateDnsSuffix;
             return this;
@@ -395,25 +391,31 @@ public class Environment {
             return this;
         }
 
+        public Builder dockerNetworking(DockerNetworking dockerNetworking) {
+            this.dockerNetworking = dockerNetworking;
+            return this;
+        }
+
         public Environment build() {
             return new Environment(configServerConfig,
                                    trustStorePath,
                                    environment,
                                    region,
                                    system,
+                                   cloud,
                                    parentHostHostname,
                                    Optional.ofNullable(ipAddresses).orElseGet(IPAddressesImpl::new),
                                    Optional.ofNullable(pathResolver).orElseGet(PathResolver::new),
                                    logstashNodes,
                                    coredumpFeedEndpoint,
                                    nodeType,
-                                   cloud,
                                    Optional.ofNullable(containerEnvironmentResolver).orElseGet(DefaultContainerEnvironmentResolver::new),
                                    certificateDnsSuffix,
                                    ztsUri,
                                    nodeAthenzIdentity,
                                    nodeAgentCertEnabled,
-                                   isRunningOnHost);
+                                   isRunningOnHost,
+                                   Optional.ofNullable(dockerNetworking).orElseGet(() -> DockerNetworking.from(cloud, nodeType, isRunningOnHost)));
         }
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerNetworking.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerNetworking.java
index 60146a4fbb7..7678ad8169a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerNetworking.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerNetworking.java
@@ -10,13 +10,22 @@ import com.yahoo.config.provision.NodeType;
  */
 public enum DockerNetworking {
     /** Each container has an associated macvlan bridge. */
-    MACVLAN,
+    MACVLAN("vespa-macvlan"),
 
     /** Network Prefix-Translated networking. */
-    NPT,
+    NPT("vespa-bridge"),
 
     /** A host running a single container in the host network namespace. */
-    HOST_NETWORK;
+    HOST_NETWORK("host");
+
+    private final String dockerNetworkMode;
+    DockerNetworking(String dockerNetworkMode) {
+        this.dockerNetworkMode = dockerNetworkMode;
+    }
+
+    public String getDockerNetworkMode() {
+        return dockerNetworkMode;
+    }
 
     public static DockerNetworking from(String cloud, NodeType nodeType, boolean hostAdmin) {
         if (cloud.equals("AWS")) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index 47729ebb416..2b39239e8dc 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -45,8 +45,7 @@ public class DockerOperationsImpl implements DockerOperations {
 
     private static final String IPV6_NPT_PREFIX = "fd00::";
     private static final String IPV4_NPT_PREFIX = "172.17.0.0";
-    private static final String DOCKER_CUSTOM_BRIDGE_NETWORK_NAME = "vespa-bridge";
-    
+
     private final Docker docker;
     private final Environment environment;
     private final ProcessExecuter processExecuter;
@@ -107,11 +106,14 @@ public class DockerOperationsImpl implements DockerOperations {
             }
         }
 
-        if (environment.getDockerNetworking() == DockerNetworking.MACVLAN) {
+        DockerNetworking networking = environment.getDockerNetworking();
+        command.withNetworkMode(networking.getDockerNetworkMode());
+
+        if (networking == DockerNetworking.MACVLAN) { // TODO: Remove this else-if when migration to host-admin is complete
             command.withIpAddress(ipV6Address);
             command.withNetworkMode(DockerImpl.DOCKER_CUSTOM_MACVLAN_NETWORK_NAME);
             command.withSharedVolume("/etc/hosts", "/etc/hosts");
-        } else {
+        } else if (networking == DockerNetworking.NPT) {
             InetAddress ipV6Prefix = InetAddresses.forString(IPV6_NPT_PREFIX);
             InetAddress ipV6Local = IPAddresses.prefixTranslate(ipV6Address, ipV6Prefix, 8);
             command.withIpAddress(ipV6Local);
@@ -125,8 +127,6 @@ public class DockerOperationsImpl implements DockerOperations {
             ipV4Local.ifPresent(command::withIpAddress);
 
             addEtcHosts(containerData, node.getHostname(), ipV4Local, ipV6Local);
-
-            command.withNetworkMode(DOCKER_CUSTOM_BRIDGE_NETWORK_NAME);
         }
 
         for (Path pathInNode : directoriesToMount.keySet()) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index e2db75eb6fb..fa94a7ff819 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -39,6 +39,7 @@ public class DockerOperationsImplTest {
             .environment("prod")
             .system("main")
             .cloud("mycloud")
+            .dockerNetworking(DockerNetworking.HOST_NETWORK)
             .build();
     private final Docker docker = mock(Docker.class);
     private final ProcessExecuter processExecuter = mock(ProcessExecuter.class);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index d0e4377ffc5..15bb2825738 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl;
 import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
@@ -69,8 +70,9 @@ public class DockerTester implements AutoCloseable {
                 .region("us-east-1")
                 .environment("prod")
                 .system("main")
-                .pathResolver(new PathResolver(PATH_TO_VESPA_HOME, Paths.get("/tmp"), Paths.get("/tmp")))
                 .cloud("mycloud")
+                .pathResolver(new PathResolver(PATH_TO_VESPA_HOME, Paths.get("/tmp"), Paths.get("/tmp")))
+                .dockerNetworking(DockerNetworking.HOST_NETWORK)
                 .build();
 
         NodeSpec hostSpec = new NodeSpec.Builder()
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
index 77c0a30ae18..f418552553e 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/logging/FilebeatConfigProviderTest.java
@@ -6,6 +6,7 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Test;
 
@@ -104,6 +105,7 @@ public class FilebeatConfigProviderTest {
                 .system(system)
                 .logstashNodes(logstashNodes)
                 .cloud("mycloud")
+                .dockerNetworking(DockerNetworking.HOST_NETWORK)
                 .build();
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index 627517b824e..d9cce7f80a0 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
@@ -41,8 +42,9 @@ public class StorageMaintainerTest {
             .region("us-east-1")
             .environment("prod")
             .system("main")
-            .pathResolver(new PathResolver())
             .cloud("mycloud")
+            .pathResolver(new PathResolver())
+            .dockerNetworking(DockerNetworking.HOST_NETWORK)
             .coredumpFeedEndpoint("http://domain.tld/docid")
             .build();
     private final DockerOperations docker = mock(DockerOperations.class);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index f5d4dcf4e5e..ebed20326a3 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer;
@@ -88,9 +89,10 @@ public class NodeAgentImplTest {
             .environment("dev")
             .region("us-east-1")
             .system("main")
+            .cloud("mycloud")
             .parentHostHostname("parent.host.name.yahoo.com")
             .pathResolver(pathResolver)
-            .cloud("mycloud")
+            .dockerNetworking(DockerNetworking.HOST_NETWORK)
             .build();
 
     private final NodeSpec.Builder nodeBuilder = new NodeSpec.Builder()
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
index a3a455605ad..893607f1806 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/EnvironmentTest.java
@@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
+import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import org.junit.Test;
 
 import java.nio.file.Path;
@@ -22,8 +23,9 @@ public class EnvironmentTest {
             .region("us-east-1")
             .environment("prod")
             .system("main")
-            .pathResolver(new PathResolver())
             .cloud("mycloud")
+            .pathResolver(new PathResolver())
+            .dockerNetworking(DockerNetworking.HOST_NETWORK)
             .build();
 
     @Test