Test that laoding an invalid session throws exception

Followup to https://github.com/vespa-engine/vespa/pull/18264

3 files changed, 27 insertions, 15 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
index 41d050025bf..9778b1fc1f2 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -175,7 +175,11 @@ public class SessionRepository {
     private void loadSessions(BooleanFlag loadLocalSessions) {
         ExecutorService executor = Executors.newFixedThreadPool(Math.max(8, Runtime.getRuntime().availableProcessors()),
                                                                 new DaemonThreadFactory("load-sessions-"));
-        if (loadLocalSessions.value())
+        loadSessions(loadLocalSessions.value(), executor);
+    }
+
+    void loadSessions(boolean loadLocalSessions, ExecutorService executor) {
+        if (loadLocalSessions)
             loadLocalSessions(executor);
         loadRemoteSessions(executor);
         try {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
index 31d29e7b78e..4790d8f4ae2 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.session;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
+import com.yahoo.concurrent.InThreadExecutorService;
 import com.yahoo.config.application.api.ApplicationFile;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.NullConfigModelRegistry;
@@ -37,6 +38,7 @@ import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.VespaModelFactory;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
@@ -79,6 +81,9 @@ public class SessionRepositoryTest {
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     public void setup() throws Exception {
         setup(new ModelFactoryRegistry(List.of(new VespaModelFactory(new NullConfigModelRegistry()))));
     }
@@ -172,21 +177,25 @@ public class SessionRepositoryTest {
         assertStatusChange(sessionId, Session.Status.ACTIVATE);
     }
 
-    // If reading a session throws an exception it should be handled and not prevent other applications
-    // from loading. In this test we just show that we end up with one session in remote session
-    // repo even if it had bad data (by making getSessionIdForApplication() in FailingTenantApplications
-    // throw an exception).
+    // If reading a session throws an exception when bootstrapping SessionRepository it should fail,
+    // to make sure config server does not comes up and serves invalid/old config or, if this is hosted,
+    // serves empty config (takes down services on all nodes belonging to an application)
     @Test
-    public void testBadApplicationRepoOnActivate() throws Exception {
+    public void testInvalidSessionWhenBootstrappingSessionRepo() throws Exception {
         setup();
-        long sessionId = 3L;
-        TenantName mytenant = TenantName.from("mytenant");
-        curator.set(TenantRepository.getApplicationsPath(mytenant).append("mytenant:appX:default"), new byte[0]); // Invalid data
-        tenantRepository.addTenant(mytenant);
-        curator.create(TenantRepository.getSessionsPath(mytenant));
+
+        // Create a session with invalid data and set active session for application to this session
+        String sessionIdString = "3";
+        Path sessionPath = TenantRepository.getSessionsPath(tenantName).append(sessionIdString);
+        curator.create(sessionPath);
+        curator.set(sessionPath.append("applicationId"), new byte[0]); // Invalid data
+        Path applicationsPath = TenantRepository.getApplicationsPath(tenantName);
+        curator.set(applicationsPath.append(applicationId.serializedForm()), Utf8.toBytes(sessionIdString));
+
+        expectedException.expectMessage("Could not load remote session " + sessionIdString);
+        expectedException.expect(RuntimeException.class);
+        sessionRepository.loadSessions(false, new InThreadExecutorService());
         assertThat(sessionRepository.getRemoteSessionsFromZooKeeper().size(), is(0));
-        createSession(sessionId, true);
-        assertThat(sessionRepository.getRemoteSessionsFromZooKeeper().size(), is(1));
     }
 
     @Test(expected = InvalidApplicationException.class)
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
index 6f7e0541cc7..464b3d1ab64 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
@@ -191,7 +191,6 @@ public class TenantRepositoryTest {
     public void testFailingBootstrap() {
         tenantRepository.close(); // stop using the one setup in Before method
 
-        // Should get exception if config is true
         expectedException.expect(RuntimeException.class);
         expectedException.expectMessage("Could not create all tenants when bootstrapping, failed to create: [default]");
         new FailingDuringBootstrapTenantRepository(configserverConfig);
@@ -213,7 +212,7 @@ public class TenantRepositoryTest {
                   Metrics.createTestMetrics(),
                   new StripedExecutor<>(new InThreadExecutorService()),
                   new StripedExecutor<>(new InThreadExecutorService()),
-                  new FileDistributionFactory(new ConfigserverConfig.Builder().build()),
+                  new FileDistributionFactory(configserverConfig),
                   new InMemoryFlagSource(),
                   new InThreadExecutorService(),
                   new MockSecretStore(),