Remove special handling of deployment of routing application

* Routing application uses nodes from node repo in all zones now,
  handle as other applications
VESPA-5118

5 files changed, 21 insertions, 184 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
index 3aef44d1f90..35ea2134813 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
@@ -16,23 +16,16 @@ import com.yahoo.config.model.api.ModelCreateResult;
 import com.yahoo.config.model.api.ModelFactory;
 import com.yahoo.config.model.application.provider.ApplicationPackageXmlFilesValidator;
 import com.yahoo.config.model.builder.xml.ConfigModelBuilder;
-import com.yahoo.config.model.builder.xml.XmlHelper;
 import com.yahoo.config.model.deploy.DeployProperties;
 import com.yahoo.config.model.deploy.DeployState;
-import com.yahoo.config.model.provision.HostsXmlProvisioner;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Version;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.text.XML;
 import com.yahoo.vespa.config.VespaVersion;
 import com.yahoo.vespa.model.application.validation.Validation;
 
-import org.w3c.dom.Document;
-import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
 
 import java.io.IOException;
-import java.io.Reader;
 import java.time.Clock;
 import java.util.ArrayList;
 import java.util.List;
@@ -147,45 +140,8 @@ public class VespaModelFactory implements ModelFactory {
     }
 
     private static HostProvisioner createHostProvisioner(ModelContext modelContext) {
-        // TODO: Remove this (use only else part of if statement) when zone applications uses nodes in node repo in all zones
-        if (isHostedVespaRoutingApplication(modelContext) && ! useRoutingNodesInNodeRepo(modelContext.applicationPackage().getServices())) {
-            //TODO: This belongs in HostedVespaProvisioner.
-            //Added here for now since com.yahoo.config.model.api.HostProvisioner is not created per application,
-            //and allocation is independent of ApplicationPackage.
-            return new HostsXmlProvisioner(hostsXml(modelContext));
-        } else {
-            return modelContext.hostProvisioner().orElse(
-                    DeployState.getDefaultModelHostProvisioner(modelContext.applicationPackage()));
-        }
-    }
-
-    // Returns true if nodes element has an attribute 'type', false otherwise
-    public static boolean useRoutingNodesInNodeRepo(Reader servicesReader) {
-        Document services = XmlHelper.getDocument(servicesReader);
-
-        Element jdisc = XML.getChild(services.getDocumentElement(), "jdisc");
-        if (jdisc == null) return false;
-
-        Element nodes = XML.getChild(jdisc, "nodes");
-        if (nodes == null) return false;
-
-        return nodes.hasAttribute("type");
-    }
-
-    private static Reader hostsXml(ModelContext modelContext) {
-        Reader hosts = modelContext.applicationPackage().getHosts();
-        if (hosts == null) {
-            //TODO: throw InvalidApplicationException directly. Not possible now, as it resides in the configserver module.
-            //SessionPreparer maps IllegalArgumentException -> InvalidApplicationException
-            throw new IllegalArgumentException("Hosted vespa routing application must use " + ApplicationPackage.HOSTS
-                                               + " to allocate hosts.");
-        }
-        return hosts;
-    }
-
-    private static boolean isHostedVespaRoutingApplication(ModelContext modelContext) {
-        ApplicationId id = modelContext.properties().applicationId();
-        return modelContext.properties().hostedVespa() && id.isHostedVespaRoutingApplication();
+        return modelContext.hostProvisioner().orElse(
+                DeployState.getDefaultModelHostProvisioner(modelContext.applicationPackage()));
     }
 
     private void validateXML(ApplicationPackage applicationPackage, boolean ignoreValidationErrors) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java
