diff options
42 files changed, 237 insertions, 207 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java index d2365bcbf00..990bce539ba 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java @@ -18,8 +18,7 @@ public interface FileDistribution { void sendDeployedFiles(String hostName, Set<FileReference> fileReferences); void reloadDeployFileDistributor(); - // TODO: Remove when 6.150 is the oldest version used - void limitSendingOfDeployedFilesTo(Collection<String> hostNames); + void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames); static String getDefaultFileDBRoot() { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java index fc3e46a46d6..b0c04cea1b0 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Flavor.java @@ -76,6 +76,9 @@ public class Flavor { } public Type getType() { return type; } + + /** Convenience, returns getType() == Type.DOCKER_CONTAINER */ + public boolean isDocker() { return type == Type.DOCKER_CONTAINER; } /** The free capacity we would like to preserve for this flavor */ public int getIdealHeadroom() { @@ -131,6 +134,14 @@ public class Flavor { public void freeze() { replacesFlavors = ImmutableList.copyOf(replacesFlavors); } + + /** Returns whether this flavor has at least as much as each hardware resource as the given flavor */ + public boolean isLargerThan(Flavor other) { + return this.minCpuCores >= other.minCpuCores && + this.minDiskAvailableGb >= other.minDiskAvailableGb && + this.minMainMemoryAvailableGb >= other.minMainMemoryAvailableGb && + this.fastDisk || ! other.fastDisk; + } @Override public int hashCode() { return name.hashCode(); } diff --git a/config-provisioning/src/main/resources/configdefinitions/flavors.def b/config-provisioning/src/main/resources/configdefinitions/flavors.def index a2dcfd3fd07..57affc2f104 100644 --- a/config-provisioning/src/main/resources/configdefinitions/flavors.def +++ b/config-provisioning/src/main/resources/configdefinitions/flavors.def @@ -25,7 +25,7 @@ flavor[].cost int default=0 # for some historical reason. These nodes are assigned to applications by exact match and ignoring cost. flavor[].stock bool default=true -# The type of node (e.g. bare metal, docker..). +# The type of node: BARE_METAL, VIRTUAL_MACHINE or DOCKER_CONTAINER flavor[].environment string default="undefined" # The minimum number of CPU cores available. diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java index d7dce9d3f3d..f0ce6104496 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java @@ -31,11 +31,6 @@ public class FileDBHandler implements FileDistribution { } @Override - public void limitSendingOfDeployedFilesTo(Collection<String> hostNames) { - manager.limitSendingOfDeployedFilesTo(hostNames); - } - - @Override public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) { manager.removeDeploymentsThatHaveDifferentApplicationId(targetHostnames); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java index 81c777df393..eb23d38e23e 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java @@ -14,7 +14,6 @@ import java.util.Set; public class MockFileDBHandler implements FileDistribution { public int sendDeployedFilesCalled = 0; public int reloadDeployFileDistributorCalled = 0; - public int limitSendingOfDeployedFilesToCalled = 0; public int removeDeploymentsThatHaveDifferentApplicationIdCalled = 0; @Override @@ -28,11 +27,6 @@ public class MockFileDBHandler implements FileDistribution { } @Override - public void limitSendingOfDeployedFilesTo(Collection<String> hostNames) { - limitSendingOfDeployedFilesToCalled++; - } - - @Override public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) { removeDeploymentsThatHaveDifferentApplicationIdCalled++; } 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 5dc529e3381..2069ae48d76 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 @@ -126,7 +126,6 @@ public class SessionPreparerTest extends TestWithCurator { new PrepareParams.Builder().dryRun(true).timeoutBudget(TimeoutBudgetTest.day()).build(), Optional.empty(), tenantPath, Instant.now()); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().sendDeployedFilesCalled, is(0)); - assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().limitSendingOfDeployedFilesToCalled, is(0)); assertThat(fileDistributionFactory.mockFileDistributionProvider.getMockFileDBHandler().reloadDeployFileDistributorCalled, is(0)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java index 5802edfad36..59dec6946f9 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantsTestCase.java @@ -146,25 +146,23 @@ public class TenantsTestCase extends TestWithCurator { @Test public void testTenantWatching() throws Exception { - TestComponentRegistry reg = new TestComponentRegistry.Builder().curator(curator).build(); - Tenants t = new Tenants(reg, Metrics.createTestMetrics()); TenantName newTenant = TenantName.from("newTenant"); List<TenantName> expectedTenants = Arrays.asList(TenantName.defaultName(), newTenant); try { - t.addTenant(newTenant); + tenants.addTenant(newTenant); // Poll for the watcher to pick up the tenant from zk, and add it int tries=0; while(true) { if (tries > 5000) fail("Didn't react on watch"); - if (t.getAllTenantNames().containsAll(expectedTenants)) { + if (tenants.getAllTenantNames().containsAll(expectedTenants)) { break; } tries++; Thread.sleep(10); } } finally { - assertTrue(t.getAllTenantNames().containsAll(expectedTenants)); - t.close(); + assertTrue(tenants.getAllTenantNames().containsAll(expectedTenants)); + tenants.close(); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 66bb076e099..5677eb2c08d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -315,7 +315,9 @@ public class ApplicationController { if (application.deploying().isPresent() && application.deploying().get() instanceof Change.ApplicationChange) application = application.withDeploying(Optional.of(Change.ApplicationChange.of(revision))); if ( ! canDeployDirectlyTo(zone, options) && jobType.isPresent()) { - // Update with (potentially) missing information about what we triggered + // Update with (potentially) missing information about what we triggered: + // * When someone else triggered the job, we need to store a stand-in triggering event. + // * When this is the system test job, we need to record the new revision, for future use. JobStatus.JobRun triggering = getOrCreateTriggering(application, version, jobType.get()); application = application.with(application.deploymentJobs() .withTriggering(jobType.get(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index b0424282ace..2b78bac48af 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -61,6 +61,7 @@ public class LockedApplication extends Application { deploying(), hasOutstandingChange()), lock); } + // TODO: runId is always -1. Don't pass it, and pretend it's there. public LockedApplication withJobTriggering(long runId, DeploymentJobs.JobType type, Optional<Change> change, String reason, Instant triggerTime, Controller controller) { return new LockedApplication(new Application(id(), deploymentSpec(), validationOverrides(), deployments(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java index 2d9bf45a39a..c532344f3a3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java @@ -122,7 +122,7 @@ public class DeploymentJobs { /** Returns whether change can be deployed to the given environment */ public boolean isDeployableTo(Environment environment, Optional<Change> change) { - if (environment == null || !change.isPresent()) { + if (environment == null || ! change.isPresent()) { return true; } if (environment == Environment.staging) { @@ -143,8 +143,8 @@ public class DeploymentJobs { /** Returns whether job has completed successfully */ public boolean isSuccessful(Change change, JobType jobType) { return Optional.ofNullable(jobStatus().get(jobType)) - .filter(JobStatus::isSuccess) - .filter(status -> status.lastCompletedFor(change)) + .flatMap(JobStatus::lastSuccess) + .filter(status -> status.lastCompletedWas(change)) .isPresent(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java index 7d1795aeaa3..6223b07d27a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobList.java @@ -45,6 +45,8 @@ public class JobList { // ----------------------------------- Accessors + // TODO: Add sorting based on various stuff, such as deployment order, time of last completion, etc.. + /** Returns the jobstatuses in this as an immutable list */ public List<JobStatus> asList() { return list; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java index 929bf186eea..9b8377fb087 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Controller; -import java.time.Duration; import java.time.Instant; import java.util.Objects; import java.util.Optional; @@ -75,7 +74,7 @@ public class JobStatus { } else if (! lastTriggered.isPresent()) { throw new IllegalStateException("Got notified about completion of " + this + - ", but that has not been triggered nor deployed"); + ", but that has neither been triggered nor deployed"); } else { @@ -110,7 +109,7 @@ public class JobStatus { if ( ! lastTriggered.isPresent()) return false; if (lastTriggered.get().at().isBefore(timeoutLimit)) return false; if ( ! lastCompleted.isPresent()) return true; - return lastTriggered.get().at().isAfter(lastCompleted.get().at()); + return ! lastTriggered.get().at().isBefore(lastCompleted.get().at()); } /** The error of the last completion, or empty if the last run succeeded */ @@ -131,18 +130,6 @@ public class JobStatus { /** Returns the run when this last succeeded, or empty if it has never succeeded */ public Optional<JobRun> lastSuccess() { return lastSuccess; } - /** Returns whether the job last completed for the given change */ - public boolean lastCompletedFor(Change change) { - if (change instanceof Change.ApplicationChange) { - Change.ApplicationChange applicationChange = (Change.ApplicationChange) change; - return lastCompleted().isPresent() && lastCompleted().get().revision().equals(applicationChange.revision()); - } else if (change instanceof Change.VersionChange) { - Change.VersionChange versionChange = (Change.VersionChange) change; - return lastCompleted().isPresent() && lastCompleted().get().version().equals(versionChange.version()); - } - throw new IllegalArgumentException("Unexpected change: " + change.getClass()); - } - @Override public String toString() { return "job status of " + type + "[ " + @@ -191,10 +178,12 @@ public class JobStatus { this.reason = reason; this.at = at; } - + + // TODO: Replace with proper ID, and make the build number part optional, or something -- it's not there for lastTriggered! /** Returns the id of this run of this job, or -1 if not known */ public long id() { return id; } + // TODO: Fix how this is set, and add an applicationChange() method as well, in the same vein. /** Returns whether this job run was a Vespa upgrade */ public boolean upgrade() { return upgrade; } @@ -210,6 +199,19 @@ public class JobStatus { /** Returns the time if this triggering or completion */ public Instant at() { return at; } + // TODO: Consider a version and revision for each JobStatus, to compare against a Target (instead of Change, which is, really, a Target). + /** Returns whether the job last completed for the given change */ + public boolean lastCompletedWas(Change change) { + if (change instanceof Change.ApplicationChange) { + Change.ApplicationChange applicationChange = (Change.ApplicationChange) change; + return revision().equals(applicationChange.revision()); + } else if (change instanceof Change.VersionChange) { + Change.VersionChange versionChange = (Change.VersionChange) change; + return version().equals(versionChange.version()); + } + throw new IllegalArgumentException("Unexpected change: " + change.getClass()); + } + @Override public int hashCode() { return Objects.hash(version, revision, upgrade, at); @@ -220,7 +222,7 @@ public class JobStatus { if (this == o) return true; if ( ! (o instanceof JobRun)) return false; JobRun jobRun = (JobRun) o; - return id == id && + return id == jobRun.id && Objects.equals(version, jobRun.version) && Objects.equals(revision, jobRun.revision) && upgrade == jobRun.upgrade && diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java index 9201c74e761..6e81f08a4a4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java @@ -89,7 +89,7 @@ public class DeploymentOrder { } /** Returns whether the given job causes an application change */ - public boolean givesApplicationChange(JobType job) { + public boolean givesNewRevision(JobType job) { return job == JobType.component; } @@ -120,6 +120,7 @@ public class DeploymentOrder { .collect(collectingAndThen(toList(), Collections::unmodifiableList)); } + // TODO: These sorts should throw when not all items to sort are listed. /** Returns deployments sorted according to declared zones */ public List<Deployment> sortBy(List<DeploymentSpec.DeclaredZone> zones, Collection<Deployment> deployments) { List<Zone> productionZones = zones.stream() @@ -185,7 +186,7 @@ public class DeploymentOrder { List<DeploymentSpec.Step> remainingSteps = application.deploymentSpec().steps() .subList(stepIndex + 1, application.deploymentSpec().steps().size()); for (DeploymentSpec.Step s : remainingSteps) { - if (!(s instanceof DeploymentSpec.Delay)) { + if (! (s instanceof DeploymentSpec.Delay)) { break; } totalDelay = totalDelay.plus(((DeploymentSpec.Delay) s).duration()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 3d2c2ed0319..473c279e28e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -19,6 +19,7 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; +import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; @@ -79,13 +80,13 @@ public class DeploymentTrigger { try (Lock lock = applications().lock(report.applicationId())) { LockedApplication application = applications().require(report.applicationId(), lock); application = application.withJobCompletion(report, clock.instant(), controller); - + // Handle successful starting and ending if (report.success()) { - if (order.givesApplicationChange(report.jobType())) { + if (order.givesNewRevision(report.jobType())) { if (acceptNewRevisionNow(application)) { // Set this as the change we are doing, unless we are already pushing a platform change - if ( ! ( application.deploying().isPresent() && + if ( ! ( application.deploying().isPresent() && (application.deploying().get() instanceof Change.VersionChange))) application = application.withDeploying(Optional.of(Change.ApplicationChange.unknown())); } @@ -93,7 +94,8 @@ public class DeploymentTrigger { applications().store(application.withOutstandingChange(true)); return; } - } + } + // TODO: Should rather fix deployingCompleted() (let it check that all declared zones have the change). else if (order.isLast(report.jobType(), application) && application.deployingCompleted()) { // change completed application = application.withDeploying(Optional.empty()); @@ -206,6 +208,7 @@ public class DeploymentTrigger { if ( ! application.deploying().isPresent()) return; // No ongoing change, no need to retry // Retry first failing job + // TODO: Use JobList, requires JobList to sort according to deploymentSpec. for (JobType jobType : order.jobsFrom(application.deploymentSpec())) { JobStatus jobStatus = application.deploymentJobs().jobStatus().get(jobType); if (isFailing(application.deploying().get(), jobStatus)) { @@ -293,9 +296,9 @@ public class DeploymentTrigger { /** Returns whether a job is failing for the current change in the given application */ private boolean isFailing(Change change, JobStatus status) { - return status != null && - !status.isSuccess() && - status.lastCompletedFor(change); + return status != null + && ! status.isSuccess() + && status.lastCompleted().get().lastCompletedWas(change); } private boolean isCapacityConstrained(JobType jobType) { @@ -365,6 +368,7 @@ public class DeploymentTrigger { * @param reason describes why the job is triggered * @return the application in the triggered state, which *must* be stored by the caller */ + // TODO: Improve explanation for first parameter. private LockedApplication trigger(JobType jobType, LockedApplication application, boolean first, String reason) { if (isRunningProductionJob(application)) return application; return triggerAllowParallel(jobType, application, first, false, reason); @@ -427,8 +431,10 @@ public class DeploymentTrigger { } private boolean isRunningProductionJob(Application application) { - return application.deploymentJobs().jobStatus().entrySet().stream() - .anyMatch(entry -> entry.getKey().isProduction() && entry.getValue().isRunning(jobTimeoutLimit())); + return JobList.from(application) + .production() + .running(jobTimeoutLimit()) + .anyMatch(); } /** @@ -468,9 +474,11 @@ public class DeploymentTrigger { private boolean acceptNewRevisionNow(LockedApplication application) { if ( ! application.deploying().isPresent()) return true; if ( application.deploying().get() instanceof Change.ApplicationChange) return true; // more changes are ok - + + // TODO: Don't these two below allow concurrent App and Version changes? if ( application.deploymentJobs().hasFailures()) return true; // allow changes to fix upgrade problems if ( application.isBlocked(clock.instant())) return true; // allow testing changes while upgrade blocked (debatable) + // Otherwise, the application is currently upgrading, without failures, and we should wait with the revision. return false; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index e4f4bd2ee96..5b87f9eaa86 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -26,8 +26,6 @@ import java.util.logging.Logger; */ public class Upgrader extends Maintainer { - private static final Duration upgradeTimeout = Duration.ofHours(12); - private static final Logger log = Logger.getLogger(Upgrader.class.getName()); private final CuratorDb curator; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 2745862f68a..04394f08e64 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -202,9 +202,7 @@ public class ApplicationSerializer { } private void toSlime(DeploymentJobs deploymentJobs, Cursor cursor) { - deploymentJobs.projectId() - .filter(id -> id > 0) // TODO: Discards invalid data. Remove filter after October 2017 - .ifPresent(projectId -> cursor.setLong(projectIdField, projectId)); + deploymentJobs.projectId().ifPresent(projectId -> cursor.setLong(projectIdField, projectId)); jobStatusToSlime(deploymentJobs.jobStatus().values(), cursor.setArray(jobStatusField)); deploymentJobs.issueId().ifPresent(jiraIssueId -> cursor.setString(issueIdField, jiraIssueId.value())); } @@ -347,8 +345,7 @@ public class ApplicationSerializer { } private DeploymentJobs deploymentJobsFromSlime(Inspector object) { - Optional<Long> projectId = optionalLong(object.field(projectIdField)) - .filter(id -> id > 0); // TODO: Discards invalid data. Remove filter after October 2017 + Optional<Long> projectId = optionalLong(object.field(projectIdField)); List<JobStatus> jobStatusList = jobStatusListFromSlime(object.field(jobStatusField)); Optional<IssueId> issueId = optionalString(object.field(issueIdField)).map(IssueId::from); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 554c496bc71..e5616f025ce 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -176,10 +176,6 @@ public class CuratorDb { VersionStatusSerializer serializer = new VersionStatusSerializer(); NestedTransaction transaction = new NestedTransaction(); try { - // TODO: Removes unused data. Remove after October 2017 - if (curator.getData(systemVersionPath()).isPresent()) { - curator.delete(systemVersionPath()); - } curator.set(versionStatusPath(), SlimeUtils.toJsonBytes(serializer.toSlime(status))); } catch (IOException e) { throw new UncheckedIOException("Failed to serialize version status", e); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java index 37677a5e393..ab240b9dea9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MemoryControllerDb.java @@ -24,9 +24,9 @@ import java.util.stream.Collectors; */ public class MemoryControllerDb extends ControllerDb { - private Map<TenantId, Tenant> tenants = new HashMap<>(); - private Map<String, Application> applications = new HashMap<>(); - private Map<RotationId, ApplicationId> rotationAssignments = new HashMap<>(); + private final Map<TenantId, Tenant> tenants = new HashMap<>(); + private final Map<String, Application> applications = new HashMap<>(); + private final Map<RotationId, ApplicationId> rotationAssignments = new HashMap<>(); @Override public void createTenant(Tenant tenant) { @@ -46,23 +46,14 @@ public class MemoryControllerDb extends ControllerDb { @Override public void deleteTenant(TenantId tenantId) { - Object removed = tenants.remove(tenantId); - if (removed == null) + if (tenants.remove(tenantId) == null) { throw new NotExistsException(tenantId); + } } @Override public Optional<Tenant> getTenant(TenantId tenantId) throws PersistenceException { - Optional<Tenant> tenant = Optional.ofNullable(tenants.get(tenantId)); - if(tenant.isPresent()) { - Tenant t_noquota = tenant.get(); - Tenant t_withquota = new Tenant( - t_noquota.getId(), t_noquota.getUserGroup(), t_noquota.getProperty(), - t_noquota.getAthensDomain(), t_noquota.getPropertyId()); - return Optional.of(t_withquota); - } else { - return tenant; - } + return Optional.ofNullable(tenants.get(tenantId)); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index 529ac292d8b..27b219cd892 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -35,6 +35,7 @@ import static java.util.Comparator.comparing; * * @author bratseth */ +@SuppressWarnings("unused") // Injected public class DeploymentApiHandler extends LoggingRequestHandler { private final Controller controller; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AccessControlHeaders.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AccessControlHeaders.java index aea59c16cd5..8a539720a21 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AccessControlHeaders.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AccessControlHeaders.java @@ -3,16 +3,15 @@ package com.yahoo.vespa.hosted.controller.restapi.filter; import com.google.common.collect.ImmutableMap; +import java.time.Duration; import java.util.Map; -import static java.util.concurrent.TimeUnit.DAYS; - /** * @author gv */ public interface AccessControlHeaders { - String CORS_PREFLIGHT_REQUEST_CACHE_TTL = Long.toString(DAYS.toSeconds(7)); + String CORS_PREFLIGHT_REQUEST_CACHE_TTL = Long.toString(Duration.ofDays(7).getSeconds()); String ALLOW_ORIGIN_HEADER = "Access-Control-Allow-Origin"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/securitycontext/CreateSecurityContextFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/securitycontext/CreateSecurityContextFilter.java index 850130ca970..6073307bafa 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/securitycontext/CreateSecurityContextFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/securitycontext/CreateSecurityContextFilter.java @@ -19,6 +19,7 @@ import java.security.Principal; */ @After("BouncerFilter") @Provides("SecurityContext") +@SuppressWarnings("unused") // Injected public class CreateSecurityContextFilter implements SecurityRequestFilter { @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 7875c1f4964..b607241c18c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -30,8 +30,8 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationRevision; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError; -import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport; import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType; +import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; @@ -104,8 +104,8 @@ public class ControllerTest { Optional<ApplicationRevision> revision = ((Change.ApplicationChange)tester.controller().applications().require(app1.id()).deploying().get()).revision(); assertTrue("Revision has been set during deployment", revision.isPresent()); assertStatus(JobStatus.initial(stagingTest) - .withTriggering(-1, version1, revision, false, "", tester.clock().instant()) - .withCompletion(-1, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); + .withTriggering(-1, version1, revision, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); // Causes first deployment job to be triggered assertStatus(JobStatus.initial(productionCorpUsEast1) @@ -118,9 +118,9 @@ public class ControllerTest { JobStatus expectedJobStatus = JobStatus.initial(productionCorpUsEast1) .withTriggering(-1, version1, revision, false, "", tester.clock().instant()) // Triggered first without revision info - .withCompletion(-1, Optional.of(JobError.unknown), tester.clock().instant(), tester.controller()) + .withCompletion(42, Optional.of(JobError.unknown), tester.clock().instant(), tester.controller()) .withTriggering(-1, version1, revision, false, "", tester.clock().instant()); // Re-triggering (due to failure) has revision info - + assertStatus(expectedJobStatus, app1.id(), tester.controller()); // Simulate restart @@ -133,21 +133,24 @@ public class ControllerTest { InstanceName.from("default")))); assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size()); - tester.clock().advance(Duration.ofSeconds(1)); + + tester.clock().advance(Duration.ofHours(1)); + + tester.notifyJobCompletion(productionCorpUsEast1, app1, false); // Need to complete the job, or new jobs won't start. // system and staging test job - succeeding tester.notifyJobCompletion(component, app1, true); tester.deployAndNotify(app1, applicationPackage, true, false, systemTest); assertStatus(JobStatus.initial(systemTest) - .withTriggering(-1, version1, revision, false, "", tester.clock().instant()) - .withCompletion(-1, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); + .withTriggering(-1, version1, revision, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller()); tester.deployAndNotify(app1, applicationPackage, true, stagingTest); // production job succeeding now tester.deployAndNotify(app1, applicationPackage, true, productionCorpUsEast1); expectedJobStatus = expectedJobStatus - .withTriggering(-1, version1, revision, false, "", tester.clock().instant()) - .withCompletion(-1, Optional.empty(), tester.clock().instant(), tester.controller()); + .withTriggering(-1, version1, revision, false, "", tester.clock().instant().minus(Duration.ofMillis(1))) + .withCompletion(42, Optional.empty(), tester.clock().instant(), tester.controller()); assertStatus(expectedJobStatus, app1.id(), tester.controller()); // causes triggering of next production job @@ -322,13 +325,13 @@ public class ControllerTest { tester.notifyJobCompletion(component, app, true); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", - initialFailure, firstFailing(app, tester).get().at()); + initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); // Failure again -- failingSince should remain the same tester.clock().advance(Duration.ofMillis(1000)); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at second consecutive failure", - initialFailure, firstFailing(app, tester).get().at()); + initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); // Success resets failingSince tester.clock().advance(Duration.ofMillis(1000)); @@ -346,13 +349,13 @@ public class ControllerTest { tester.notifyJobCompletion(component, app, true); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at initial failure", - initialFailure, firstFailing(app, tester).get().at()); + initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); // Failure again -- failingSince should remain the same tester.clock().advance(Duration.ofMillis(1000)); tester.deployAndNotify(app, applicationPackage, false, systemTest); assertEquals("Failure age is right at second consecutive failure", - initialFailure, firstFailing(app, tester).get().at()); + initialFailure.plus(Duration.ofMillis(2)), firstFailing(app, tester).get().at()); } private Optional<JobStatus.JobRun> firstFailing(Application application, DeploymentTester tester) { @@ -437,6 +440,7 @@ public class ControllerTest { // app1: 15 minutes pass, staging-test job is still failing due out of capacity, but is no longer re-queued by // out of capacity retry mechanism tester.clock().advance(Duration.ofMinutes(15)); + tester.notifyJobCompletion(stagingTest, app1, Optional.of(JobError.outOfCapacity)); // Clear the previous staging test tester.notifyJobCompletion(component, app1, true); tester.deployAndNotify(app1, applicationPackage, true, false, systemTest); tester.deploy(stagingTest, app1, applicationPackage); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 773ecf313cc..9f1a373f3dd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -69,6 +69,8 @@ public class DeploymentTester { public ApplicationController applications() { return tester.controller().applications(); } + // TODO: This thing simulates the wrong thing: the build system won't hold the jobs that are running, + // and so these should be consumed immediately upon triggering, and be "somewhere else" while running. public BuildSystem buildSystem() { return tester.controller().applications().deploymentTrigger().buildSystem(); } public DeploymentTrigger deploymentTrigger() { return tester.controller().applications().deploymentTrigger(); } @@ -170,7 +172,7 @@ public class DeploymentTester { jobs = jobs.stream().filter(job -> ! job.isProduction()).collect(Collectors.toList()); for (JobType job : jobs) { boolean failJob = failOnJob.map(j -> j.equals(job)).orElse(false); - deployAndNotify(application, applicationPackage, !failJob, false, job); + deployAndNotify(application, applicationPackage, ! failJob, false, job); if (failJob) { break; } @@ -191,6 +193,7 @@ public class DeploymentTester { } public void notifyJobCompletion(JobType jobType, Application application, Optional<DeploymentJobs.JobError> jobError) { + clock().advance(Duration.ofMillis(1)); applications().notifyJobCompletion(jobReport(application, jobType, jobError)); } @@ -227,7 +230,7 @@ public class DeploymentTester { deployAndNotify(application, applicationPackage, success, true, jobs); } - public void deployAndNotify(Application application, ApplicationPackage applicationPackage, boolean success, + public void deployAndNotify(Application application, ApplicationPackage applicationPackage, boolean success, boolean expectOnlyTheseJobs, JobType... jobs) { consumeJobs(application, expectOnlyTheseJobs, jobs); for (JobType job : jobs) { @@ -263,7 +266,7 @@ public class DeploymentTester { .count(); } - private static ApplicationPackage applicationPackage(String upgradePolicy) { + public static ApplicationPackage applicationPackage(String upgradePolicy) { return new ApplicationPackageBuilder() .upgradePolicy(upgradePolicy) .environment(Environment.prod) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index 5a753617761..3ca5e915ca9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -59,9 +59,10 @@ public class DeploymentTriggerTest { // system-test fails and is retried tester.deployAndNotify(app, applicationPackage, false, JobType.systemTest); assertEquals("Retried immediately", 1, tester.buildSystem().jobs().size()); - tester.buildSystem().takeJobsToRun(); - assertEquals("Job removed", 0, tester.buildSystem().jobs().size()); - tester.clock().advance(Duration.ofHours(4).plus(Duration.ofSeconds(1))); + tester.clock().advance(Duration.ofHours(1)); + tester.deployAndNotify(app, applicationPackage, false, JobType.systemTest); + tester.clock().advance(Duration.ofHours(1)); + assertEquals("Nothing scheduled", 0, tester.buildSystem().jobs().size()); tester.failureRedeployer().maintain(); // Causes retry of systemTests assertEquals("Scheduled retry", 1, tester.buildSystem().jobs().size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java index 87ef7ed07b1..d540db7c790 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/FailureRedeployerTest.java @@ -62,6 +62,7 @@ public class FailureRedeployerTest { // Another version is released, which cancels any pending upgrades to lower versions version = Version.fromString("5.2"); tester.updateVersionStatus(version); + tester.deployAndNotify(app, applicationPackage, true, DeploymentJobs.JobType.productionUsEast3); // Finish previous production job. tester.upgrader().maintain(); assertEquals("Application starts upgrading to new version", 1, tester.buildSystem().jobs().size()); assertEquals("Application has pending upgrade to " + version, version, tester.versionChange(app.id()).get().version()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 41f1d1914f3..6ef48bac7fa 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -95,12 +95,20 @@ public class UpgraderTest { assertEquals("New system version: Should upgrade Canaries", 2, tester.buildSystem().jobs().size()); tester.completeUpgradeWithError(canary0, version, "canary", DeploymentJobs.JobType.stagingTest); assertEquals("Other Canary was cancelled", 2, tester.buildSystem().jobs().size()); + // TODO: Cancelled? tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); assertEquals("Version broken, but Canaries should keep trying", 2, tester.buildSystem().jobs().size()); + // Exhaust canary retries. + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, canary1, false); + tester.clock().advance(Duration.ofHours(1)); + tester.deployAndNotify(canary0, DeploymentTester.applicationPackage("canary"), false, DeploymentJobs.JobType.stagingTest); + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, canary1, false); + //tester.deployAndNotify(canary1, DeploymentTester.applicationPackage("canary"), false, DeploymentJobs.JobType.stagingTest); + // --- A new version is released - which repairs the Canary app and fails a default version = Version.fromString("5.3"); tester.updateVersionStatus(version); @@ -128,15 +136,23 @@ public class UpgraderTest { tester.completeUpgrade(default2, version, "default"); tester.updateVersionStatus(version); - assertEquals("Not enough evidence to mark this neither broken nor high", + assertEquals("Not enough evidence to mark this as neither broken nor high", VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence()); - tester.upgrader().maintain(); + assertEquals("Upgrade with error should retry", 1, tester.buildSystem().jobs().size()); + // Finish previous run, with exhausted retry. + tester.clock().advance(Duration.ofHours(1)); + tester.notifyJobCompletion(DeploymentJobs.JobType.stagingTest, default0, false); + // --- Failing application is repaired by changing the application, causing confidence to move above 'high' threshold // Deploy application change tester.deployCompletely("default0"); + // Let maintainer trigger version change, and deploy it, too + tester.upgrader().maintain(); + tester.deployCompletely("default0"); + tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.high, tester.controller().versionStatus().systemVersion().get().confidence()); tester.upgrader().maintain(); @@ -190,9 +206,13 @@ public class UpgraderTest { tester.completeUpgradeWithError(default1, version55, "default", DeploymentJobs.JobType.stagingTest); tester.completeUpgradeWithError(default2, version55, "default", DeploymentJobs.JobType.stagingTest); tester.completeUpgradeWithError(default3, version55, "default", DeploymentJobs.JobType.productionUsWest1); - tester.completeUpgrade(default4, version55, "default"); tester.updateVersionStatus(version55); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); + + // Finish running job, without retry. + tester.clock().advance(Duration.ofHours(1)); + tester.notifyJobCompletion(DeploymentJobs.JobType.productionUsWest1, default3, false); + tester.upgrader().maintain(); assertEquals("Upgrade of defaults are scheduled on 5.4 instead, since 5.5 broken: " + "This is default3 since it failed upgrade on both 5.4 and 5.5", @@ -614,8 +634,16 @@ public class UpgraderTest { tester.completeUpgradeWithError(default3, version, "default", DeploymentJobs.JobType.systemTest); tester.updateVersionStatus(version); assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence()); + tester.upgrader().maintain(); + // Exhaust retries and finish runs + tester.clock().advance(Duration.ofHours(1)); + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default0, false); + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default1, false); + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default2, false); + tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default3, false); + // 5th app never reports back and has a dead job, but no ongoing change Application deadLocked = tester.applications().require(default4.id()); assertTrue("Jobs in progress", deadLocked.deploymentJobs().isRunning(tester.controller().applications().deploymentTrigger().jobTimeoutLimit())); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json index 32d34edd576..425b9d4512d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/testdata/pr-instance-with-dead-locked-job.json @@ -4,7 +4,7 @@ "validationOverrides": "<deployment version='1.0'/>", "deployments": [], "deploymentJobs": { - "projectId": 0, + "projectId": 42, "jobStatus": [ { "jobType": "system-test", diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index b38a38c3120..c408f2ee214 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -200,15 +200,6 @@ public class ApplicationSerializerTest { assertFalse(application.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.systemTest).lastCompleted().get().upgrade()); } - // TODO: Remove after October 2017 - @Test - public void testLegacySerializationWithZeroProjectId() { - Application original = applicationSerializer.fromSlime(applicationSlime(0, false)); - assertFalse(original.deploymentJobs().projectId().isPresent()); - Application serialized = applicationSerializer.fromSlime(applicationSerializer.toSlime(original)); - assertFalse(serialized.deploymentJobs().projectId().isPresent()); - } - private Slime applicationSlime(boolean error) { return applicationSlime(123, error); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java index 45a8972eafe..6c5120df515 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java @@ -88,6 +88,12 @@ public class ContainerControllerTester { } public void notifyJobCompletion(ApplicationId applicationId, long projectId, boolean success, DeploymentJobs.JobType job) { + try { + Thread.sleep(1); + } + catch (InterruptedException e) { + throw new RuntimeException(e); + } controller().applications().notifyJobCompletion(new DeploymentJobs.JobReport(applicationId, job, projectId, 42, success ? Optional.empty() : Optional.of(DeploymentJobs.JobError.unknown) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/responses/unexpected-completion.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/responses/unexpected-completion.json index e293d85b594..8ffd9511a96 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/responses/unexpected-completion.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/responses/unexpected-completion.json @@ -1,4 +1,4 @@ { "error-code": "BAD_REQUEST", - "message": "Got notified about completion of job status of productionUsEast3[ last triggered: (never), last completed: (never), first failing: (not failing), lastSuccess: (never)], but that has not been triggered nor deployed" + "message": "Got notified about completion of job status of productionUsEast3[ last triggered: (never), last completed: (never), first failing: (not failing), lastSuccess: (never)], but that has neither been triggered nor deployed" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index f67863370fc..4f97c078c9b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -20,6 +20,7 @@ import org.junit.Test; import java.net.URI; import java.net.URISyntaxException; +import java.time.Duration; import java.util.Collections; import java.util.List; @@ -161,6 +162,12 @@ public class VersionStatusTest { assertEquals("One canary failed: Broken", Confidence.broken, confidence(tester.controller(), version1)); + // Finish running jobs + tester.deployAndNotify(canary2, DeploymentTester.applicationPackage("canary"), false, systemTest); + tester.clock().advance(Duration.ofHours(1)); + tester.deployAndNotify(canary1, DeploymentTester.applicationPackage("canary"), false, productionUsWest1); + tester.deployAndNotify(canary2, DeploymentTester.applicationPackage("canary"), false, systemTest); + // New version is released Version version2 = new Version("5.2"); tester.upgradeSystem(version2); @@ -295,6 +302,7 @@ public class VersionStatusTest { Version versionWithUnknownTag = new Version("6.1.2"); Application app = tester.createAndDeploy("tenant1", "domain1","application1", Environment.test, 11); + tester.clock().advance(Duration.ofMillis(1)); applications.notifyJobCompletion(DeploymentTester.jobReport(app, component, true)); applications.notifyJobCompletion(DeploymentTester.jobReport(app, systemTest, true)); diff --git a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp index 954cce23205..f0825343f27 100644 --- a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp +++ b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp @@ -174,20 +174,6 @@ Java_com_yahoo_vespa_filedistribution_FileDistributionManager_setDeployedFilesIm JNIEXPORT void JNICALL -Java_com_yahoo_vespa_filedistribution_FileDistributionManager_limitSendingOfDeployedFilesToImpl( - JNIEnv *env, jobject self, jobjectArray hostNamesArg, jbyteArray appIdArg) -{ - try { - JNIArray<JNIString> hostNames(hostNamesArg, env); - JNIString appId(appIdArg, env); - - nativeFileDistributionManagerField.get(self, env)->_fileDBModel-> - cleanDeployedFilesToDownload(hostNames._value, appId._value); - } STANDARDCATCH() -} - -JNIEXPORT -void JNICALL Java_com_yahoo_vespa_filedistribution_FileDistributionManager_removeDeploymentsThatHaveDifferentApplicationIdImpl( JNIEnv *env, jobject self, jobjectArray hostNamesArg, jbyteArray appIdArg) { @@ -200,40 +186,3 @@ Java_com_yahoo_vespa_filedistribution_FileDistributionManager_removeDeploymentsT } STANDARDCATCH() } - - -JNIEXPORT -void JNICALL -Java_com_yahoo_vespa_filedistribution_FileDistributionManager_limitFilesTo( - JNIEnv *env, jobject self, jobjectArray fileReferencesArg) -{ - try { - JNIArray<JNIString> fileReferences(fileReferencesArg, env); - - nativeFileDistributionManagerField.get(self, env)->_fileDBModel-> - cleanFiles(fileReferences._value); - } STANDARDCATCH() -} - - -JNIEXPORT -jbyteArray JNICALL -Java_com_yahoo_vespa_filedistribution_FileDistributionManager_getProgressImpl( - JNIEnv *env, jobject self, jbyteArray fileReferenceArg, jobjectArray hostNamesArg) -{ - try { - JNIString fileReference(fileReferenceArg, env); - JNIArray<JNIString> hostNames(hostNamesArg, env); - - const FileDBModel::Progress progress = - nativeFileDistributionManagerField.get(self, env)->_fileDBModel-> - getProgress(fileReference._value, hostNames._value); - - jbyteArray result = env->NewByteArray(progress.size()); - if (!result) - return 0; //exception thrown when returning - - env->SetByteArrayRegion(result, 0, progress.size(), &*progress.begin()); - return result; - } STANDARDCATCH(return 0) -} diff --git a/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java b/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java index 0a803269ff6..f0c463abe70 100644 --- a/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java +++ b/filedistributionmanager/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionManager.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.filedistribution; import java.io.File; import java.io.IOException; import java.util.Collection; -import java.util.List; import java.util.concurrent.locks.Lock; /** @@ -30,13 +29,8 @@ public class FileDistributionManager { private native void setDeployedFilesImpl(byte[] host, byte[] appId, byte[][] fileReferences); - private native void limitSendingOfDeployedFilesToImpl(byte[][] hostNames, byte[] appId); - private native void limitFilesToImpl(byte[][] fileReferences); private native void removeDeploymentsThatHaveDifferentApplicationIdImpl(byte[][] asByteArrays, byte[] bytes); - private native byte[] getProgressImpl(byte[] fileReference, - byte[][] hostNames); - private byte[][] getAsByteArrays(Collection<String> strings) { byte[][] byteArrays = new byte[strings.size()][]; int i = 0; @@ -121,17 +115,6 @@ public class FileDistributionManager { } } - public void limitSendingOfDeployedFilesTo(Collection<String> hostNames) { - try (LockGuard guard = new LockGuard(lock)) { - limitSendingOfDeployedFilesToImpl(getAsByteArrays(hostNames), appId.getBytes()); - } - } - - public byte[] getProgress(String fileReference, - List<String> hostNamesSortedAscending) { - return getProgressImpl(fileReference.getBytes(), getAsByteArrays(hostNamesSortedAscending)); - } - public native void shutdown(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index bc50de719aa..5ffa99e0de1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -97,10 +97,10 @@ class Activator { List<Node> updated = new ArrayList<>(); for (Node node : nodes) { HostSpec hostSpec = getHost(node.hostname(), hosts); - node = hostSpec.membership().get().retired() - ? node.retire(clock.instant()) - : node.unretire(); + node = hostSpec.membership().get().retired() ? node.retire(clock.instant()) : node.unretire(); node = node.with(node.allocation().get().with(hostSpec.membership().get())); + if (hostSpec.flavor().isPresent()) // Docker nodes may change flavor + node = node.with(hostSpec.flavor().get()); updated.add(node); } return updated; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 0bf48b805aa..fae178adb87 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -49,7 +49,7 @@ class GroupPreparer { List<Node> surplusActiveNodes, MutableInteger highestIndex, int nofSpares, BiConsumer<List<Node>, String> debugRecorder) { try (Mutex lock = nodeRepository.lock(application)) { - // Use new, ready nodes. Lock ready pool to ensure that nodes are not grabbed by others. + // Lock ready pool to ensure that ready nodes are not simultaneously grabbed by others try (Mutex readyLock = nodeRepository.lockUnallocated()) { // Create a prioritized set of nodes diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index b9f682bc79f..d9d7b4a5d12 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -90,15 +90,15 @@ class NodeAllocation { // conditions on which we want to retire nodes that were allocated previously if ( offeredNodeHasParentHostnameAlreadyAccepted(this.nodes, offered)) wantToRetireNode = true; - if ( !hasCompatibleFlavor(offered)) wantToRetireNode = true; + if ( ! hasCompatibleFlavor(offered)) wantToRetireNode = true; if ( offered.flavor().isRetired()) wantToRetireNode = true; if ( offered.status().wantToRetire()) wantToRetireNode = true; - if ((!saturated() && hasCompatibleFlavor(offered)) || acceptToRetire(offered) ) { + if (( ! saturated() && hasCompatibleFlavor(offered)) || acceptToRetire(offered) ) { accepted.add(acceptNode(offeredPriority, wantToRetireNode)); } } - else if (! saturated() && hasCompatibleFlavor(offered)) { + else if ( ! saturated() && hasCompatibleFlavor(offered)) { if ( offeredNodeHasParentHostnameAlreadyAccepted(this.nodes, offered)) { ++rejectedWithClashingParentHost; continue; @@ -233,6 +233,12 @@ class NodeAllocation { } } } + + // Update flavor of allocated docker nodes as we can change it in place + for (PrioritizableNode node : nodes) { + if (node.node.allocation().isPresent()) + node.node = requestedNodes.assignRequestedFlavor(node.node); + } return nodes.stream().map(n -> n.node).collect(Collectors.toList()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index 10d562eff98..4c6f1b022a4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Flavor; +import com.yahoo.vespa.hosted.provision.Node; import java.util.Objects; @@ -39,6 +40,13 @@ public interface NodeSpec { /** Returns a specification of a fraction of all the nodes of this. It is assumed the argument is a valid divisor. */ NodeSpec fraction(int divisor); + /** + * Assigns the flavor requested by this to the given node and returns it, + * if one is requested and it is allowed to change. + * Otherwise, the node is returned unchanged. + */ + Node assignRequestedFlavor(Node node); + static NodeSpec from(int nodeCount, Flavor flavor) { return new CountNodeSpec(nodeCount, flavor); } @@ -51,31 +59,36 @@ public interface NodeSpec { class CountNodeSpec implements NodeSpec { private final int count; - private final Flavor flavor; + private final Flavor requestedFlavor; public CountNodeSpec(int count, Flavor flavor) { Objects.requireNonNull(flavor, "A flavor must be specified"); this.count = count; - this.flavor = flavor; + this.requestedFlavor = flavor; } + // TODO: Remove usage of this public Flavor getFlavor() { - return flavor; + return requestedFlavor; } + // TODO: Remove usage of this public int getCount() { return count; } @Override public NodeType type() { return NodeType.tenant; } @Override - public boolean isCompatible(Flavor flavor) { return flavor.satisfies(this.flavor); } + public boolean isCompatible(Flavor flavor) { + if (flavor.satisfies(requestedFlavor)) return true; + return requestedFlavorCanBeAchievedByResizing(flavor); + } @Override - public boolean matchesExactly(Flavor flavor) { return flavor.equals(this.flavor); } + public boolean matchesExactly(Flavor flavor) { return flavor.equals(this.requestedFlavor); } @Override - public boolean specifiesNonStockFlavor() { return ! flavor.isStock(); } + public boolean specifiesNonStockFlavor() { return ! requestedFlavor.isStock(); } @Override public boolean fulfilledBy(int count) { return count >= this.count; } @@ -87,10 +100,23 @@ public interface NodeSpec { public int surplusGiven(int count) { return count - this.count; } @Override - public NodeSpec fraction(int divisor) { return new CountNodeSpec(count/divisor, flavor); } + public NodeSpec fraction(int divisor) { return new CountNodeSpec(count/divisor, requestedFlavor); } + + @Override + public Node assignRequestedFlavor(Node node) { + // Docker nodes can change flavor in place + if (requestedFlavorCanBeAchievedByResizing(node.flavor())) + return node.with(requestedFlavor); + return node; + } @Override - public String toString() { return "request for " + count + " nodes of " + flavor; } + public String toString() { return "request for " + count + " nodes of " + requestedFlavor; } + + /** Docker nodes can be downsized in place */ + private boolean requestedFlavorCanBeAchievedByResizing(Flavor flavor) { + return flavor.isDocker() && requestedFlavor.isDocker() && flavor.isLargerThan(requestedFlavor); + } } @@ -128,6 +154,9 @@ public interface NodeSpec { public NodeSpec fraction(int divisor) { return this; } @Override + public Node assignRequestedFlavor(Node node) { return node; } + + @Override public String toString() { return "request for all nodes of type '" + type + "'"; } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java index 134808b7114..c38d6f6bd40 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AclProvisioningTest.java @@ -178,8 +178,8 @@ public class AclProvisioningTest { // Populate repo List<Node> dockerHostNodes = tester.makeReadyNodes(2, "default", NodeType.host); Node dockerHostNodeUnderTest = dockerHostNodes.get(0); - List<Node> dockerNodes = tester.makeReadyDockerNodes(5, "docker1", - dockerHostNodeUnderTest.hostname()); + List<Node> dockerNodes = tester.makeReadyDockerNodes(5, "dockerSmall", + dockerHostNodeUnderTest.hostname()); List<NodeAcl> acls = tester.nodeRepository().getNodeAcls(dockerHostNodeUnderTest, true); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java index d17aa8ef741..a6760ef37ce 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java @@ -24,7 +24,7 @@ import static org.junit.Assert.assertEquals; */ public class DockerProvisioningTest { - private static final String dockerFlavor = "docker1"; + private static final String dockerFlavor = "dockerSmall"; @Test public void docker_application_deployment() { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index dea65f432ec..afdce0d25cc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -232,6 +232,29 @@ public class ProvisioningTest { } @Test + public void application_deployment_with_inplace_downsize() { + ProvisioningTester tester = new ProvisioningTester(new Zone(Environment.prod, RegionName.from("us-east"))); + + ApplicationId application1 = tester.makeApplicationId(); + + tester.makeReadyNodes(14, "dockerLarge"); + + // deploy + SystemState state1 = prepare(application1, 2, 2, 4, 4, "dockerLarge", tester); + tester.activate(application1, state1.allHosts); + + // redeploy with smaller docker flavor - causes in-place flavor change + SystemState state2 = prepare(application1, 2, 2, 4, 4, "dockerSmall", tester); + tester.activate(application1, state2.allHosts); + + assertEquals(12, tester.getNodes(application1, Node.State.active).asList().size()); + for (Node node : tester.getNodes(application1, Node.State.active).asList()) + assertEquals("Node changed flavor in place", "dockerSmall", node.flavor().name()); + assertEquals("No nodes are retired", + 0, tester.getNodes(application1, Node.State.active).retired().size()); + } + + @Test public void application_deployment_multiple_flavors_default_per_type() { ConfigserverConfig.Builder config = new ConfigserverConfig.Builder(); config.environment("prod"); @@ -506,7 +529,7 @@ public class ProvisioningTest { fail("Expected exception"); } catch (IllegalArgumentException e) { - assertEquals("Unknown flavor 'nonexisting'. Flavors are [default, docker1, large, old-large1, old-large2, small, v-4-8-100]", e.getMessage()); + assertEquals("Unknown flavor 'nonexisting'. Flavors are [default, dockerLarge, dockerSmall, large, old-large1, old-large2, small, v-4-8-100]", e.getMessage()); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index d9ce04513a5..3c932f492db 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -94,7 +94,8 @@ public class ProvisioningTester implements AutoCloseable { FlavorConfigBuilder b = new FlavorConfigBuilder(); b.addFlavor("default", 2., 4., 100, Flavor.Type.BARE_METAL).cost(3); b.addFlavor("small", 1., 2., 50, Flavor.Type.BARE_METAL).cost(2); - b.addFlavor("docker1", 1., 1., 10, Flavor.Type.DOCKER_CONTAINER).cost(1); + b.addFlavor("dockerSmall", 1., 1., 10, Flavor.Type.DOCKER_CONTAINER).cost(1); + b.addFlavor("dockerLarge", 2., 1., 20, Flavor.Type.DOCKER_CONTAINER).cost(3); b.addFlavor("v-4-8-100", 4., 8., 100, Flavor.Type.VIRTUAL_MACHINE).cost(4); b.addFlavor("old-large1", 2., 4., 100, Flavor.Type.BARE_METAL).cost(6); b.addFlavor("old-large2", 2., 5., 100, Flavor.Type.BARE_METAL).cost(14); diff --git a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp index 41c8974470f..1186cc23c5f 100644 --- a/storage/src/vespa/storage/distributor/pendingclusterstate.cpp +++ b/storage/src/vespa/storage/distributor/pendingclusterstate.cpp @@ -252,8 +252,10 @@ PendingClusterState::distributorChanged( const lib::State& nw(newState.getNodeState(node).getState()); if (nodeWasUpButNowIsDown(old, nw)) { - return (nodeInSameGroupAsSelf(i) - || nodeNeedsOwnershipTransferFromGroupDown(i, newState)); + if (nodeInSameGroupAsSelf(i) || + nodeNeedsOwnershipTransferFromGroupDown(i, newState)) { + return true; + } } } |