Only allow skipping creation of models on newest major version for major version >= 8

For major version 7 you either need to have an application package
that works on major version 7 or you need to explicitly specify
major-version=6 in application package, otherwise deployment will fail.

Remove tests in RemoteSession that are covered by other tests.

3 files changed, 46 insertions, 73 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
index 42afffaaddc..07c06f22497 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java
@@ -117,8 +117,9 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
                 throw e;
             }
             catch (RuntimeException e) {
-                boolean isOldestMajor = i == majorVersions.size() - 1;
-                if (isOldestMajor) {
+                if (shouldSkipCreatingMajorVersionOnError(majorVersions, majorVersion)) {
+                    log.log(LogLevel.INFO, applicationId + ": Skipping major version " + majorVersion, e);
+                } else  {
                     if (e instanceof NullPointerException || e instanceof NoSuchElementException | e instanceof UncheckedTimeoutException) {
                         log.log(LogLevel.WARNING, "Unexpected error when building model ", e);
                         throw new InternalServerException(applicationId + ": Error loading model", e);
@@ -126,8 +127,6 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
                         log.log(LogLevel.WARNING, "Input error when building model ", e);
                         throw new IllegalArgumentException(applicationId + ": Error loading model", e);
                     }
-                } else {
-                    log.log(LogLevel.INFO, applicationId + ": Skipping major version " + majorVersions.get(i), e);
                 }
             }
         }
@@ -135,6 +134,14 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
         return allApplicationModels;
     }
 