index 9e149a8cf2e..671690030b6 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/VespaModelFactoryTest.java
@@ -10,7 +10,6 @@ import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.config.provision.*;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.internal.stubbing.answers.ThrowsExceptionClass;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -23,9 +22,6 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 /**
  * @author lulf
@@ -72,94 +68,6 @@ public class VespaModelFactoryTest {
         assertTrue(createResult.getConfigChangeActions().isEmpty());
     }
 
-    // TODO: This test seems to do exactly the same as hostedVespaZoneApplicationAllocatesNodesWithHostsXml()
-    @Test
-    public void hostedVespaRoutingApplicationAllocatesNodesWithHostsXml() {
-        String hostName = "test-host-name";
-        String routingClusterName = "routing-cluster";
-
-        String hosts =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                        "<hosts>\n" +
-                        "  <host name='" + hostName + "'>\n" +
-                        "    <alias>proxy1</alias>\n" +
-                        "  </host>\n" +
-                        "</hosts>";
-
-        String services =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                        "<services version='1.0' xmlns:deploy='vespa'>\n" +
-                        "    <admin version='2.0'>\n" +
-                        "        <adminserver hostalias='proxy1' />\n" +
-                        "    </admin>" +
-                        "    <jdisc id='" + routingClusterName + "' version='1.0'>\n" +
-                        "        <nodes>\n" +
-                        "            <node hostalias='proxy1' />\n" +
-                        "        </nodes>\n" +
-                        "    </jdisc>\n" +
-                        "</services>";
-
-        HostProvisioner provisionerToOverride =
-                mock(HostProvisioner.class, new ThrowsExceptionClass(UnsupportedOperationException.class));
-        ModelContext modelContext = createMockModelContext(hosts, services, provisionerToOverride);
-        Model model = new VespaModelFactory(new NullConfigModelRegistry()).createModel(modelContext);
-
-        List<HostInfo> allocatedHosts = new ArrayList<>(model.getHosts());
-        assertThat(allocatedHosts.size(), is(1));
-        HostInfo hostInfo = allocatedHosts.get(0);
-
-        assertThat(hostInfo.getHostname(), is(hostName));
-
-        assertTrue("Routing service should run on host " + hostName,
-                hostInfo.getServices().stream()
-                        .map(ServiceInfo::getConfigId)
-                        .anyMatch(configId -> configId.contains(routingClusterName)));
-    }
-
-    @Test
-    public void hostedVespaZoneApplicationAllocatesNodesWithHostsXml() {
-        String hostName = "test-host-name";
-        String routingClusterName = "routing-cluster";
-
-        String hosts =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                        "<hosts>\n" +
-                        "  <host name='" + hostName + "'>\n" +
-                        "    <alias>proxy1</alias>\n" +
-                        "  </host>\n" +
-                        "</hosts>";
-
-        String services =
-                "<?xml version='1.0' encoding='utf-8' ?>\n" +
-                        "<services version='1.0' xmlns:deploy='vespa'>\n" +
-                        "    <admin version='2.0'>\n" +
-                        "        <adminserver hostalias='proxy1' />\n" +
-                        "    </admin>" +
-                        "    <jdisc id='" + routingClusterName + "' version='1.0'>\n" +
-                        "        <nodes>\n" +
-                        "            <node hostalias='proxy1' />\n" +
-                        "        </nodes>\n" +
-                        "    </jdisc>\n" +
-                        "</services>";
-
-        HostProvisioner provisionerToOverride =
-                mock(HostProvisioner.class, new ThrowsExceptionClass(UnsupportedOperationException.class));
-
-        ModelContext modelContext = createMockModelContext(hosts, services, provisionerToOverride);
-        Model model = new VespaModelFactory(new NullConfigModelRegistry()).createModel(modelContext);
-
-        List<HostInfo> allocatedHosts = new ArrayList<>(model.getHosts());
-        assertThat(allocatedHosts.size(), is(1));
-        HostInfo hostInfo = allocatedHosts.get(0);
-
-        assertThat(hostInfo.getHostname(), is(hostName));
-
-        assertTrue("Routing service should run on host " + hostName,
-                hostInfo.getServices().stream()
-                        .map(ServiceInfo::getConfigId)
-                        .anyMatch(configId -> configId.contains(routingClusterName)));
-    }
-
     @Test
     public void hostedVespaZoneApplicationAllocatesNodesFromNodeRepo() {
         String hostName = "test-host-name";
@@ -257,7 +165,9 @@ public class VespaModelFactoryTest {
 
                     @Override
                     public ApplicationId applicationId() {
-                        return ApplicationId.HOSTED_ZONE_APPLICATION_ID;
+                        return ApplicationId.from(TenantName.from("hosted-vespa"),
+                                                  ApplicationName.from("routing"),
+                                                  InstanceName.defaultName());
                     }
 
                     @Override
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java
index 3c465db5e5c..af6f67040cb 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java
@@ -62,6 +62,8 @@ public final class ApplicationId implements Comparable<ApplicationId> {
         this.serializedForm = toSerializedForm();
     }
 
+    // TODO: Remove code between lines with ----------- when oldest config model used is 6.49.
+    /* ------------------- */
     public static final TenantName HOSTED_VESPA_TENANT = TenantName.from("hosted-vespa");
     // TODO: Remove references to routing application, or rename them to zone
     // application, once everything (like Chef recipes) refers to the zone
@@ -81,6 +83,7 @@ public final class ApplicationId implements Comparable<ApplicationId> {
 	    (ROUTING_APPLICATION.equals(application) ||
 	     ZONE_APPLICATION.equals(application));
     }
