Register applications in TenantApplications before activating their sessions

9 files changed, 44 insertions, 29 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
index 656030bb8d2..92add4d053b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
@@ -32,6 +32,7 @@ import java.util.stream.Collectors;
  * of whatever session may be activated next, if any, and /lock is used for synchronizing writes to all these paths.
  *
  * @author Ulf Lilleengen
+ * @author jonmv
  */
 public class TenantApplications {
 
@@ -73,6 +74,7 @@ public class TenantApplications {
     public List<ApplicationId> activeApplications() {
         return curator.getChildren(applicationsPath).stream()
                       .filter(this::isValid)
+                      .sorted()
                       .map(ApplicationId::fromSerializedForm)
                       .filter(id -> activeSessionOf(id).isPresent())
                       .collect(Collectors.toUnmodifiableList());
@@ -109,11 +111,14 @@ public class TenantApplications {
      * @param sessionId Id of the session containing the application package for this id.
      */
     public Transaction createPutTransaction(ApplicationId applicationId, long sessionId) {
-        if (curator.exists(applicationPath(applicationId))) {
-            return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId)));
-        } else {
-            return new CuratorTransaction(curator).add(CuratorOperations.create(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId)));
-        }
+        return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId)));
+    }
+
+    /**
+     * Creates a node for the given application, marking its existence.
+     */
+    public void createApplication(ApplicationId id) {
+        curator.create(applicationPath(id));
     }
 
     /**
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java
index d8ba5890545..cee18580094 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java
@@ -139,6 +139,7 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader {
 
     private LocalSession create(File applicationFile, ApplicationId applicationId, long currentlyActiveSessionId,
                                 boolean internalRedeploy, TimeoutBudget timeoutBudget) {
+        applicationRepo.createApplication(applicationId);
         long sessionId = sessionCounter.nextSessionId();
         Path sessionIdPath = sessionsPath.append(String.valueOf(sessionId));
         log.log(LogLevel.DEBUG, TenantRepository.logPre(tenant) + "Next session id is " + sessionId + " , sessionIdPath=" + sessionIdPath.getAbsolute());
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
index 01a7d5e0239..69c88dc0275 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
@@ -41,10 +41,10 @@ public class TenantApplicationsTest {
         TenantApplications repo = createZKAppRepo();
         List<ApplicationId> applications = repo.activeApplications();
         assertThat(applications.size(), is(2));
-        assertThat(applications.get(0).application().value(), is("foo"));
-        assertThat(applications.get(1).application().value(), is("bar"));
-        assertThat(repo.requireActiveSessionOf(applications.get(0)), is(3L));
-        assertThat(repo.requireActiveSessionOf(applications.get(1)), is(4L));
+        assertThat(applications.get(0).application().value(), is("bar"));
+        assertThat(applications.get(1).application().value(), is("foo"));
+        assertThat(repo.requireActiveSessionOf(applications.get(0)), is(4L));
+        assertThat(repo.requireActiveSessionOf(applications.get(1)), is(3L));
     }
 
     @Test
@@ -77,6 +77,7 @@ public class TenantApplicationsTest {
     public void require_that_application_ids_can_be_written() throws Exception {
         TenantApplications repo = createZKAppRepo();
         ApplicationId myapp = createApplicationId("myapp");
+        repo.createApplication(myapp);
         repo.createPutTransaction(myapp, 3l).commit();
         String path = TenantRepository.getApplicationsPath(tenantName).append(myapp.serializedForm()).getAbsolute();
         assertTrue(curatorFramework.checkExists().forPath(path) != null);
@@ -91,6 +92,8 @@ public class TenantApplicationsTest {
         TenantApplications repo = createZKAppRepo();
         ApplicationId id1 = createApplicationId("myapp");
         ApplicationId id2 = createApplicationId("myapp2");
+        repo.createApplication(id1);
+        repo.createApplication(id2);
         repo.createPutTransaction(id1, 1).commit();
         repo.createPutTransaction(id2, 1).commit();
         assertThat(repo.activeApplications().size(), is(2));
@@ -126,11 +129,7 @@ public class TenantApplicationsTest {
     }
 
     private static ApplicationId createApplicationId(String name) {
-        return new ApplicationId.Builder()
-                .tenant(tenantName.value())
-                .applicationName(name)
-                .instanceName("myinst")
-                .build();
+        return ApplicationId.from(tenantName.value(), name, "myinst");
     }
 
     private void writeApplicationData(ApplicationId applicationId, long sessionId) throws Exception {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java
index 3d34a4eeaf5..c6a8e1f2f9d 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java
@@ -56,11 +56,13 @@ public class ApplicationContentHandlerTest extends ContentHandlerTestBase {
         session2 = new MockSession(2l, FilesApplicationPackage.fromFile(new File("src/test/apps/content")));
         Tenant tenant1 = tenantRepository.getTenant(tenantName1);
         tenant1.getLocalSessionRepo().addSession(session2);
+        tenant1.getApplicationRepo().createApplication(idTenant1);
         tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2l).commit();
 
         MockSession session3 = new MockSession(3l, FilesApplicationPackage.fromFile(new File("src/test/apps/content2")));
         Tenant tenant2 = tenantRepository.getTenant(tenantName2);
         tenant2.getLocalSessionRepo().addSession(session3);
+        tenant2.getApplicationRepo().createApplication(idTenant2);
         tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3l).commit();
 
         handler = new ApplicationHandler(ApplicationHandler.testOnlyContext(),
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java
index fb75e91dfd6..1db70956407 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java
@@ -46,6 +46,7 @@ public class HostHandlerTest {
     private HostHandler hostHandler;
 
     static void addMockApplication(Tenant tenant, ApplicationId applicationId, long sessionId) {
+        tenant.getApplicationRepo().createApplication(applicationId);
         tenant.getApplicationRepo().createPutTransaction(applicationId, sessionId).commit();
         ApplicationPackage app = FilesApplicationPackage.fromFile(testApp);
         tenant.getLocalSessionRepo().addSession(new SessionHandlerTest.MockSession(sessionId, app, applicationId));
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java
index b34f7a0c487..f97bc443a38 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java
@@ -51,17 +51,17 @@ public class ListApplicationsHandlerTest {
         final String url = "http://myhost:14000/application/v2/tenant/mytenant/application/";
         assertResponse(url, Response.Status.OK,
                 "[]");
-        applicationRepo.createPutTransaction(
-                new ApplicationId.Builder().tenant("tenant").applicationName("foo").instanceName("quux").build(),
-                1).commit();
+        ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux");
+        applicationRepo.createApplication(id1);
+        applicationRepo.createPutTransaction(id1, 1).commit();
         assertResponse(url, Response.Status.OK,
                 "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]");
-        applicationRepo.createPutTransaction(
-                new ApplicationId.Builder().tenant("tenant").applicationName("bali").instanceName("quux").build(),
-                1).commit();
+        ApplicationId id2 = ApplicationId.from("mytenant", "bali", "quux");
+        applicationRepo.createApplication(id2);
+        applicationRepo.createPutTransaction(id2, 1).commit();
         assertResponse(url, Response.Status.OK,
-                "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"," +
-                        "\"" + url + "bali/environment/dev/region/us-east/instance/quux\"]"
+                "[\"" + url + "bali/environment/dev/region/us-east/instance/quux\"," +
+                        "\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"
         );
     }
 
@@ -82,12 +82,12 @@ public class ListApplicationsHandlerTest {
 
     @Test
     public void require_that_listing_works_with_multiple_tenants() throws Exception {
-        applicationRepo.createPutTransaction(new ApplicationId.Builder()
-                .tenant("tenant")
-                .applicationName("foo").instanceName("quux").build(), 1).commit();
-        applicationRepo2.createPutTransaction(new ApplicationId.Builder()
-                .tenant("tenant")
-                .applicationName("quux").instanceName("foo").build(), 1).commit();
+        ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux");
+        applicationRepo.createApplication(id1);
+        applicationRepo.createPutTransaction(id1, 1).commit();
+        ApplicationId id2 = ApplicationId.from("foobar", "quux", "foo");
+        applicationRepo2.createApplication(id2);
+        applicationRepo2.createPutTransaction(id2, 1).commit();
         String url = "http://myhost:14000/application/v2/tenant/mytenant/application/";
         assertResponse(url, Response.Status.OK,
                 "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]");
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
index 380b76c30af..858d1e0eaa7 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
@@ -323,6 +323,9 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
                                             Optional.of(AllocatedHosts.withHosts(Collections.singleton(new HostSpec("bar", Collections.emptyList())))));
             session = createRemoteSession(sessionId, initialStatus, zkClient);
             addLocalSession(sessionId, deployData, zkClient);
+            tenantRepository.getTenant(tenantName).getApplicationRepo().createApplication(ApplicationId.from(tenantName.value(),
+                                                                                                             deployData.getApplicationName(),
+                                                                                                             InstanceName.defaultName().value()));
             metaData = localRepo.getSession(sessionId).getMetaData();
             actResponse = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, sessionId, subPath));
             return this;
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
index bc509fcd802..0946ef3992c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java
@@ -194,6 +194,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
                               .applicationName("foo")
                               .instanceName("quux")
                               .build();
+        applicationRepo.createApplication(fooId);
         applicationRepo.createPutTransaction(fooId, 2).commit();
         assertFromParameter("3", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foo/environment/test/region/baz/instance/quux");
         localSessionRepo.addSession(new SessionHandlerTest.MockSession(5l, FilesApplicationPackage.fromFile(testApp)));
@@ -202,6 +203,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest {
                               .applicationName("foobio")
                               .instanceName("quux")
                               .build();
+        applicationRepo.createApplication(bioId);
         applicationRepo.createPutTransaction(bioId, 5).commit();
         assertFromParameter("6", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foobio/environment/staging/region/baz/instance/quux");
     }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java
index e7db4dcf58f..a4432dcbfcd 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java
@@ -196,12 +196,14 @@ public class LocalSessionTest {
         zkClient.write(Collections.singletonMap(new Version(0, 0, 0), new MockFileRegistry()));
         File sessionDir = new File(tenantFileSystemDirs.sessionsPath(), String.valueOf(sessionId));
         sessionDir.createNewFile();
+        TenantApplications applications = TenantApplications.create(curator, new MockReloadHandler(), tenant);
+        applications.createApplication(zkc.readApplicationId());
         return new LocalSession(tenant, sessionId, preparer,
                 new SessionContext(
                         FilesApplicationPackage.fromFile(testApp),
                         zkc,
                         sessionDir,
-                        TenantApplications.create(curator, new MockReloadHandler(), tenant),
+                        applications,
                         new HostRegistry<>(),
                         superModelGenerationCounter,
                         flagSource));