From 0861faaf480fbbd185d650e6bcf8122d88d3c8de Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 11 Nov 2019 12:58:02 +0100 Subject: Always try to build config model for old versions Try to build old config models, but do not fail deployment if skip-old-config-models validation override is true and building an old model fails. In manually deployed zones we previously did not build old config models, since skip-old-config-models was always true. With this change we will try to build those old models, but not fail if building one or more old models fails. --- .../com/yahoo/vespa/model/VespaModelFactory.java | 14 ++-- .../config/server/modelfactory/ModelsBuilder.java | 33 +++++----- .../vespa/config/server/deploy/DeployTester.java | 13 ++-- .../config/server/deploy/HostedDeployTest.java | 75 ++++++++++++++++++++-- 4 files changed, 106 insertions(+), 29 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java index 1dc0b838470..54807697da4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java @@ -66,14 +66,20 @@ public class VespaModelFactory implements ModelFactory { this.clock = Clock.systemUTC(); } - + + // For testing only public VespaModelFactory(ConfigModelRegistry configModelRegistry) { this(configModelRegistry, Clock.systemUTC()); } + + // For testing only public VespaModelFactory(ConfigModelRegistry configModelRegistry, Clock clock) { - this(new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro), configModelRegistry, clock); + this(new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro), configModelRegistry, + clock, Zone.defaultZone()); } - public VespaModelFactory(Version version, ConfigModelRegistry configModelRegistry, Clock clock) { + + // For testing only + public VespaModelFactory(Version version, ConfigModelRegistry configModelRegistry, Clock clock, Zone zone) { this.version = version; if (configModelRegistry == null) { this.configModelRegistry = new NullConfigModelRegistry(); @@ -82,7 +88,7 @@ public class VespaModelFactory implements ModelFactory { this.configModelRegistry = configModelRegistry; } this.modelImporters = Collections.emptyList(); - this.zone = Zone.defaultZone(); + this.zone = zone; this.clock = clock; } 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 e25574c86d3..2de64ed145c 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 @@ -106,10 +106,6 @@ public abstract class ModelsBuilder { allApplicationModels.addAll(buildModelVersions(keepMajorVersion(majorVersion, versions), applicationId, wantedNodeVespaVersion, applicationPackage, allocatedHosts, now, buildLatestModelForThisMajor, majorVersion)); - - // skip old config models if requested after we have found a major version which works - if (allApplicationModels.size() > 0 && allApplicationModels.get(0).getModel().skipOldConfigModels(now)) - break; buildLatestModelForThisMajor = false; // We have successfully built latest model version, do it only for this major } catch (OutOfCapacityException | ApplicationLockException | TransientException e) { @@ -169,9 +165,6 @@ public abstract class ModelsBuilder { now); allocatedHosts.set(latestModelVersion.getModel().allocatedHosts()); // Update with additional clusters allocated allApplicationVersions.add(latestModelVersion); - - if (latestModelVersion.getModel().skipOldConfigModels(now)) - return allApplicationVersions; } // load old model versions @@ -184,14 +177,24 @@ public abstract class ModelsBuilder { for (Version version : versions) { if (latest.isPresent() && version.equals(latest.get())) continue; // already loaded - MODELRESULT modelVersion = buildModelVersion(modelFactoryRegistry.getFactory(version), - applicationPackage, - applicationId, - wantedNodeVespaVersion, - allocatedHosts.asOptional(), - now); - allocatedHosts.set(modelVersion.getModel().allocatedHosts()); // Update with additional clusters allocated - allApplicationVersions.add(modelVersion); + MODELRESULT modelVersion; + try { + modelVersion = buildModelVersion(modelFactoryRegistry.getFactory(version), + applicationPackage, + applicationId, + wantedNodeVespaVersion, + allocatedHosts.asOptional(), + now); + allocatedHosts.set(modelVersion.getModel().allocatedHosts()); // Update with additional clusters allocated + allApplicationVersions.add(modelVersion); + } catch (RuntimeException e) { + // allow failure to create old config models if there is a validation override that allow skipping old + // config models (which is always true for manually deployed zones) + if (allApplicationVersions.size() > 0 && allApplicationVersions.get(0).getModel().skipOldConfigModels(now)) + log.log(LogLevel.INFO, applicationId + ": Skipping old version (due to validation override)"); + else + throw e; + } } return allApplicationVersions; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index f619bd92bef..eacc2d5a4fc 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -143,12 +143,17 @@ public class DeployTester { /** Create a model factory for a particular version */ public static CountingModelFactory createModelFactory(Version version) { - return new CountingModelFactory(version, Clock.systemUTC()); + return createModelFactory(version, Clock.systemUTC()); } /** Create a model factory for a particular version */ public static CountingModelFactory createModelFactory(Version version, Clock clock) { - return new CountingModelFactory(version, clock); + return createModelFactory(version, clock, Zone.defaultZone()); + } + + /** Create a model factory for a particular version */ + public static CountingModelFactory createModelFactory(Version version, Clock clock, Zone zone) { + return new CountingModelFactory(version, clock, zone); } /** Create a model factory which always fails validation */ @@ -299,8 +304,8 @@ public class DeployTester { this.wrapped = new VespaModelFactory(new NullConfigModelRegistry(), clock); } - public CountingModelFactory(Version version, Clock clock) { - this.wrapped = new VespaModelFactory(version, new NullConfigModelRegistry(), clock); + public CountingModelFactory(Version version, Clock clock, Zone zone) { + this.wrapped = new VespaModelFactory(version, new NullConfigModelRegistry(), clock, zone); } /** Returns the number of models created successfully by this instance */ 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 74d3fd4ec74..26cc72672b9 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 @@ -135,7 +135,6 @@ public class HostedDeployTest { assertTrue("Newest is always included", factory720.creationCount() > 0); } - /** * Test that only the minimal set of models are created (the wanted version and the latest version for * the latest major, since nodes are without version) @@ -161,6 +160,63 @@ public class HostedDeployTest { assertTrue("Newest model for latest major version is always included", factory720.creationCount() > 0); } + /** + * Test that deploying an application in a manually deployed zone creates all needed model versions + * (not just the latest one, manually deployed apps always have skipOldConfigModels set to true) + */ + @Test + public void testCreateNeededModelVersionsForManuallyDeployedApps() { + List hosts = List.of(createHost("host1", "7.0.0"), createHost("host2", "7.0.0"), + createHost("host3", "7.0.0"), createHost("host4", "7.0.0")); + InMemoryProvisioner provisioner = new InMemoryProvisioner(new Hosts(hosts), true); + + ManualClock clock = new ManualClock("2019-11-08T00:00:00"); + Zone zone = new Zone(Environment.dev, RegionName.defaultName()); + CountingModelFactory factory700 = DeployTester.createModelFactory(Version.fromString("7.0.0"), clock, zone); + CountingModelFactory factory710 = DeployTester.createModelFactory(Version.fromString("7.1.0"), clock, zone); + CountingModelFactory factory720 = DeployTester.createModelFactory(Version.fromString("7.2.0"), clock, zone); + List modelFactories = List.of(factory700, factory710, factory720); + + DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(zone), + clock, zone, provisioner); + // Deploy with version that does not exist on hosts, the model for this version should also be created + tester.deployApp("src/test/apps/hosted/", "7.2.0", Instant.now()); + assertEquals(4, tester.getAllocatedHostsOf(tester.applicationId()).getHosts().size()); + + // Check >0 not ==0 as the session watcher thread is running and will redeploy models in the background + // Nodes are on 7.0.0 (should be created), no nodes on 7.1.0 (should not be created), 7.2.0 should always be creates + assertTrue(factory700.creationCount() > 0); + assertFalse(factory710.creationCount() > 0); + assertTrue("Newest model for latest major version is always included", factory720.creationCount() > 0); + } + + /** + * Test that deploying an application in a manually deployed zone creates latest model version successfully, + * even if creating one of the older model fails + */ + @Test + public void testCreateModelVersionsForManuallyDeployedAppsWhenCreatingFailsForOneVersion() { + List hosts = List.of(createHost("host1", "7.0.0"), createHost("host2", "7.0.0"), + createHost("host3", "7.0.0"), createHost("host4", "7.0.0")); + InMemoryProvisioner provisioner = new InMemoryProvisioner(new Hosts(hosts), true); + + Zone zone = new Zone(Environment.dev, RegionName.defaultName()); + ManualClock clock = new ManualClock("2019-11-08T00:00:00"); + ModelFactory factory700 = DeployTester.createFailingModelFactory(Version.fromString("7.0.0")); + CountingModelFactory factory720 = DeployTester.createModelFactory(Version.fromString("7.2.0"), clock, zone); + List modelFactories = List.of(factory700, factory720); + + DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(zone), + clock, zone, provisioner); + // Deploy with version that does not exist on hosts, the model for this version should be created even + // if creating 7.0.0 fails + tester.deployApp("src/test/apps/hosted/", "7.2.0", Instant.now()); + assertEquals(4, 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("Newest model for latest major version is always included", factory720.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 @@ -244,8 +300,7 @@ public class HostedDeployTest { CountingModelFactory factory720 = DeployTester.createModelFactory(Version.fromString("7.2.0"), clock); List modelFactories = Arrays.asList(factory700, factory720); - DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(), - clock, new Zone(Environment.dev, RegionName.defaultName()), provisioner); + DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(), clock, provisioner); tester.deployApp("src/test/apps/hosted-routing-app/", "7.2.0", Instant.now()); assertFalse(factory700.creationCount() > 0); assertTrue("Newest is always included", factory720.creationCount() > 0); @@ -262,8 +317,9 @@ public class HostedDeployTest { CountingModelFactory factory620 = DeployTester.createModelFactory(Version.fromString("6.2.0")); List modelFactories = Arrays.asList(factory600, factory610, factory620); - DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(), - Clock.systemUTC(), new Zone(Environment.prod, RegionName.defaultName()), provisioner); + Zone zone = new Zone(Environment.prod, RegionName.defaultName()); + DeployTester tester = new DeployTester(modelFactories, createConfigserverConfig(zone), + Clock.systemUTC(), zone, provisioner); ApplicationId applicationId = tester.applicationId(); // Deploy with oldest version tester.deployApp("src/test/apps/hosted/", "6.0.0", Instant.now()); @@ -339,11 +395,18 @@ public class HostedDeployTest { } private static ConfigserverConfig createConfigserverConfig() { + return createConfigserverConfig(Zone.defaultZone()); + } + + private static ConfigserverConfig createConfigserverConfig(Zone zone) { return new ConfigserverConfig(new ConfigserverConfig.Builder() .configServerDBDir(Files.createTempDir().getAbsolutePath()) .configDefinitionsDir(Files.createTempDir().getAbsolutePath()) .hostedVespa(true) - .multitenant(true)); + .multitenant(true) + .region(zone.region().value()) + .environment(zone.environment().value()) + .system(zone.system().value())); } private Host createHost(String hostname, String version) { -- cgit v1.2.3