diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2022-06-09 11:32:44 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-09 11:32:44 +0200 |
commit | 321326830dee4ab7e7589e10159a99e4cd9b94f5 (patch) | |
tree | a0e7877515c26c8669c70d9b0be2f49a559e6a1c /configserver/src | |
parent | 9eca77acfbe01f5ceec2486b8171c28cdedcf441 (diff) | |
parent | af0281d676eb3d798a4d27aebcf57cf34666e3dd (diff) |
Merge pull request #23017 from vespa-engine/jonmv/always-fail-when-requested-model-version-faild-to-build
Always fail model building when wanted version fails
Diffstat (limited to 'configserver/src')
-rw-r--r-- | configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java | 26 | ||||
-rw-r--r-- | configserver/src/test/java/com/yahoo/vespa/config/server/deploy/HostedDeployTest.java | 34 |
2 files changed, 58 insertions, 2 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 f6f6a9ea24c..563989ddf05 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 @@ -128,6 +128,9 @@ 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)) { log.log(Level.INFO, applicationId + ": Skipping major version " + majorVersion, e); @@ -177,7 +180,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { if (buildLatestModelForThisMajor) { latest = Optional.of(findLatest(versions)); // load latest application version - MODELRESULT latestModelVersion = buildModelVersion(modelFactoryRegistry.getFactory(latest.get()), + MODELRESULT latestModelVersion = tryBuildModelVersion(modelFactoryRegistry.getFactory(latest.get()), applicationPackage, applicationId, wantedDockerImageRepository, @@ -192,7 +195,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { if (latest.isPresent() && version.equals(latest.get())) continue; // already loaded try { - MODELRESULT modelVersion = buildModelVersion(modelFactoryRegistry.getFactory(version), + MODELRESULT modelVersion = tryBuildModelVersion(modelFactoryRegistry.getFactory(version), applicationPackage, applicationId, wantedDockerImageRepository, @@ -252,6 +255,25 @@ 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); 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 10e3ab1ec97..5dfc1bbd3ef 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 @@ -52,6 +52,7 @@ import static com.yahoo.vespa.config.server.deploy.DeployTester.createHostedMode import static java.util.stream.Collectors.toList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -276,6 +277,39 @@ public class HostedDeployTest { * that are still using features that do not work on version 8.x) */ @Test + public void testWantedVersionIsRequiredAlsoWhenThereIsAnOlderMajorThatDoesNotFailModelBuilding() { + int oldMajor = 7; + int newMajor = 8; + Version wantedVersion = new Version(newMajor, 1, 2); + Version oldVersion = new Version(oldMajor, 2, 3); + List<Host> hosts = createHosts(9, oldVersion.toFullString(), wantedVersion.toFullString()); + + CountingModelFactory oldFactory = createHostedModelFactory(oldVersion); + ModelFactory newFactory = createFailingModelFactory(wantedVersion); + List<ModelFactory> modelFactories = List.of(oldFactory, newFactory); + + DeployTester tester = createTester(hosts, modelFactories, prodZone); + + // Not OK when failing version is requested. + assertEquals("Invalid application package", + assertThrows(IllegalArgumentException.class, + () -> tester.deployApp("src/test/apps/hosted/", wantedVersion.toFullString())) + .getMessage()); + + // OK when older version is requested. + tester.deployApp("src/test/apps/hosted/", oldVersion.toFullString()); + assertEquals(9, 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(oldFactory.creationCount() > 0); + } + + /** + * 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); } |