Merge pull request #5 from yahoo/smorgrav/bcp_id_fix

Lookup ApplicationInstanceReference matching an ApplicationId

6 files changed, 49 insertions, 38 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationIdNotFoundException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationIdNotFoundException.java
index 55fc720307e..d12bd66bc86 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationIdNotFoundException.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationIdNotFoundException.java
@@ -7,4 +7,12 @@ package com.yahoo.vespa.orchestrator;
  * @author smorgrav
  */
 public class ApplicationIdNotFoundException extends Exception {
+
+    public ApplicationIdNotFoundException() {
+        super();
+    }
+
+    public ApplicationIdNotFoundException(String reason) {
+        super(reason);
+    }
 }
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
index 0033de1288f..7197c9bcd43 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java
@@ -136,7 +136,7 @@ public class OrchestratorImpl implements Orchestrator {
     @Override
     public ApplicationInstanceStatus getApplicationInstanceStatus(
             final ApplicationId appId) throws ApplicationIdNotFoundException {
-        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId);
+        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId,instanceLookupService);
         return statusService.forApplicationInstance(appRef).getApplicationInstanceStatus();
     }
 
@@ -271,7 +271,8 @@ public class OrchestratorImpl implements Orchestrator {
             final ApplicationId appId,
             final ApplicationInstanceStatus status) throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException{
 
-        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId);
+
+        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService);
         try (MutableStatusRegistry statusRegistry =
                      statusService.lockApplicationInstance_forCurrentThreadOnly(appRef)) {
 
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
index ca7bd058169..fff69cfd17b 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorUtil.java
@@ -5,8 +5,6 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.orchestrator.status.HostStatus;
-import com.yahoo.vespa.orchestrator.status.ReadOnlyStatusRegistry;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceId;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
@@ -14,8 +12,11 @@ import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceCluster;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
 import com.yahoo.vespa.applicationmodel.TenantId;
+import com.yahoo.vespa.orchestrator.status.HostStatus;
+import com.yahoo.vespa.orchestrator.status.ReadOnlyStatusRegistry;
 
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -108,37 +109,38 @@ public class OrchestratorUtil {
     }
 
 
-    public static ApplicationInstanceReference toApplicationInstanceReference(ApplicationId appId) {
-        TenantId tenantId = new TenantId(appId.tenant().toString());
+    public static ApplicationInstanceReference toApplicationInstanceReference(ApplicationId appId,
+                                                                              InstanceLookupService instanceLookupService)
+            throws ApplicationIdNotFoundException {
+
+        Set<ApplicationInstanceReference> appRefs = instanceLookupService.knownInstances();
+        List<ApplicationInstanceReference> appRefList = appRefs.stream()
+                .filter(a -> OrchestratorUtil.toApplicationId(a).equals(appId))
+                .collect(Collectors.toList());
 
-        String appName = appId.application().toString();
-        String instanceName = appId.instance().toString();
-        ApplicationInstanceId appInstanceId = new ApplicationInstanceId(appName + ":" + instanceName);
+        if (appRefList.size() > 1) {
+            String msg = String.format("ApplicationId '%s' was not unique but mapped to '%s'", appId, appRefList);
+            throw new ApplicationIdNotFoundException(msg);
+        }
 
-        return new ApplicationInstanceReference(tenantId,appInstanceId);
+        if (appRefList.size() == 0) {
+            throw new ApplicationIdNotFoundException();
+        }
+
+        return appRefList.get(0);
     }
 
     public static ApplicationId toApplicationId(ApplicationInstanceReference appRef) {
-        TenantName tenantName = TenantName.from(appRef.tenantId().toString());
 
-        // Now for the application/instance pair we need to split this
-        String appNameStr = appRef.applicationInstanceId().toString();
+        String appNameStr = appRef.toString();
         String[] appNameParts = appNameStr.split(":");
-
-        // We assume a valid application reference has at lest two parts appname:instancename
-        // TODO is this assumption valid?
-        if (appNameParts.length < 2)  {
-            // TODO Since this is used internally we should perhapes use another exception type?
-            throw new IllegalArgumentException("Application reference not valid: " + appRef);
+        // TODO model ApplicationInstanceReference properly and validate this there
+        if (appNameParts.length != 5)  {
+            throw new IllegalArgumentException("Application reference not valid (not 5 parts): " + appRef);
         }
 
-        // Last part of string is the instance name
-        InstanceName instanceName = InstanceName.from(appNameParts[appNameParts.length-1]);
-
-        // The rest is application
-        int whereAppNameEnds = appNameStr.lastIndexOf(":");
-        ApplicationName appName = ApplicationName.from(appNameStr.substring(0, whereAppNameEnds));
-
-        return ApplicationId.from(tenantName, appName, instanceName);
+        return ApplicationId.from(TenantName.from(appNameParts[0]),
+                ApplicationName.from(appNameParts[1]),
+                InstanceName.from(appNameParts[4]));
     }
 }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
index e1b73c1fe65..d48bc1f302b 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/DummyInstanceLookupService.java
@@ -37,7 +37,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
     static {
         apps.add(new ApplicationInstance<>(
                 new TenantId("test-tenant-id"),
-                new ApplicationInstanceId("application:instance"),
+                new ApplicationInstanceId("application:prod:utopia-1:instance"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("test-cluster-id-1"),
@@ -49,7 +49,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
                                                 ServiceMonitorStatus.UP),
                                         new ServiceInstance<>(
                                                 new ConfigId("storage/storage/2"),
-                                                new HostName("test2.prod.utpoia-1.vespahosted.ut1.yahoo.com"),
+                                                new HostName("test2.prod.utopoia-1.vespahosted.ut1.yahoo.com"),
                                                 ServiceMonitorStatus.UP))),
                         new ServiceCluster<>(
                                 new ClusterId("clustercontroller"),
@@ -65,7 +65,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
 
         apps.add(new ApplicationInstance<>(
                 new TenantId("mediasearch"),
-                new ApplicationInstanceId("imagesearch:default"),
+                new ApplicationInstanceId("imagesearch:prod:utopia-1:default"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("image"),
@@ -93,7 +93,7 @@ public class DummyInstanceLookupService implements InstanceLookupService {
 
         apps.add(new ApplicationInstance<>(
                 new TenantId("tenant-id-3"),
-                new ApplicationInstanceId("application-instance-3:default"),
+                new ApplicationInstanceId("application-instance-3:prod:utopia-1:default"),
                 TestUtil.makeServiceClusterSet(
                         new ServiceCluster<>(
                                 new ClusterId("cluster-id-3"),
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
index b9812ac7ab6..548d67689d2 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java
@@ -216,7 +216,7 @@ public class OrchestratorImplTest {
         InstanceLookupService service = new DummyInstanceLookupService();
         String applicationInstanceId = service.findInstanceByHost(DummyInstanceLookupService.TEST1_HOST_NAME).get()
                 .reference().toString();
-        assertEquals("test-tenant-id:application:instance", applicationInstanceId);
+        assertEquals("test-tenant-id:application:prod:utopia-1:instance", applicationInstanceId);
     }
 
     @Test
@@ -291,7 +291,7 @@ public class OrchestratorImplTest {
 
     private boolean isInMaintenance(ApplicationId appId, HostName hostName) throws ApplicationIdNotFoundException {
         for (ApplicationInstance<ServiceMonitorStatus> app : DummyInstanceLookupService.getApplications()) {
-            if (app.reference().equals(OrchestratorUtil.toApplicationInstanceReference(appId))) {
+            if (app.reference().equals(OrchestratorUtil.toApplicationInstanceReference(appId,new DummyInstanceLookupService()))) {
                 return clustercontroller.isInMaintenance(app, hostName);
             }
         }
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
index 41daef34c25..2655f6b2b49 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorUtilTest.java
@@ -18,12 +18,12 @@ public class OrchestratorUtilTest {
 
     private static final ApplicationId APPID_1 = ApplicationId.from(
             TenantName.from("mediasearch"),
-            ApplicationName.from("tumblr-search"),
+            ApplicationName.from("imagesearch"),
             InstanceName.defaultName());
 
     private static final ApplicationInstanceReference APPREF_1 = new ApplicationInstanceReference(
-            new TenantId("test-tenant"),
-            new ApplicationInstanceId("test-application:test-environment:test-region:test-instance-key"));
+            new TenantId("test-tenant-id"),
+            new ApplicationInstanceId("application:prod:utopia-1:instance"));
 
     /**
      * Here we don't care how the internal of the different application
@@ -35,14 +35,14 @@ public class OrchestratorUtilTest {
     public void applicationid_conversion_are_symmetric() throws Exception {
 
         // From appId to appRef and back
-        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(APPID_1);
+        ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(APPID_1, new DummyInstanceLookupService());
         ApplicationId appIdRoundTrip = OrchestratorUtil.toApplicationId(appRef);
 
         Assert.assertEquals(APPID_1, appIdRoundTrip);
 
         // From appRef to appId and back
         ApplicationId appId = OrchestratorUtil.toApplicationId(APPREF_1);
-        ApplicationInstanceReference appRefRoundTrip = OrchestratorUtil.toApplicationInstanceReference(appId);
+        ApplicationInstanceReference appRefRoundTrip = OrchestratorUtil.toApplicationInstanceReference(appId, new DummyInstanceLookupService());
 
         Assert.assertEquals(APPREF_1, appRefRoundTrip);
     }