+    /* ------------------- */
 
     public static ApplicationId from(TenantName tenant, ApplicationName application, InstanceName instanceName) {
         return new ApplicationId(tenant, application, instanceName);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
index 11de94813c5..cb4533903bd 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
@@ -2,17 +2,11 @@
 package com.yahoo.vespa.config.server.deploy;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
-import com.yahoo.config.application.api.ApplicationFile;
-import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
-import com.yahoo.config.model.builder.xml.XmlHelper;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostFilter;
 import com.yahoo.config.provision.Provisioner;
-import com.yahoo.config.provision.ProvisionInfo;
 import com.yahoo.log.LogLevel;
 import com.yahoo.path.Path;
-import com.yahoo.text.XML;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.transaction.Transaction;
 import com.yahoo.vespa.config.server.tenant.ActivateLock;
@@ -24,11 +18,7 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo;
 import com.yahoo.vespa.config.server.session.PrepareParams;
 import com.yahoo.vespa.config.server.session.Session;
 import com.yahoo.vespa.config.server.session.SilentDeployLogger;
-import org.w3c.dom.Document;
-import org.w3c.dom.Element;
 
-import java.io.FileNotFoundException;
-import java.io.Reader;
 import java.time.Clock;
 import java.time.Duration;
 import java.util.Optional;
@@ -140,8 +130,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
             transaction.add(deactivateCurrentActivateNew(localSessionRepo.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure));
 
             // TODO: (October 2016) Remove the second part of this if statement as soon as all zone applications stop using hosts.xml for routing nodes
-            if (hostProvisioner.isPresent() &&
-                    (isNotHostedRoutingApplication(session.getApplicationId()) || isHostedRoutingApplicationUsingRoutingNodesInNodeRepo(session))) {
+            if (hostProvisioner.isPresent()) {
                 hostProvisioner.get().activate(transaction, session.getApplicationId(), session.getProvisionInfo().getHosts());
             }
             transaction.commit();
@@ -227,36 +216,4 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         }
     }
 
-    private boolean isNotHostedRoutingApplication(ApplicationId applicationId) {
-        return ! applicationId.isHostedVespaRoutingApplication();
-    }
-
-    // Precondition: session is for a hosted routing application
-    boolean isHostedRoutingApplicationUsingRoutingNodesInNodeRepo(LocalSession session) {
-        Path servicesPath = Path.fromString(".preprocessed/" + ApplicationPackage.SERVICES);
-        ApplicationFile services = session.getApplicationFile(servicesPath, LocalSession.Mode.READ);
-
-        if ( ! services.exists()) return false;
-
-        try {
-            return usesRoutingNodesInNodeRepo(services.createReader());
-        } catch (FileNotFoundException e) {
-            throw new RuntimeException("Could not create reader for " + servicesPath + " for '" + session.getApplicationId() + "'");
-        }
-    }
-
-    // TODO: Copied verbatim from VespaModelFactory, since we need it now and it is not available for all model versions yet.
-    //       Remove or use the one from VespaModelFactory as soon as possible
-    private boolean usesRoutingNodesInNodeRepo(Reader servicesReader) {
-        Document services = XmlHelper.getDocument(servicesReader);
-
-        Element jdisc = XML.getChild(services.getDocumentElement(), "jdisc");
-        if (jdisc == null) return false;
-
-        Element nodes = XML.getChild(jdisc, "nodes");
-        if (nodes == null) return false;
-
-        return nodes.hasAttribute("type");
-    }
-
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/model/RoutingProducer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/model/RoutingProducer.java
index 6a63269ae6e..32d718f9390 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/model/RoutingProducer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/model/RoutingProducer.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.model;
 import com.yahoo.cloud.config.RoutingConfig;
 import com.yahoo.config.model.api.HostInfo;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.config.server.application.Application;
 
@@ -16,6 +17,9 @@ import java.util.Map;
  * @since 5.60
  */
 public class RoutingProducer implements RoutingConfig.Producer {
+    private static final TenantName HOSTED_VESPA_TENANT = TenantName.from("hosted-vespa");
+    private static final ApplicationName ROUTING_APPLICATION = ApplicationName.from("routing");
+    private static final ApplicationName ZONE_APPLICATION = ApplicationName.from("zone");
 
     private final Map<TenantName, Map<ApplicationId, Application>> models;
 
@@ -26,11 +30,18 @@ public class RoutingProducer implements RoutingConfig.Producer {
     @Override
     public void getConfig(RoutingConfig.Builder builder) {
         for (Map<ApplicationId, Application> model : models.values()) {
-            model.values().stream().filter(application -> application.getId().isHostedVespaRoutingApplication()).forEach(application -> {
+            model.values().stream().filter(application -> isHostedVespaRoutingApplication(application.getId())).forEach(application -> {
                 for (HostInfo host : application.getModel().getHosts()) {
                     builder.hosts(host.getHostname());
                 }
             });
         }
     }
+
+    private boolean isHostedVespaRoutingApplication(ApplicationId applicationId) {
+        return HOSTED_VESPA_TENANT.equals(applicationId.tenant()) &&
+                (ROUTING_APPLICATION.equals(applicationId.application()) ||
+                        ZONE_APPLICATION.equals(applicationId.application()));
+    }
+
 }