Disallow model building failures on any version with a major shared by a deployed or target version

1 files changed, 15 insertions, 28 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 563989ddf05..361d966ebc5 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
@@ -92,6 +92,9 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
         Instant start = Instant.now();
         log.log(Level.FINE, () -> "Will build models for " + applicationId);
         Set<Version> versions = modelFactoryRegistry.allVersions();
+        if (applicationPackage.getMajorVersion().isPresent() && applicationPackage.getMajorVersion().get() != wantedNodeVespaVersion.getMajor())
+            throw new IllegalArgumentException("requested node version (" + wantedNodeVespaVersion + ") has a different major version " +
+                                               "than specified in deployment.xml (" + applicationPackage.getMajorVersion().get() + ")");
 
         // If the application specifies a major, skip models on a newer major
         Optional<Integer> requestedMajorVersion = applicationPackage.getMajorVersion();
@@ -128,11 +131,8 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
                 // caused by the state of the system, not the model version/application combination
                 throw e;
             }
-            catch (RequireModelVersionFailureException e) {
-                throw e.getCause();
-            }
             catch (RuntimeException e) {
-                if (shouldSkipCreatingMajorVersionOnError(majorVersions, majorVersion)) {
+                if (shouldSkipCreatingMajorVersionOnError(majorVersions, majorVersion, wantedNodeVespaVersion, allocatedHosts)) {
                     log.log(Level.INFO, applicationId + ": Skipping major version " + majorVersion, e);
                 }
                 else {
@@ -157,9 +157,15 @@ 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
+    private boolean shouldSkipCreatingMajorVersionOnError(List<Integer> majorVersions, Integer majorVersion, Version wantedVersion,
+                                                          AllocatedHostsFromAllModels allHosts) {
+        if (majorVersion.equals(wantedVersion.getMajor())) return false;        // Ensure we are valid for our targeted major.
+        if (allHosts.toAllocatedHosts().getHosts().stream()
+                    .flatMap(host -> host.version().stream())
+                    .map(Version::getMajor)
+                    .anyMatch(majorVersion::equals)) return false;              // Ensure we are valid for our currently deployed major.
+        if (majorVersion.equals(Collections.min(majorVersions))) return false;  // Probably won't happen if the other two are both false ... ?
+        // Note: This needs to be bumped 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;
@@ -180,7 +186,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
         if (buildLatestModelForThisMajor) {
             latest = Optional.of(findLatest(versions));
             // load latest application version
-            MODELRESULT latestModelVersion = tryBuildModelVersion(modelFactoryRegistry.getFactory(latest.get()),
+            MODELRESULT latestModelVersion = buildModelVersion(modelFactoryRegistry.getFactory(latest.get()),
                                                                applicationPackage,
                                                                applicationId,
                                                                wantedDockerImageRepository,
@@ -195,7 +201,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
             if (latest.isPresent() && version.equals(latest.get())) continue; // already loaded
 
             try {
-                MODELRESULT modelVersion = tryBuildModelVersion(modelFactoryRegistry.getFactory(version),
+                MODELRESULT modelVersion = buildModelVersion(modelFactoryRegistry.getFactory(version),
                                                              applicationPackage,
                                                              applicationId,
                                                              wantedDockerImageRepository,
@@ -255,25 +261,6 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> {
                                .anyMatch(host -> host.version().isPresent() && host.version().get().equals(version));
     }
 
-    private MODELRESULT tryBuildModelVersion(ModelFactory modelFactory, ApplicationPackage applicationPackage,
-                                             ApplicationId applicationId, Optional<DockerImage> dockerImageRepository,
-                                             Version wantedNodeVespaVersion) {
-        try {
-            return buildModelVersion(modelFactory, applicationPackage, applicationId, dockerImageRepository, wantedNodeVespaVersion);
-        }
-        catch (RuntimeException e) {
-            if (modelFactory.version().equals(wantedNodeVespaVersion))
-                throw new RequireModelVersionFailureException(e);
-
-            throw e;
-        }
-    }
-
-    private static class RequireModelVersionFailureException extends RuntimeException {
-        RequireModelVersionFailureException(RuntimeException cause) { super(cause); }
-        @Override public RuntimeException getCause() { return (RuntimeException) super.getCause(); }
-    }
-
     protected abstract MODELRESULT buildModelVersion(ModelFactory modelFactory, ApplicationPackage applicationPackage,
                                                      ApplicationId applicationId, Optional<DockerImage> dockerImageRepository,
                                                      Version wantedNodeVespaVersion);