diff options
6 files changed, 67 insertions, 58 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 b0cb287ed0b..560f650dcba 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 @@ -94,9 +94,9 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { List<MODELRESULT> allApplicationModels = new ArrayList<>(); for (int i = 0; i < majorVersions.size(); i++) { try { - allApplicationModels.addAll(buildModelVersion(filterByMajorVersion(majorVersions.get(i), versions), - applicationId, wantedNodeVespaVersion, applicationPackage, - allocatedHosts, now)); + allApplicationModels.addAll(buildModelVersions(filterByMajorVersion(majorVersions.get(i), versions), + applicationId, wantedNodeVespaVersion, applicationPackage, + allocatedHosts, now)); // 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)) @@ -125,11 +125,13 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { return allApplicationModels; } - private List<MODELRESULT> buildModelVersion(Set<Version> versions, ApplicationId applicationId, - com.yahoo.component.Version wantedNodeVespaVersion, - ApplicationPackage applicationPackage, - SettableOptional<AllocatedHosts> allocatedHosts, - Instant now) { + // versions is the set of versions for one particular major version + private List<MODELRESULT> buildModelVersions(Set<Version> versions, + ApplicationId applicationId, + com.yahoo.component.Version wantedNodeVespaVersion, + ApplicationPackage applicationPackage, + SettableOptional<AllocatedHosts> allocatedHosts, + Instant now) { Version latest = findLatest(versions); // load latest application version MODELRESULT latestModelVersion = buildModelVersion(modelFactoryRegistry.getFactory(latest), @@ -152,6 +154,11 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { || Arrays.asList("corp-us-east-1", "ap-southeast-1").contains(zone().region().value())) versions = keepThoseUsedOn(allocatedHosts.get(), versions); + // Make sure we build wanted version if we are building models for this major version + Version wantedVersion = Version.fromIntValues(wantedNodeVespaVersion.getMajor(), wantedNodeVespaVersion.getMinor(), wantedNodeVespaVersion.getMicro()); + if (wantedVersion.getMajor() == latest.getMajor()) + versions.add(wantedVersion); + // TODO: We use the allocated hosts from the newest version when building older model versions. // This is correct except for the case where an old model specifies a cluster which the new version // does not. In that case we really want to extend the set of allocated hosts to include those of that diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java index b53ee9a5988..f53a48b3783 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java @@ -95,6 +95,11 @@ public final class PrepareParams { return this; } + public Builder vespaVersion(com.yahoo.config.provision.Version vespaVersion) { + this.vespaVersion = Optional.ofNullable(Version.fromString(vespaVersion.toSerializedForm())); + return this; + } + public Builder rotations(String rotationsString) { this.rotations = new LinkedHashSet<>(); if (rotationsString != null && !rotationsString.isEmpty()) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java b/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java index d2965b8a489..5f00499598a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.model.VespaModelFactory; -import java.io.File; import java.time.Clock; import java.util.Collections; import java.util.Optional; @@ -155,7 +154,7 @@ public class TestComponentRegistry implements GlobalComponentRegistry { final PermanentApplicationPackage permApp = this.permanentApplicationPackage .orElse(new PermanentApplicationPackage(configserverConfig)); FileDistributionFactory fileDistributionFactory = this.fileDistributionFactory - .orElse(new MockFileDistributionFactory(new File(configserverConfig.fileReferencesDir()))); + .orElse(new MockFileDistributionFactory(configserverConfig)); HostProvisionerProvider hostProvisionerProvider = hostProvisioner.isPresent() ? HostProvisionerProvider.withProvisioner(hostProvisioner.get()) : HostProvisionerProvider.empty(); 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 b151ac57352..6ee4720b6b0 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 @@ -16,9 +16,9 @@ import com.yahoo.test.ManualClock; import static com.yahoo.vespa.config.server.deploy.DeployTester.CountingModelFactory; import com.yahoo.vespa.config.server.session.LocalSession; -import org.junit.Ignore; import org.junit.Test; +import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -38,7 +38,8 @@ public class HostedDeployTest { @Test public void testRedeployWithVersion() { - DeployTester tester = new DeployTester("src/test/apps/hosted/", createConfigserverConfig()); + CountingModelFactory modelFactory = DeployTester.createModelFactory(Version.fromString("4.5.6"), Clock.systemUTC()); + DeployTester tester = new DeployTester("src/test/apps/hosted/", Collections.singletonList(modelFactory), createConfigserverConfig()); tester.deployApp("myApp", "4.5.6", Instant.now()); Optional<com.yahoo.config.provision.Deployment> deployment = tester.redeployFromLocalActive(); @@ -73,14 +74,14 @@ public class HostedDeployTest { modelFactories.add(DeployTester.createModelFactory(Version.fromString("6.2.0"), clock)); modelFactories.add(DeployTester.createModelFactory(Version.fromString("7.0.0"), clock)); DeployTester tester = new DeployTester("src/test/apps/hosted/", modelFactories, createConfigserverConfig(), clock, Zone.defaultZone()); - ApplicationId app = tester.deployApp("myApp", Instant.now()); + ApplicationId app = tester.deployApp("myApp", "6.2.0", Instant.now()); assertEquals(3, tester.getAllocatedHostsOf(app).getHosts().size()); } - /** Test that unused versions are skipped in dev */ + /** Test that only the minimal set of models are created (model versions used on hosts, the wanted version and the latest version) */ @Test - public void testDeployMultipleVersionsInDev() { + public void testCreateOnlyNeededModelVersions() { List<Host> hosts = new ArrayList<>(); hosts.add(createHost("host1", "6.0.0")); hosts.add(createHost("host2", "6.0.2")); @@ -104,14 +105,15 @@ public class HostedDeployTest { DeployTester tester = new DeployTester("src/test/apps/hosted/", modelFactories, createConfigserverConfig(), clock, new Zone(Environment.dev, RegionName.defaultName()), provisioner); - ApplicationId app = tester.deployApp("myApp", Instant.now()); + // Deploy with version that does not exist on hosts, the model for this version should also be created + ApplicationId app = tester.deployApp("myApp", "7.0.0", Instant.now()); assertEquals(3, tester.getAllocatedHostsOf(app).getHosts().size()); // Check >0 not ==0 as the session watcher thread is running and will redeploy models in the background assertTrue(factory600.creationCount() > 0); assertFalse(factory610.creationCount() > 0); assertTrue(factory620.creationCount() > 0); - assertFalse(factory700.creationCount() > 0); + assertTrue(factory700.creationCount() > 0); assertTrue(factory710.creationCount() > 0); assertTrue("Newest is always included", factory720.creationCount() > 0); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockFileDistributionFactory.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockFileDistributionFactory.java index 2c05017d449..e1874c622c2 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockFileDistributionFactory.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/MockFileDistributionFactory.java @@ -14,9 +14,9 @@ public class MockFileDistributionFactory extends FileDistributionFactory { public final MockFileDistributionProvider mockFileDistributionProvider; - public MockFileDistributionFactory(File fileReferencesDir) { - super(new ConfigserverConfig(new ConfigserverConfig.Builder())); - mockFileDistributionProvider = new MockFileDistributionProvider(fileReferencesDir); + public MockFileDistributionFactory(ConfigserverConfig configserverConfig) { + super(configserverConfig); + mockFileDistributionProvider = new MockFileDistributionProvider(new File(configserverConfig.fileReferencesDir())); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java index 92fb67fdd54..6f26323b558 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java @@ -21,6 +21,7 @@ import com.yahoo.vespa.config.server.*; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.server.application.MemoryTenantApplications; import com.yahoo.vespa.config.server.application.PermanentApplicationPackage; +import com.yahoo.vespa.config.server.configchange.ConfigChangeActions; import com.yahoo.vespa.config.server.configchange.MockRestartAction; import com.yahoo.vespa.config.server.configchange.RestartActions; import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger; @@ -49,8 +50,7 @@ import static org.hamcrest.Matchers.contains; import static org.junit.Assert.*; /** - * @author lulf - * @since 5.1 + * @author Ulf Lilleengen */ public class SessionPreparerTest { @@ -58,6 +58,10 @@ public class SessionPreparerTest { private static final Path sessionsPath = tenantPath.append("sessions").append("testapp"); private static final File testApp = new File("src/test/apps/app"); private static final File invalidTestApp = new File("src/test/apps/illegalApp"); + private static final Version version123 = Version.fromIntValues(1, 2, 3); + private static final Version version124 = Version.fromIntValues(1, 2, 4); + private static final Version version321 = Version.fromIntValues(3, 2, 1); + private static final Version version323 = Version.fromIntValues(3, 2, 3); private MockCurator curator; private ConfigCurator configCurator; @@ -83,9 +87,8 @@ public class SessionPreparerTest { } private SessionPreparer createPreparer(HostProvisionerProvider hostProvisionerProvider) { - ModelFactoryRegistry modelFactoryRegistry = new ModelFactoryRegistry(Arrays.asList( - new TestModelFactory(Version.fromIntValues(1, 2, 3)), - new TestModelFactory(Version.fromIntValues(3, 2, 1)))); + ModelFactoryRegistry modelFactoryRegistry = + new ModelFactoryRegistry(Arrays.asList(new TestModelFactory(version123), new TestModelFactory(version321))); return createPreparer(modelFactoryRegistry, hostProvisionerProvider); } @@ -104,37 +107,29 @@ public class SessionPreparerTest { @Test(expected = InvalidApplicationException.class) public void require_that_application_validation_exception_is_not_caught() throws IOException { - FilesApplicationPackage app = getApplicationPackage(invalidTestApp); - preparer.prepare(getContext(app), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath, Instant.now()); + prepare(invalidTestApp); } @Test public void require_that_application_validation_exception_is_ignored_if_forced() throws IOException { - FilesApplicationPackage app = getApplicationPackage(invalidTestApp); - preparer.prepare(getContext(app), getLogger(), - new PrepareParams.Builder().ignoreValidationErrors(true).timeoutBudget(TimeoutBudgetTest.day()).build(), - Optional.empty(), tenantPath, Instant.now()); + prepare(invalidTestApp, new PrepareParams.Builder().ignoreValidationErrors(true).timeoutBudget(TimeoutBudgetTest.day()).build()); } @Test public void require_that_zookeeper_is_not_written_to_if_dryrun() throws IOException { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), - new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build(), - Optional.empty(), tenantPath, Instant.now()); + prepare(testApp, new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build()); assertFalse(configCurator.exists(sessionsPath.append(ConfigCurator.USERAPP_ZK_SUBPATH).append("services.xml").getAbsolute())); } @Test public void require_that_filedistribution_is_ignored_on_dryrun() throws IOException { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), - new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build(), - Optional.empty(), tenantPath, Instant.now()); + prepare(testApp, new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build()); assertThat(fileDistributionFactory.mockFileDistributionProvider.timesCalled, is(0)); } @Test public void require_that_application_is_prepared() throws Exception { - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath, Instant.now()); + prepare(testApp); assertThat(fileDistributionFactory.mockFileDistributionProvider.timesCalled, is(2)); assertTrue(configCurator.exists(sessionsPath.append(ConfigCurator.USERAPP_ZK_SUBPATH).append("services.xml").getAbsolute())); } @@ -142,19 +137,19 @@ public class SessionPreparerTest { @Test public void require_that_prepare_succeeds_if_newer_version_fails() throws IOException { ModelFactoryRegistry modelFactoryRegistry = new ModelFactoryRegistry(Arrays.asList( - new TestModelFactory(Version.fromIntValues(1, 2, 3)), - new FailingModelFactory(Version.fromIntValues(3, 2, 1), new IllegalArgumentException("BOOHOO")))); + new TestModelFactory(version123), + new FailingModelFactory(version321, new IllegalArgumentException("BOOHOO")))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath, Instant.now()); + prepare(testApp); } @Test(expected = InvalidApplicationException.class) public void require_that_prepare_fails_if_older_version_fails() throws IOException { ModelFactoryRegistry modelFactoryRegistry = new ModelFactoryRegistry(Arrays.asList( - new TestModelFactory(Version.fromIntValues(3, 2, 3)), - new FailingModelFactory(Version.fromIntValues(1, 2, 1), new IllegalArgumentException("BOOHOO")))); + new TestModelFactory(version323), + new FailingModelFactory(version123, new IllegalArgumentException("BOOHOO")))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), new PrepareParams.Builder().build(), Optional.empty(), tenantPath, Instant.now()); + prepare(testApp); } @Test(expected = InvalidApplicationException.class) @@ -184,7 +179,7 @@ public class SessionPreparerTest { .tenant(tenant) .applicationName("foo").instanceName("quux").build(); PrepareParams params = new PrepareParams.Builder().applicationId(origId).build(); - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), params, Optional.empty(), tenantPath, Instant.now()); + prepare(testApp, params); SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, sessionsPath); assertTrue(configCurator.exists(sessionsPath.append(SessionZooKeeperClient.APPLICATION_ID_PATH).getAbsolute())); assertThat(zkc.readApplicationId(), is(origId)); @@ -194,15 +189,10 @@ public class SessionPreparerTest { public void require_that_config_change_actions_are_collected_from_all_models() throws IOException { ServiceInfo service = new ServiceInfo("serviceName", "serviceType", null, new HashMap<>(), "configId", "hostName"); ModelFactoryRegistry modelFactoryRegistry = new ModelFactoryRegistry(Arrays.asList( - new ConfigChangeActionsModelFactory(Version.fromIntValues(1, 2, 3), - new MockRestartAction("change", Arrays.asList(service))), - new ConfigChangeActionsModelFactory(Version.fromIntValues(1, 2, 4), - new MockRestartAction("other change", Arrays.asList(service))))); + new ConfigChangeActionsModelFactory(version123, new MockRestartAction("change", Arrays.asList(service))), + new ConfigChangeActionsModelFactory(version124, new MockRestartAction("other change", Arrays.asList(service))))); preparer = createPreparer(modelFactoryRegistry, HostProvisionerProvider.empty()); - List<RestartActions.Entry> actions = - preparer.prepare(getContext(getApplicationPackage(testApp)), getLogger(), - new PrepareParams.Builder().build(), Optional.empty(), tenantPath, Instant.now()) - .getRestartActions().getEntries(); + List<RestartActions.Entry> actions = prepare(testApp).getRestartActions().getEntries(); assertThat(actions.size(), is(1)); assertThat(actions.get(0).getMessages(), equalTo(ImmutableSet.of("change", "other change"))); } @@ -216,15 +206,13 @@ public class SessionPreparerTest { final String rotations = "mediasearch.msbe.global.vespa.yahooapis.com"; final ApplicationId applicationId = applicationId("test"); PrepareParams params = new PrepareParams.Builder().applicationId(applicationId).rotations(rotations).build(); - File app = new File("src/test/resources/deploy/app"); - preparer.prepare(getContext(getApplicationPackage(app)), getLogger(), params, Optional.empty(), tenantPath, Instant.now()); + prepare(new File("src/test/resources/deploy/app"), params); assertThat(readRotationsFromZK(applicationId), contains(new Rotation(rotations))); } @Test public void require_that_rotations_are_read_from_zookeeper_and_used() throws IOException { - final Version vespaVersion = Version.fromIntValues(1, 2, 3); - final TestModelFactory modelFactory = new TestModelFactory(vespaVersion); + final TestModelFactory modelFactory = new TestModelFactory(version123); preparer = createPreparer(new ModelFactoryRegistry(Arrays.asList(modelFactory)), HostProvisionerProvider.empty()); @@ -233,7 +221,7 @@ public class SessionPreparerTest { new Rotations(curator, tenantPath).writeRotationsToZooKeeper(applicationId, Collections.singleton(new Rotation(rotations))); final PrepareParams params = new PrepareParams.Builder().applicationId(applicationId).build(); final File app = new File("src/test/resources/deploy/app"); - preparer.prepare(getContext(getApplicationPackage(app)), getLogger(), params, Optional.empty(), tenantPath, Instant.now()); + prepare(app, params); // check that the rotation from zookeeper were used final ModelContext modelContext = modelFactory.getModelContext(); @@ -244,6 +232,14 @@ public class SessionPreparerTest { assertThat(readRotationsFromZK(applicationId), contains(new Rotation(rotations))); } + private ConfigChangeActions prepare(File app) throws IOException { + return prepare(app, new PrepareParams.Builder().build()); + } + + private ConfigChangeActions prepare(File app, PrepareParams params) throws IOException { + return preparer.prepare(getContext(getApplicationPackage(app)), getLogger(), params, Optional.empty(), tenantPath, Instant.now()); + } + private SessionContext getContext(FilesApplicationPackage app) throws IOException { return new SessionContext(app, new SessionZooKeeperClient(curator, sessionsPath), app.getAppDir(), new MemoryTenantApplications(), new HostRegistry<>(), new SuperModelGenerationCounter(curator)); } |