+    private boolean shouldSkipCreatingMajorVersionOnError(List<Integer> majorVersions, Integer majorVersion) {
+        if (majorVersion.equals(Collections.min(majorVersions))) return false;
+        // Note: This needs to be updated when we no longer want to support successfully deploying
+        // applications that are not working on version 8, but are working on a lower major version (unless
+        // apps have explicitly defined major version to deploy to in application package)
+        return majorVersion >= 8;
+    }
+
     // versions is the set of versions for one particular major version
     private List<MODELRESULT> buildModelVersions(Set<Version> versions,
                                                  ApplicationId applicationId,
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
index 689c66aa0f6..c5d1e8dc0a1 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java
@@ -165,29 +165,52 @@ public class HostedDeployTest {
     }
 
     /**
+     * Tests that we create the minimal set of models and that version 7.x is created
+     * if creating version 8.x fails (to support upgrades to new major version for applications
+     * that are still using features that do not work on version 8.x)
+     **/
+    @Test
+    public void testCreateLatestMajorOnPreviousMajorIfItFailsOnMajorVersion8() {
+        deployWithModelForLatestMajorVersionFailing(8);
+    }
+
+    /**
+     * Tests that we fail deployment for version 7.x if creating version 7.x fails (i.e. that we do not skip
+     * building 7.x and only build version 6.x). Skipping creation of models for a major version is only supported
+     * for major version >= 8 (see test above) or when major-version=6 is set in application package.
+     **/
+    @Test(expected = InvalidApplicationException.class)
+    public void testFailingToCreateModelVersion7FailsDeployment() {
+        deployWithModelForLatestMajorVersionFailing(7);
+    }
+
+    /**
      * Tests that we create the minimal set of models, but latest model version is created for
      * previous major if creating latest model version on latest major version fails
      **/
-    @Test
-    public void testCreateLatestMajorOnPreviousMajorIfItFailsOnNewestMajor() {
-        List<Host> hosts = Arrays.asList(createHost("host1", "6.0.0"),
-                                         createHost("host2", "6.1.0"),
-                                         createHost("host3", "6.1.0"));
+    private void deployWithModelForLatestMajorVersionFailing(int newestMajorVersion) {
+        int oldestMajorVersion = newestMajorVersion - 1;
+        String oldestVersion = oldestMajorVersion + ".0.0";
+        String newestOnOldMajorVersion = oldestMajorVersion + ".1.0";
+        String newestOnNewMajorVersion = newestMajorVersion + ".2.0";
+        List<Host> hosts = Arrays.asList(createHost("host1", oldestVersion),
+                                         createHost("host2", newestOnOldMajorVersion),
+                                         createHost("host3", newestOnOldMajorVersion));
         InMemoryProvisioner provisioner = new InMemoryProvisioner(new Hosts(hosts), true);
 
-        CountingModelFactory factory600 = DeployTester.createModelFactory(Version.fromString("6.0.0"));
-        CountingModelFactory factory610 = DeployTester.createModelFactory(Version.fromString("6.1.0"));
-        ModelFactory factory720 = DeployTester.createFailingModelFactory(Version.fromString("7.2.0"));
-        List<ModelFactory> modelFactories = Arrays.asList(factory600, factory610, factory720);
+        CountingModelFactory factory1 = DeployTester.createModelFactory(Version.fromString(oldestVersion));
+        CountingModelFactory factory2 = DeployTester.createModelFactory(Version.fromString(newestOnOldMajorVersion));
+        ModelFactory factory3 = DeployTester.createFailingModelFactory(Version.fromString(newestOnNewMajorVersion));
+        List<ModelFactory> modelFactories = Arrays.asList(factory1, factory2, factory3);
 
         DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(), Clock.systemUTC(), provisioner);
-        tester.deployApp("src/test/apps/hosted/", "6.0.0", Instant.now());
+        tester.deployApp("src/test/apps/hosted/", oldestVersion, Instant.now());
         assertEquals(3, tester.getAllocatedHostsOf(tester.applicationId()).getHosts().size());
 
         // Check >0 not ==0 as the session watcher thread is running and will redeploy models in the background
-        assertTrue(factory600.creationCount() > 0);
+        assertTrue(factory1.creationCount() > 0);
         assertTrue("Latest model for previous major version is included if latest model for latest major version fails to build",
-                   factory610.creationCount() > 0);
+                   factory2.creationCount() > 0);
     }
 
     /**
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java
index d5d0fe72dbe..c89c2f23873 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java
@@ -100,39 +100,6 @@ public class RemoteSessionTest {
     }
 
     @Test
-    public void require_that_application_incompatible_with_latestmajor_is_loaded_on_earlier_major() {
-        MockModelFactory okFactory1 = new MockModelFactory();
-        okFactory1.vespaVersion = new Version(1, 1, 0);
-        okFactory1.throwOnLoad = false;
-
-        MockModelFactory okFactory2 = new MockModelFactory();
-        okFactory2.vespaVersion = new Version(1, 2, 0);
-        okFactory2.throwOnLoad = false;
-
-        MockModelFactory failingFactory = new MockModelFactory();
-        failingFactory.vespaVersion = new Version(2, 0, 0);
-        failingFactory.throwOnLoad = true;
-
-        RemoteSession session = createSession(3, Arrays.asList(okFactory1, failingFactory, okFactory2));
-        session.loadPrepared();
-    }
-
-    @Test
-    public void require_that_old_invalid_application_does_not_throw_exception_if_skipped() {
-        MockModelFactory failingFactory = new MockModelFactory();
-        failingFactory.vespaVersion = new Version(1, 1, 0);
-        failingFactory.throwOnLoad = true;
-
-        MockModelFactory okFactory =
-            new MockModelFactory("<validation-overrides><allow until='2000-01-30'>skip-old-config-models</allow></validation-overrides>");
-        okFactory.vespaVersion = new Version(1, 2, 0);
-        okFactory.throwOnLoad = false;
-
-        RemoteSession session = createSession(3, Arrays.asList(okFactory, failingFactory));
-        session.loadPrepared();
-    }
-
-    @Test
     public void require_that_old_invalid_application_does_not_throw_exception_if_skipped_also_across_major_versions() {
         MockModelFactory failingFactory = new MockModelFactory();
         failingFactory.vespaVersion = new Version(1, 0, 0);
@@ -148,26 +115,6 @@ public class RemoteSessionTest {
     }
 
     @Test
-    public void require_that_old_invalid_application_does_not_throw_exception_if_skipped_also_when_new_major_is_incompatible() {
-        MockModelFactory failingFactory = new MockModelFactory();
-        failingFactory.vespaVersion = new Version(1, 0, 0);
-        failingFactory.throwOnLoad = true;
-
-        MockModelFactory okFactory =
-                new MockModelFactory("<validation-overrides><allow until='2000-01-30'>skip-old-config-models</allow></validation-overrides>");
-        okFactory.vespaVersion = new Version(1, 1, 0);
-        okFactory.throwOnLoad = false;
-
-        MockModelFactory tooNewFactory =
-                new MockModelFactory("<validation-overrides><allow until='2000-01-30'>skip-old-config-models</allow></validation-overrides>");
-        tooNewFactory.vespaVersion = new Version(2, 0, 0);
-        tooNewFactory.throwOnLoad = true;
-
-        RemoteSession session = createSession(3, Arrays.asList(tooNewFactory, okFactory, failingFactory));
-        session.loadPrepared();
-    }
-
-    @Test
     public void require_that_an_application_package_can_limit_to_one_major_version() {
         ApplicationPackage application =
                 new MockApplicationPackage.Builder().withServices("<services version='1.0'/>")
@@ -241,10 +188,6 @@ public class RemoteSessionTest {
         assertTrue(mockModelFactory.modelContext.permanentApplicationPackage().isPresent());
     }
 
-    private RemoteSession createSession(long sessionId) {
-        return createSession(sessionId, Collections.singletonList(new VespaModelFactory(new NullConfigModelRegistry())), Clock.systemUTC());
-    }
-
     private RemoteSession createSession(long sessionId, Clock clock) {
         return createSession(sessionId, Collections.singletonList(new VespaModelFactory(new NullConfigModelRegistry())), clock);
     }