diff options
Diffstat (limited to 'controller-server/src/main/java/com/yahoo')
35 files changed, 555 insertions, 539 deletions
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 047c059e9a6..92c3198175a 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 @@ -59,7 +59,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageValid import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificates; import com.yahoo.vespa.hosted.controller.concurrent.Once; -import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.deployment.JobStatus; import com.yahoo.vespa.hosted.controller.deployment.Run; @@ -139,6 +138,7 @@ public class ApplicationController { private final StringFlag dockerImageRepoFlag; private final ListFlag<String> incompatibleVersions; private final BillingController billingController; + private final ListFlag<String> cloudAccountsFlag; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, Clock clock, FlagSource flagSource, BillingController billingController) { @@ -153,6 +153,7 @@ public class ApplicationController { applicationStore = controller.serviceRegistry().applicationStore(); dockerImageRepoFlag = PermanentFlags.DOCKER_IMAGE_REPO.bindTo(flagSource); incompatibleVersions = PermanentFlags.INCOMPATIBLE_VERSIONS.bindTo(flagSource); + cloudAccountsFlag = PermanentFlags.CLOUD_ACCOUNTS.bindTo(flagSource); deploymentTrigger = new DeploymentTrigger(controller, clock); applicationPackageValidator = new ApplicationPackageValidator(controller); endpointCertificates = new EndpointCertificates(controller, @@ -178,6 +179,11 @@ public class ApplicationController { }); } + /** Validate the given application package */ + public void validatePackage(ApplicationPackage applicationPackage, Application application) { + applicationPackageValidator.validate(application, applicationPackage, clock.instant()); + } + /** Returns the application with the given id, or null if it is not present */ public Optional<Application> getApplication(TenantAndApplicationId id) { return curator.readApplication(id); @@ -344,10 +350,10 @@ public class ApplicationController { // Target platforms are all versions not older than the oldest installed platform, unless forcing a major version change. // Only platforms not older than the system version, and with appropriate confidence, are considered targets. Predicate<Version> isTargetPlatform = wantedMajor.isEmpty() && oldestInstalledPlatform.isEmpty() - ? __ -> true - : wantedMajor.isEmpty() || wantedMajor.getAsInt() == oldestInstalledPlatform.get().getMajor() - ? version -> ! version.isBefore(oldestInstalledPlatform.get()) - : version -> wantedMajor.getAsInt() == version.getMajor(); + ? __ -> true // No preferences for version: any platform version is ok. + : wantedMajor.isEmpty() || (oldestInstalledPlatform.isPresent() && wantedMajor.getAsInt() == oldestInstalledPlatform.get().getMajor()) + ? version -> ! version.isBefore(oldestInstalledPlatform.get()) // Major empty, or on same as oldest: ensure not a platform downgrade. + : version -> wantedMajor.getAsInt() == version.getMajor(); // Major specified, and not on same as oldest (possibly empty): any on that major. Set<Version> platformVersions = versionStatus.deployableVersions().stream() .filter(version -> version.confidence().equalOrHigherThan(targetConfidence)) .map(VespaVersion::versionNumber) @@ -538,7 +544,7 @@ public class ApplicationController { /** Stores the deployment spec and validation overrides from the application package, and runs cleanup. */ public void storeWithUpdatedConfig(LockedApplication application, ApplicationPackage applicationPackage) { - applicationPackageValidator.validate(application.get(), applicationPackage, clock.instant()); + validatePackage(applicationPackage, application.get()); application = application.with(applicationPackage.deploymentSpec()); application = application.with(applicationPackage.validationOverrides()); @@ -630,9 +636,7 @@ public class ApplicationController { List<X509Certificate> operatorCertificates = controller.supportAccess().activeGrantsFor(deployment).stream() .map(SupportAccessGrant::certificate) .collect(toList()); - Optional<CloudAccount> cloudAccount = applicationPackage.deploymentSpec() - .instance(application.instance()) - .flatMap(spec -> spec.cloudAccount(zone.environment(), zone.region())); + Optional<CloudAccount> cloudAccount = decideCloudAccountOf(deployment, applicationPackage.deploymentSpec()); ConfigServer.PreparedApplication preparedApplication = configServer.deploy(new DeploymentData(application, zone, applicationPackage.zippedContent(), platform, endpoints, endpointCertificateMetadata, dockerImageRepo, domain, @@ -649,6 +653,30 @@ public class ApplicationController { } } + private Optional<CloudAccount> decideCloudAccountOf(DeploymentId deployment, DeploymentSpec spec) { + ZoneId zoneId = deployment.zoneId(); + Optional<CloudAccount> requestedAccount = spec.instance(deployment.applicationId().instance()) + .flatMap(instanceSpec -> instanceSpec.cloudAccount(zoneId.environment(), + Optional.of(zoneId.region()))); + if (requestedAccount.isEmpty()) { + return Optional.empty(); + } + TenantName tenant = deployment.applicationId().tenant(); + Set<CloudAccount> tenantAccounts = cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value()) + .value().stream() + .map(CloudAccount::new) + .collect(Collectors.toSet()); + if (!tenantAccounts.contains(requestedAccount.get())) { + throw new IllegalArgumentException("Requested cloud account '" + requestedAccount.get().value() + + "' is not valid for tenant '" + tenant + "'"); + } + if (!controller.zoneRegistry().hasZone(zoneId, requestedAccount.get())) { + throw new IllegalArgumentException("Zone " + zoneId + " is not configured in requested cloud account '" + + requestedAccount.get().value() + "'"); + } + return requestedAccount; + } + private LockedApplication withoutDeletedDeployments(LockedApplication application, InstanceName instance) { DeploymentSpec deploymentSpec = application.get().deploymentSpec(); List<ZoneId> deploymentsToRemove = application.get().require(instance).productionDeployments().values().stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java index 626937dc6ac..2441da19b90 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -11,6 +11,9 @@ import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatus; import com.yahoo.vespa.hosted.controller.deployment.DeploymentStatusList; +import com.yahoo.vespa.hosted.controller.versions.VersionStatus; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Instant; import java.util.Collection; @@ -18,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalInt; import java.util.function.Function; import static java.util.Comparator.comparing; @@ -52,17 +56,25 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL } /** - * Returns the subset of instances whose application have a deployment on the given major, or specify it in deployment spec. + * Returns the subset of instances whose application have a deployment on the given major, + * or specify it in deployment spec, + * or which are on a {@link VespaVersion.Confidence#legacy} platform, and do not specify that in deployment spec. * * @param targetMajorVersion the target major version which applications returned allows upgrading to */ - public InstanceList allowingMajorVersion(int targetMajorVersion) { + public InstanceList allowingMajorVersion(int targetMajorVersion, VersionStatus versions) { return matching(id -> { Application application = application(id); - return application.deploymentSpec().majorVersion().map(allowed -> targetMajorVersion <= allowed) - .orElseGet(() -> application.productionDeployments().values().stream() - .flatMap(List::stream) - .anyMatch(deployment -> targetMajorVersion <= deployment.version().getMajor())); + Optional<Integer> majorVersion = application.deploymentSpec().majorVersion(); + if (majorVersion.isPresent()) + return majorVersion.get() >= targetMajorVersion; + + for (List<Deployment> deployments : application.productionDeployments().values()) + for (Deployment deployment : deployments) { + if (deployment.version().getMajor() >= targetMajorVersion) return true; + if (versions.version(deployment.version()).confidence() == Confidence.legacy) return true; + } + return false; }); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java index 5a131ba4a29..8e8a4e24970 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java @@ -7,17 +7,12 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.Endpoint; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; -import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.ListFlag; -import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.EndpointId; @@ -31,7 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; /** @@ -42,11 +36,9 @@ import java.util.stream.Collectors; public class ApplicationPackageValidator { private final Controller controller; - private final ListFlag<String> cloudAccountsFlag; public ApplicationPackageValidator(Controller controller) { this.controller = Objects.requireNonNull(controller, "controller must be non-null"); - this.cloudAccountsFlag = PermanentFlags.CLOUD_ACCOUNTS.bindTo(controller.flagSource()); } /** @@ -55,7 +47,6 @@ public class ApplicationPackageValidator { * @throws IllegalArgumentException if any validations fail */ public void validate(Application application, ApplicationPackage applicationPackage, Instant instant) { - validateCloudAccounts(application, applicationPackage.deploymentSpec()); validateSteps(applicationPackage.deploymentSpec()); validateEndpointRegions(applicationPackage.deploymentSpec()); validateEndpointChange(application, applicationPackage, instant); @@ -90,20 +81,10 @@ public class ApplicationPackageValidator { for (var spec : deploymentSpec.instances()) { for (var zone : spec.zones()) { Environment environment = zone.environment(); - if (environment.isManuallyDeployed()) - throw new IllegalArgumentException("region must be one with automated deployments, but got: " + environment); - - if (environment == Environment.prod) { - RegionName region = zone.region().orElseThrow(); - if (!controller.zoneRegistry().hasZone(ZoneId.from(environment, region))) { - throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); - } - Optional<CloudAccount> cloudAccount = spec.cloudAccount(environment, region); - if (cloudAccount.isPresent() && !controller.zoneRegistry().hasZone(ZoneId.from(environment, region), cloudAccount.get())) { - throw new IllegalArgumentException("Zone " + zone + " in deployment spec is not configured for " + - "use in cloud account '" + cloudAccount.get().value() + - "', in this system"); - } + if (zone.region().isEmpty()) continue; + ZoneId zoneId = ZoneId.from(environment, zone.region().get()); + if (!controller.zoneRegistry().hasZone(zoneId)) { + throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!"); } } } @@ -185,25 +166,6 @@ public class ApplicationPackageValidator { ". " + ValidationOverrides.toAllowMessage(validationId)); } - /** Verify that declared cloud accounts are allowed to be used by the tenant */ - private void validateCloudAccounts(Application application, DeploymentSpec deploymentSpec) { - TenantName tenant = application.id().tenant(); - Set<CloudAccount> validAccounts = cloudAccountsFlag.with(FetchVector.Dimension.TENANT_ID, tenant.value()) - .value().stream() - .map(CloudAccount::new) - .collect(Collectors.toSet()); - for (var spec : deploymentSpec.instances()) { - for (var zone : spec.zones()) { - if (!zone.environment().isProduction()) continue; - Optional<CloudAccount> cloudAccount = spec.cloudAccount(zone.environment(), zone.region().get()); - if (cloudAccount.isEmpty()) continue; - if (validAccounts.contains(cloudAccount.get())) continue; - throw new IllegalArgumentException("Cloud account '" + cloudAccount.get().value() + - "' is not valid for tenant '" + tenant + "'"); - } - } - } - /** Returns whether newEndpoints contains all destinations in endpoints */ private static boolean containsAllDestinationsOf(List<Endpoint> endpoints, List<Endpoint> newEndpoints) { var containsAllRegions = true; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 50492077e20..3a6a8e67a75 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -8,6 +8,7 @@ import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.DeploymentSpec.DeclaredTest; import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; +import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.application.api.DeploymentSpec.UpgradeRollout; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.CloudName; @@ -27,12 +28,12 @@ import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -170,7 +171,40 @@ public class DeploymentStatus { } /** Returns change potentially with a compatibility platform added, if required for the change to roll out to the given instance. */ - public Change withCompatibilityPlatform(Change change, InstanceName instance) { + public Change withPermittedPlatform(Change change, InstanceName instance, boolean allowOudatedPlatform) { + Change augmented = withCompatibilityPlatform(change, instance); + if (allowOudatedPlatform) + return augmented; + + boolean alreadyDeployedOnPlatform = augmented.platform().map(platform -> allJobs.production().asList().stream() + .anyMatch(job -> job.runs().values().stream() + .anyMatch(run -> run.versions().targetPlatform().getMajor() == platform.getMajor()))) + .orElse(false); + + // Verify target platform is either current, or was previously deployed for this app. + if (augmented.platform().isPresent() && ! versionStatus.isOnCurrentMajor(augmented.platform().get()) && ! alreadyDeployedOnPlatform) + throw new IllegalArgumentException("platform version " + augmented.platform().get() + " is not on a current major version in this system"); + + Version latestHighConfidencePlatform = null; + for (VespaVersion platform : versionStatus.deployableVersions()) + if (platform.confidence().equalOrHigherThan(Confidence.high)) + latestHighConfidencePlatform = platform.versionNumber(); + + // Verify package is compatible with the current major, or newer, or that there already are deployments on a compatible, outdated platform. + if (latestHighConfidencePlatform != null) { + Version target = latestHighConfidencePlatform; + augmented.revision().flatMap(revision -> application.revisions().get(revision).compileVersion()) + .filter(target::isAfter) + .ifPresent(compiled -> { + if (versionCompatibility.apply(instance).refuse(target, compiled) && ! alreadyDeployedOnPlatform) + throw new IllegalArgumentException("compile version " + compiled + " is incompatible with the current major version of this system"); + }); + } + + return augmented; + } + + private Change withCompatibilityPlatform(Change change, InstanceName instance) { if (change.revision().isEmpty()) return change; @@ -178,18 +212,18 @@ public class DeploymentStatus { .map(application.revisions()::get) .flatMap(ApplicationVersion::compileVersion); - // If the revision requires a certain platform for compatibility, add that here. + // If the revision requires a certain platform for compatibility, add that here, unless we're already deploying a compatible platform. VersionCompatibility compatibility = versionCompatibility.apply(instance); Predicate<Version> compatibleWithCompileVersion = version -> compileVersion.map(compiled -> compatibility.accept(version, compiled)).orElse(true); - if ( application.productionDeployments().isEmpty() // TODO: replace with adding this for test jobs when needed + if (change.platform().map(compatibleWithCompileVersion::test).orElse(false)) + return change; + + if ( application.productionDeployments().isEmpty() || application.productionDeployments().getOrDefault(instance, List.of()).stream() - .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) { - return targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy()) - .stream() // Pick the latest platform with appropriate confidence, which is compatible with the compile version. - .filter(compatibleWithCompileVersion) - .findFirst() - .map(platform -> change.withoutPin().with(platform)) - .orElse(change); + .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) { + for (Version platform : targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy())) + if (compatibleWithCompileVersion.test(platform)) + return change.withoutPin().with(platform); } return change; } @@ -255,39 +289,82 @@ public class DeploymentStatus { if (change == null || ! change.hasTargets()) return; - Collection<Optional<JobId>> firstProductionJobsWithDeployment = jobSteps.keySet().stream() - .filter(jobId -> jobId.type().isProduction() && jobId.type().isDeployment()) - .filter(jobId -> deploymentFor(jobId).isPresent()) - .collect(groupingBy(jobId -> findCloud(jobId.type()), - Collectors.reducing((o, n) -> o))) // Take the first. - .values(); - if (firstProductionJobsWithDeployment.isEmpty()) - firstProductionJobsWithDeployment = List.of(Optional.empty()); - - for (Optional<JobId> firstProductionJobWithDeploymentInCloud : firstProductionJobsWithDeployment) { + Map<CloudName, Optional<JobId>> firstProductionJobsWithDeployment = firstDependentProductionJobsWithDeployment(job.application().instance()); + firstProductionJobsWithDeployment.forEach((cloud, firstProductionJobWithDeploymentInCloud) -> { Versions versions = Versions.from(change, application, firstProductionJobWithDeploymentInCloud.flatMap(this::deploymentFor), fallbackPlatform(change, job)); if (step.completedAt(change, firstProductionJobWithDeploymentInCloud).isEmpty()) { - CloudName cloud = firstProductionJobWithDeploymentInCloud.map(JobId::type).map(this::findCloud).orElse(zones.systemZone().getCloudName()); JobType typeWithZone = job.type().isSystemTest() ? JobType.systemTest(zones, cloud) : JobType.stagingTest(zones, cloud); jobs.merge(job, List.of(new Job(typeWithZone, versions, step.readyAt(change), change)), DeploymentStatus::union); } - } + }); }); return Collections.unmodifiableMap(jobs); } + /** + * Returns the clouds, and their first production deployments, that depend on this instance; or, + * if no such deployments exist, all clouds the application deploy to, and their first production deployments; or + * if no clouds are deployed to at all, the system default cloud. + */ + Map<CloudName, Optional<JobId>> firstDependentProductionJobsWithDeployment(InstanceName testInstance) { + // Find instances' dependencies on each other: these are topologically ordered, so a simple traversal does it. + Map<InstanceName, Set<InstanceName>> dependencies = new HashMap<>(); + instanceSteps().forEach((name, step) -> { + dependencies.put(name, new HashSet<>()); + dependencies.get(name).add(name); + for (StepStatus dependency : step.dependencies()) { + dependencies.get(name).add(dependency.instance()); + dependencies.get(name).addAll(dependencies.get(dependency.instance)); + } + }); + + Map<CloudName, Optional<JobId>> independentJobsPerCloud = new HashMap<>(); + Map<CloudName, Optional<JobId>> jobsPerCloud = new HashMap<>(); + jobSteps.forEach((job, step) -> { + if ( ! job.type().isProduction() || ! job.type().isDeployment()) + return; + + (dependencies.get(step.instance()).contains(testInstance) ? jobsPerCloud + : independentJobsPerCloud) + .merge(findCloud(job.type()), + Optional.of(job), + (o, n) -> o.filter(v -> deploymentFor(v).isPresent()) // Keep first if its deployment is present. + .or(() -> n.filter(v -> deploymentFor(v).isPresent())) // Use next if only its deployment is present. + .or(() -> o)); // Keep first if none have deployments. + }); + + if (jobsPerCloud.isEmpty()) + jobsPerCloud.putAll(independentJobsPerCloud); + + if (jobsPerCloud.isEmpty()) + jobsPerCloud.put(zones.systemZone().getCloudName(), Optional.empty()); + + return jobsPerCloud; + } + + /** Fall back to the newest, deployable platform, which is compatible with what we want to deploy. */ public Version fallbackPlatform(Change change, JobId job) { + InstanceName instance = job.application().instance(); Optional<Version> compileVersion = change.revision().map(application.revisions()::get).flatMap(ApplicationVersion::compileVersion); - if (compileVersion.isEmpty()) - return systemVersion; - - for (VespaVersion version : reversed(versionStatus.deployableVersions())) - if (versionCompatibility.apply(job.application().instance()).accept(version.versionNumber(), compileVersion.get())) - return version.versionNumber(); + List<Version> targets = targetsForPolicy(versionStatus, + systemVersion, + application.deploymentSpec().instance(instance) + .map(DeploymentInstanceSpec::upgradePolicy) + .orElse(UpgradePolicy.defaultPolicy)); + + // Prefer fallback with proper confidence. + for (Version target : targets) + if (compileVersion.isEmpty() || versionCompatibility.apply(instance).accept(target, compileVersion.get())) + return target; + + // Try fallback with any confidence. + for (VespaVersion target : reversed(versionStatus.deployableVersions())) + if (compileVersion.isEmpty() || versionCompatibility.apply(instance).accept(target.versionNumber(), compileVersion.get())) + return target.versionNumber(); throw new IllegalArgumentException("no legal platform version exists in this system for compile version " + compileVersion.get()); } @@ -579,6 +656,7 @@ public class DeploymentStatus { /** The test jobs that need to run prior to the given production deployment jobs. */ public Map<JobId, List<Job>> testJobs(Map<JobId, List<Job>> jobs) { Map<JobId, List<Job>> testJobs = new LinkedHashMap<>(); + // First, look for a declared test in the instance of each production job. jobs.forEach((job, versionsList) -> { for (JobType testType : List.of(systemTest(job.type()), stagingTest(job.type()))) { if (job.type().isProduction() && job.type().isDeployment()) { @@ -596,6 +674,7 @@ public class DeploymentStatus { } } }); + // If no declared test in the right instance was triggered, pick one from a different instance. jobs.forEach((job, versionsList) -> { for (JobType testType : List.of(systemTest(job.type()), stagingTest(job.type()))) { for (Job productionJob : versionsList) @@ -603,7 +682,8 @@ public class DeploymentStatus { && allJobs.successOn(testType, productionJob.versions()).asList().isEmpty() && testJobs.keySet().stream() .noneMatch(test -> test.type().equals(testType) && test.type().zone().equals(testType.zone()) - && testJobs.get(test).stream().anyMatch(testJob -> testJob.versions().equals(productionJob.versions())))) { + && testJobs.get(test).stream().anyMatch(testJob -> test.type().isSystemTest() ? testJob.versions().targetsMatch(productionJob.versions()) + : testJob.versions().equals(productionJob.versions())))) { JobId testJob = firstDeclaredOrElseImplicitTest(testType); testJobs.merge(testJob, List.of(new Job(testJob.type(), @@ -1014,10 +1094,18 @@ public class DeploymentStatus { @Override Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { Optional<Instant> deployedAt = status.jobSteps().get(prodId).completedAt(change, Optional.of(prodId)); + Versions target = Versions.from(change, status.application(), status.deploymentFor(job.id()), status.fallbackPlatform(change, job.id())); + Change applied = Change.empty(); + if (change.platform().isPresent()) + applied = applied.with(target.targetPlatform()); + if (change.revision().isPresent()) + applied = applied.with(target.targetRevision()); + Change relevant = applied; + return (dependent.equals(job()) ? job.lastTriggered().filter(run -> deployedAt.map(at -> ! run.start().isBefore(at)).orElse(false)).stream() : job.runs().values().stream()) .filter(Run::hasSucceeded) - .filter(run -> run.versions().targetsMatch(change)) + .filter(run -> run.versions().targetsMatch(relevant)) .flatMap(run -> run.end().stream()).findFirst(); } }; 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 7c0a4a83e4f..9e107a52b55 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 @@ -1,8 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.deployment; -import com.yahoo.component.Version; -import com.yahoo.component.VersionCompatibility; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; @@ -14,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.ApplicationController; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; @@ -22,8 +19,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.versions.VersionStatus; -import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import java.math.BigDecimal; import java.time.Clock; @@ -43,7 +38,6 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import static java.util.Comparator.comparing; -import static java.util.Comparator.reverseOrder; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; @@ -91,11 +85,10 @@ public class DeploymentTrigger { && acceptNewRevision(status, instanceName, outstanding.revision().get()); application = application.with(instanceName, instance -> withRemainingChange(instance, - status.withCompatibilityPlatform((deployOutstanding ? outstanding - : Change.empty()) - .onTopOf(instance.change()), - instanceName), - status)); + deployOutstanding ? outstanding.onTopOf(instance.change()) + : instance.change(), + status, + false)); } applications().store(application); }); @@ -114,7 +107,10 @@ public class DeploymentTrigger { applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { if (application.get().deploymentSpec().instance(id.instance()).isPresent()) applications().store(application.with(id.instance(), - instance -> withRemainingChange(instance, instance.change(), jobs.deploymentStatus(application.get())))); + instance -> withRemainingChange(instance, + instance.change(), + jobs.deploymentStatus(application.get()), + true))); }); } @@ -289,9 +285,17 @@ public class DeploymentTrigger { /** Overrides the given instance's platform and application changes with any contained in the given change. */ public void forceChange(ApplicationId instanceId, Change change) { + forceChange(instanceId, change, true); + } + + /** Overrides the given instance's platform and application changes with any contained in the given change. */ + public void forceChange(ApplicationId instanceId, Change change, boolean allowOutdatedPlatform) { applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { applications().store(application.with(instanceId.instance(), - instance -> withRemainingChange(instance, change.onTopOf(application.get().require(instanceId.instance()).change()), jobs.deploymentStatus(application.get())))); + instance -> withRemainingChange(instance, + change.onTopOf(instance.change()), + jobs.deploymentStatus(application.get()), + allowOutdatedPlatform))); }); } @@ -308,7 +312,10 @@ public class DeploymentTrigger { default: throw new IllegalArgumentException("Unknown cancellation choice '" + cancellation + "'!"); } applications().store(application.with(instanceId.instance(), - instance -> withRemainingChange(instance, change, jobs.deploymentStatus(application.get())))); + instance -> withRemainingChange(instance, + change, + jobs.deploymentStatus(application.get()), + true))); }); } @@ -424,13 +431,14 @@ public class DeploymentTrigger { } } - private Instance withRemainingChange(Instance instance, Change change, DeploymentStatus status) { + private Instance withRemainingChange(Instance instance, Change change, DeploymentStatus status, boolean allowOutdatedPlatform) { Change remaining = change; if (status.hasCompleted(instance.name(), change.withoutApplication())) remaining = remaining.withoutPlatform(); if (status.hasCompleted(instance.name(), change.withoutPlatform())) remaining = remaining.withoutApplication(); - return instance.withChange(remaining); + + return instance.withChange(status.withPermittedPlatform(remaining, instance.name(), allowOutdatedPlatform)); } // ---------- Version and job helpers ---------- diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index ef3474e0c1e..965f1b09819 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -258,6 +258,10 @@ public class InternalStepRunner implements StepRunner { throw e; } + catch (IllegalArgumentException e) { + logger.log(WARNING, e.getMessage()); + return Optional.of(deploymentFailed); + } catch (EndpointCertificateException e) { switch (e.type()) { case CERT_NOT_AVAILABLE: diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 11a784ce899..6a4975c3458 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -5,7 +5,9 @@ import com.google.common.collect.ImmutableSortedMap; import com.yahoo.component.Version; import com.yahoo.component.VersionCompatibility; import com.yahoo.concurrent.UncheckedTimeoutException; +import com.yahoo.config.application.api.DeploymentSpec.UpgradePolicy; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.controller.Application; @@ -29,7 +31,6 @@ import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageDiff; import com.yahoo.vespa.hosted.controller.application.pkg.TestPackage; -import com.yahoo.vespa.hosted.controller.application.pkg.TestPackage.TestSummary; import com.yahoo.vespa.hosted.controller.notification.Notification; import com.yahoo.vespa.hosted.controller.notification.Notification.Type; import com.yahoo.vespa.hosted.controller.notification.NotificationSource; @@ -37,6 +38,7 @@ import com.yahoo.vespa.hosted.controller.persistence.BufferedLogStore; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.io.IOException; import java.io.InputStream; @@ -570,25 +572,7 @@ public class JobController { application = application.withRevisions(revisions -> revisions.with(version.get())); application = withPrunedPackages(application, version.get().id()); - TestSummary testSummary = TestPackage.validateTests(submission.applicationPackage().deploymentSpec(), submission.testPackage()); - if (testSummary.problems().isEmpty()) - controller.notificationsDb().removeNotification(NotificationSource.from(id), Type.testPackage); - else - controller.notificationsDb().setNotification(NotificationSource.from(id), - Type.testPackage, - Notification.Level.warning, - testSummary.problems()); - - submission.applicationPackage().parentVersion().ifPresent(parent -> { - if (parent.getMajor() < controller.readSystemVersion().getMajor()) - controller.notificationsDb().setNotification(NotificationSource.from(id), - Type.submission, - Notification.Level.warning, - "Parent version used to compile the application is on a " + - "lower major version than the current Vespa Cloud version"); - else - controller.notificationsDb().removeNotification(NotificationSource.from(id), Type.submission); - }); + validate(id, submission); applications.storeWithUpdatedConfig(application, submission.applicationPackage()); if (application.get().projectId().isPresent()) @@ -597,6 +581,33 @@ public class JobController { return version.get(); } + private void validate(TenantAndApplicationId id, Submission submission) { + controller.notificationsDb().removeNotification(NotificationSource.from(id), Type.testPackage); + controller.notificationsDb().removeNotification(NotificationSource.from(id), Type.submission); + + validateTests(id, submission); + validateMajorVersion(id, submission); + } + + private void validateTests(TenantAndApplicationId id, Submission submission) { + var testSummary = TestPackage.validateTests(submission.applicationPackage().deploymentSpec(), submission.testPackage()); + if ( ! testSummary.problems().isEmpty()) + controller.notificationsDb().setNotification(NotificationSource.from(id), + Type.testPackage, + Notification.Level.warning, + testSummary.problems()); + + } + + private void validateMajorVersion(TenantAndApplicationId id, Submission submission) { + submission.applicationPackage().deploymentSpec().majorVersion().ifPresent(explicitMajor -> { + if ( ! controller.readVersionStatus().isOnCurrentMajor(new Version(explicitMajor))) + controller.notificationsDb().setNotification(NotificationSource.from(id), Type.submission, Notification.Level.warning, + "Vespa " + explicitMajor + " will soon be end of life, upgrade to Vespa " + (explicitMajor + 1) + " now: " + + "https://cloud.vespa.ai/en/vespa" + (explicitMajor + 1) + "-release-notes.html"); // ∠( ᐛ 」∠)_ + }); + } + private LockedApplication withPrunedPackages(LockedApplication application, RevisionId latest){ TenantAndApplicationId id = application.get().id(); Application wrapped = application.get(); @@ -665,18 +676,22 @@ public class JobController { /** Stores the given package and starts a deployment of it, after aborting any such ongoing deployment. */ public void deploy(ApplicationId id, JobType type, Optional<Version> platform, ApplicationPackage applicationPackage) { - deploy(id, type, platform, applicationPackage, false); + deploy(id, type, platform, applicationPackage, false, false); } /** Stores the given package and starts a deployment of it, after aborting any such ongoing deployment.*/ - public void deploy(ApplicationId id, JobType type, Optional<Version> platform, ApplicationPackage applicationPackage, boolean dryRun) { + public void deploy(ApplicationId id, JobType type, Optional<Version> platform, ApplicationPackage applicationPackage, + boolean dryRun, boolean allowOutdatedPlatform) { if ( ! controller.zoneRegistry().hasZone(type.zone())) throw new IllegalArgumentException(type.zone() + " is not present in this system"); controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { if ( ! application.get().instances().containsKey(id.instance())) application = controller.applications().withNewInstance(application, id); - + // TODO(mpolden): Enable for public CD once all tests have been updated + if (controller.system() != SystemName.PublicCd) { + controller.applications().validatePackage(applicationPackage, application.get()); + } controller.applications().store(application); }); @@ -686,13 +701,18 @@ public class JobController { long build = 1 + lastRun.map(run -> run.versions().targetRevision().number()).orElse(0L); RevisionId revisionId = RevisionId.forDevelopment(build, new JobId(id, type)); - ApplicationVersion version = ApplicationVersion.forDevelopment(revisionId, applicationPackage.compileVersion()); + ApplicationVersion version = ApplicationVersion.forDevelopment(revisionId, applicationPackage.compileVersion(), applicationPackage.deploymentSpec().majorVersion()); byte[] diff = getDiff(applicationPackage, deploymentId, lastRun); controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { - controller.applications().applicationStore().putDev(deploymentId, version.id(), applicationPackage.zippedContent(), diff); Version targetPlatform = platform.orElseGet(() -> findTargetPlatform(applicationPackage, deploymentId, application.get().get(id.instance()))); + if ( ! allowOutdatedPlatform + && ! controller.readVersionStatus().isOnCurrentMajor(targetPlatform) + && runs(id, type).values().stream().noneMatch(run -> run.versions().targetPlatform().getMajor() == targetPlatform.getMajor())) + throw new IllegalArgumentException("platform version " + targetPlatform + " is not on a current major version in this system"); + + controller.applications().applicationStore().putDev(deploymentId, version.id(), applicationPackage.zippedContent(), diff); controller.applications().store(application.withRevisions(revisions -> revisions.with(version))); start(id, type, @@ -724,9 +744,14 @@ public class JobController { private Version findTargetPlatform(ApplicationPackage applicationPackage, DeploymentId id, Optional<Instance> instance) { // Prefer previous platform if possible. Candidates are all deployable, ascending, with existing version appended; then reversed. - List<Version> versions = controller.readVersionStatus().deployableVersions().stream() - .map(VespaVersion::versionNumber) - .collect(toList()); + VersionStatus versionStatus = controller.readVersionStatus(); + Version systemVersion = controller.systemVersion(versionStatus); + + List<Version> versions = new ArrayList<>(List.of(systemVersion)); + for (VespaVersion version : versionStatus.deployableVersions()) + if (version.confidence().equalOrHigherThan(Confidence.normal)) + versions.add(version.versionNumber()); + instance.map(Instance::deployments) .map(deployments -> deployments.get(id.zoneId())) .map(Deployment::version) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java index 5bdc980f11a..ab8f7fe22d3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java @@ -49,12 +49,6 @@ public class RevisionHistory { for (ApplicationVersion revision : productionRevisions) production.put(revision.id(), revision); - // TODO jonmv: remove once it's run once on serialised data - String hash = ""; - for (ApplicationVersion revision : List.copyOf(production.values())) - if (hash.equals(hash = revision.bundleHash().orElse("")) && ! hash.isEmpty()) - production.put(revision.id(), revision.skipped()); - NavigableMap<JobId, NavigableMap<RevisionId, ApplicationVersion>> development = new TreeMap<>(comparator); developmentRevisions.forEach((job, jobRevisions) -> { NavigableMap<RevisionId, ApplicationVersion> revisions = development.computeIfAbsent(job, __ -> new TreeMap<>()); @@ -100,7 +94,7 @@ public class RevisionHistory { // Fallback for when an application version isn't known for the given key. private static ApplicationVersion revisionOf(RevisionId id) { - return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0); + return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0); } /** Returns the production {@link ApplicationVersion} with this revision ID. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java index e366920690b..6c9de2fd584 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Submission.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevisi import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import java.util.Optional; +import java.util.OptionalInt; import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage.calculateHash; @@ -42,6 +43,7 @@ public class Submission { source, authorEmail, applicationPackage.compileVersion(), + applicationPackage.deploymentSpec().majorVersion(), applicationPackage.buildTime(), sourceUrl, source.map(SourceRevision::commit), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index d683f1cb5c7..a172671fc8e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -119,12 +119,8 @@ public class Versions { } /** Create versions using given change and application */ - public static Versions from(Change change, Application application, Optional<Deployment> deployment, - Version defaultPlatformVersion) { - return new Versions(targetPlatform(application, change, deployment.map(Deployment::version), defaultPlatformVersion), - targetRevision(application, change, deployment.map(Deployment::revision)), - deployment.map(Deployment::version), - deployment.map(Deployment::revision)); + public static Versions from(Change change, Application application, Optional<Deployment> deployment, Version defaultPlatformVersion) { + return from(change, application, deployment.map(Deployment::version), deployment.map(Deployment::revision), defaultPlatformVersion); } private static Version targetPlatform(Application application, Change change, Optional<Version> existing, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java index 93ef6d29450..fd177c469d2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentUpgrader.java @@ -5,6 +5,7 @@ import com.yahoo.component.Version; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.Deployment; @@ -37,7 +38,7 @@ public class DeploymentUpgrader extends ControllerMaintainer { Version targetPlatform = null; // Upgrade to the newest non-broken, deployable version. for (VespaVersion platform : controller().readVersionStatus().deployableVersions()) - if (platform.confidence().equalOrHigherThan(VespaVersion.Confidence.low)) + if (platform.confidence().equalOrHigherThan(VespaVersion.Confidence.normal)) targetPlatform = platform.versionNumber(); if (targetPlatform == null) @@ -53,9 +54,13 @@ public class DeploymentUpgrader extends ControllerMaintainer { Run last = controller().jobController().last(job).get(); Versions target = new Versions(targetPlatform, last.versions().targetRevision(), Optional.of(last.versions().targetPlatform()), Optional.of(last.versions().targetRevision())); if ( ! last.hasEnded()) continue; - if (application.revisions().get(last.versions().targetRevision()).compileVersion() - .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform())) - .orElse(false)) continue; + ApplicationVersion devVersion = application.revisions().get(last.versions().targetRevision()); + if (devVersion.compileVersion() + .map(version -> controller().applications().versionCompatibility(instance.id()).refuse(version, target.targetPlatform())) + .orElse(false)) continue; + if ( devVersion.allowedMajor().isPresent() + && devVersion.allowedMajor().get() < targetPlatform.getMajor()) continue; + if ( ! deployment.version().isBefore(target.targetPlatform())) continue; if ( ! isLikelyNightFor(job)) continue; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java index 85588d2cf0f..d1f6ddc1644 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java @@ -120,7 +120,8 @@ public class ResourceMeterMaintainer extends ControllerMaintainer { Map<ZoneId, Double> deploymentCosts = snapshotsByInstance.getOrDefault(instanceName, List.of()).stream() .collect(Collectors.toUnmodifiableMap( ResourceSnapshot::getZoneId, - snapshot -> cost(snapshot.allocation(), systemName))); + snapshot -> cost(snapshot.allocation(), systemName), + Double::sum)); locked = locked.with(instanceName, i -> i.withDeploymentCosts(deploymentCosts)); updateCostMetrics(tenantAndApplication.instance(instanceName), deploymentCosts); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java index 4c72f6747d5..d45818c2822 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RetriggerMaintainer.java @@ -14,6 +14,12 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +/** + * Trigger any jobs that are marked for re-triggering to effectuate some other change, e.g. a change in access to a + * deployment's nodes. + * + * @author tokle + */ public class RetriggerMaintainer extends ControllerMaintainer { private static final Logger logger = Logger.getLogger(RetriggerMaintainer.class.getName()); @@ -32,7 +38,7 @@ public class RetriggerMaintainer extends ControllerMaintainer { .filter(this::needsTrigger) .filter(entry -> readyToTrigger(entry.jobId())) .forEach(entry -> controller().applications().deploymentTrigger().reTrigger(entry.jobId().application(), entry.jobId().type(), - "re-triggered by RetriggerMaintainer")); + "re-triggered by " + getClass().getSimpleName())); // Remove all jobs that has succeeded with the required job run and persist the list List<RetriggerEntry> remaining = retriggerEntries.stream() @@ -46,9 +52,7 @@ public class RetriggerMaintainer extends ControllerMaintainer { return 1.0; } - /* - Returns true if a job is ready to run, i.e is currently not running - */ + /** Returns true if a job is ready to run, i.e. is currently not running */ private boolean readyToTrigger(JobId jobId) { Optional<Run> existingRun = controller().jobController().active(jobId.application()).stream() .filter(run -> run.id().type().equals(jobId.type())) @@ -56,9 +60,7 @@ public class RetriggerMaintainer extends ControllerMaintainer { return existingRun.isEmpty(); } - /* - Returns true of job needs triggering. I.e the job has not run since the queue item was last run. - */ + /** Returns true of job needs triggering. I.e. the job has not run since the queue item was last run */ private boolean needsTrigger(RetriggerEntry entry) { return controller().jobController().lastCompleted(entry.jobId()) .filter(run -> run.id().number() < entry.requiredRun()) 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 f3c82e72ae3..037dacfcac9 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 @@ -107,7 +107,7 @@ public class Upgrader extends ControllerMaintainer { Map<ApplicationId, Version> targets = new LinkedHashMap<>(); for (Version version : DeploymentStatus.targetsForPolicy(versionStatus, controller().systemVersion(versionStatus), policy)) { targetAndNewer.add(version); - InstanceList eligible = eligibleForVersion(remaining, version); + InstanceList eligible = eligibleForVersion(remaining, version, versionStatus); InstanceList outdated = cancellationCriterion.apply(eligible); cancelUpgradesOf(outdated.upgrading(), "Upgrading to outdated versions"); @@ -134,10 +134,10 @@ public class Upgrader extends ControllerMaintainer { } } - private InstanceList eligibleForVersion(InstanceList instances, Version version) { + private InstanceList eligibleForVersion(InstanceList instances, Version version, VersionStatus versionStatus) { Change change = Change.of(version); return instances.not().failingOn(version) - .allowingMajorVersion(version.getMajor()) + .allowingMajorVersion(version.getMajor(), versionStatus) .compatibleWithPlatform(version, controller().applications()::versionCompatibility) .not().hasCompleted(change) // Avoid rescheduling change for instances without production steps. .onLowerVersionThan(version) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java index 71b8f1cd9b7..154455c5198 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VersionStatusUpdater.java @@ -13,6 +13,7 @@ import java.util.logging.Level; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.aborted; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.broken; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.high; +import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.legacy; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.low; import static com.yahoo.vespa.hosted.controller.api.integration.organization.SystemMonitor.Confidence.normal; @@ -47,14 +48,14 @@ public class VersionStatusUpdater extends ControllerMaintainer { } static SystemMonitor.Confidence convert(VespaVersion.Confidence confidence) { - switch (confidence) { - case aborted: return aborted; - case broken: return broken; - case low: return low; - case normal: return normal; - case high: return high; - default: throw new IllegalArgumentException("Unexpected confidence '" + confidence + "'"); - } + return switch (confidence) { + case aborted -> aborted; + case broken -> broken; + case low -> low; + case legacy -> legacy; + case normal -> normal; + case high -> high; + }; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notification.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notification.java index b6e18881dab..da423995b7e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notification.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notification.java @@ -66,10 +66,11 @@ public class Notification { } public enum Type { + /** Related to contents of application package, e.g., usage of deprecated features/syntax */ applicationPackage, - /** Related to contents of application package, e.g., old parent or compile version, or errors detectable on submission */ + /** Related to contents of application package detectable by the controller on submission */ submission, /** Related to contents of application test package, e.g., mismatch between deployment spec and provided tests */ @@ -83,6 +84,7 @@ public class Notification { /** Application cluster is reindexing document(s) */ reindex + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java index d2b12ab6edc..f753f22608d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationFormatter.java @@ -28,21 +28,14 @@ public class NotificationFormatter { } public FormattedNotification format(Notification n) { - switch (n.type()) { - case applicationPackage: - case submission: - return applicationPackage(n); - case deployment: - return deployment(n); - case testPackage: - return testPackage(n); - case reindex: - return reindex(n); - case feedBlock: - return feedBlock(n); - default: - return new FormattedNotification(n, n.type().name(), "", zoneRegistry.dashboardUrl(n.source().tenant())); - } + return switch (n.type()) { + case applicationPackage, submission -> applicationPackage(n); + case deployment -> deployment(n); + case testPackage -> testPackage(n); + case reindex -> reindex(n); + case feedBlock -> feedBlock(n); + default -> new FormattedNotification(n, n.type().name(), "", zoneRegistry.dashboardUrl(n.source().tenant())); + }; } private FormattedNotification applicationPackage(Notification n) { @@ -132,13 +125,10 @@ public class NotificationFormatter { var applicationId = ApplicationId.from(source.tenant(), application, instance); Function<Environment, URI> link = (Environment env) -> zoneRegistry.dashboardUrl(new RunId(applicationId, jobType, runNumber)); var environment = jobType.zone().environment(); - switch (environment) { - case dev: - case perf: - return link.apply(environment); - default: - return link.apply(Environment.prod); - } + return switch (environment) { + case dev, perf -> link.apply(environment); + default -> link.apply(Environment.prod); + }; } private String jobText(NotificationSource source) { @@ -162,14 +152,11 @@ public class NotificationFormatter { } private String levelText(Notification.Level level, int count) { - switch (level) { - case error: - return "failed"; - case warning: - return count > 1 ? Text.format("%d warnings", count) : "a warning"; - default: - return count > 1 ? Text.format("%d messages", count) : "a message"; - } + return switch (level) { + case error -> "failed"; + case warning -> count > 1 ? Text.format("%d warnings", count) : "a warning"; + default -> count > 1 ? Text.format("%d messages", count) : "a message"; + }; } private String clusterInfo(NotificationSource source) { 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 48d8627f407..5aa847f648a 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 @@ -120,6 +120,7 @@ public class ApplicationSerializer { private static final String hasPackageField = "hasPackage"; private static final String shouldSkipField = "shouldSkip"; private static final String compileVersionField = "compileVersion"; + private static final String allowedMajorField = "allowedMajor"; private static final String buildTimeField = "buildTime"; private static final String sourceUrlField = "sourceUrl"; private static final String bundleHashField = "bundleHash"; @@ -257,6 +258,7 @@ public class ApplicationSerializer { applicationVersion.source().ifPresent(source -> toSlime(source, object.setObject(sourceRevisionField))); applicationVersion.authorEmail().ifPresent(email -> object.setString(authorEmailField, email)); applicationVersion.compileVersion().ifPresent(version -> object.setString(compileVersionField, version.toString())); + applicationVersion.allowedMajor().ifPresent(major -> object.setLong(allowedMajorField, major)); applicationVersion.buildTime().ifPresent(time -> object.setLong(buildTimeField, time.toEpochMilli())); applicationVersion.sourceUrl().ifPresent(url -> object.setString(sourceUrlField, url)); applicationVersion.commit().ifPresent(commit -> object.setString(commitField, commit)); @@ -478,6 +480,7 @@ public class ApplicationSerializer { Optional<SourceRevision> sourceRevision = sourceRevisionFromSlime(object.field(sourceRevisionField)); Optional<String> authorEmail = SlimeUtils.optionalString(object.field(authorEmailField)); Optional<Version> compileVersion = SlimeUtils.optionalString(object.field(compileVersionField)).map(Version::fromString); + Optional<Integer> allowedMajor = SlimeUtils.optionalInteger(object.field(allowedMajorField)).stream().boxed().findFirst(); Optional<Instant> buildTime = SlimeUtils.optionalInstant(object.field(buildTimeField)); Optional<String> sourceUrl = SlimeUtils.optionalString(object.field(sourceUrlField)); Optional<String> commit = SlimeUtils.optionalString(object.field(commitField)); @@ -487,8 +490,8 @@ public class ApplicationSerializer { int risk = (int) object.field(riskField).asLong(); Optional<String> bundleHash = SlimeUtils.optionalString(object.field(bundleHashField)); - return new ApplicationVersion(id, sourceRevision, authorEmail, compileVersion, buildTime, sourceUrl, - commit, bundleHash, hasPackage, shouldSkip, description, risk); + return new ApplicationVersion(id, sourceRevision, authorEmail, compileVersion, allowedMajor, buildTime, + sourceUrl, commit, bundleHash, hasPackage, shouldSkip, description, risk); } private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) { 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 38cea6f8cef..ff374c9bc8d 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 @@ -7,7 +7,6 @@ import com.yahoo.component.annotation.Inject; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; @@ -15,10 +14,13 @@ import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.MultiplePathsLock; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.identifiers.ControllerVersion; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; -import com.yahoo.vespa.hosted.controller.api.integration.ServiceRegistry; import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveBucket; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; @@ -51,10 +53,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.NavigableMap; import java.util.Optional; -import java.util.OptionalInt; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeoutException; @@ -66,7 +68,6 @@ import java.util.stream.Collectors; import java.util.stream.LongStream; import static java.util.stream.Collectors.collectingAndThen; -import static java.util.stream.Collectors.toUnmodifiableList; /** * Curator backed database for storing the persistence state of controllers. This maps controller specific operations @@ -84,7 +85,9 @@ public class CuratorDb { private static final Duration defaultTryLockTimeout = Duration.ofSeconds(1); private static final Path root = Path.fromString("/controller/v1"); + private static final Path lockRoot = root.append("locks"); + private static final Path tenantRoot = root.append("tenants"); private static final Path applicationRoot = root.append("applications"); private static final Path jobRoot = root.append("jobs"); @@ -116,6 +119,7 @@ public class CuratorDb { private final Curator curator; private final Duration tryLockTimeout; + private final StringFlag lockScheme; // For each application id (path), store the ZK node version and its deserialised data - update when version changes. // This will grow to keep all applications in memory, but this should be OK @@ -125,13 +129,14 @@ public class CuratorDb { private final Map<Path, Pair<Integer, NavigableMap<RunId, Run>>> cachedHistoricRuns = new ConcurrentHashMap<>(); @Inject - public CuratorDb(Curator curator, ServiceRegistry services) { - this(curator, defaultTryLockTimeout, services.zoneRegistry().system()); + public CuratorDb(Curator curator, FlagSource flagSource) { + this(curator, defaultTryLockTimeout, flagSource); } - CuratorDb(Curator curator, Duration tryLockTimeout, SystemName system) { + CuratorDb(Curator curator, Duration tryLockTimeout, FlagSource flagSource) { this.curator = curator; this.tryLockTimeout = tryLockTimeout; + this.lockScheme = Flags.CONTROLLER_LOCK_SCHEME.bindTo(flagSource); } /** Returns all hostnames configured to be part of this ZooKeeper cluster */ @@ -139,29 +144,52 @@ public class CuratorDb { return Arrays.stream(curator.zooKeeperEnsembleConnectionSpec().split(",")) .filter(hostAndPort -> !hostAndPort.isEmpty()) .map(hostAndPort -> hostAndPort.split(":")[0]) - .collect(Collectors.toUnmodifiableList()); + .toList(); } // -------------- Locks --------------------------------------------------- + private enum LockScheme { + + OLD, BOTH, NEW; + + boolean isLegacy() { return this == OLD; } + + } + + /** Acquire the appropriate lock according to the lock scheme set by feature flag */ + private Mutex lock(Function<LockScheme, Path> pathFactory, Duration timeout) { + LockScheme scheme = LockScheme.valueOf(lockScheme.value().toUpperCase(Locale.ENGLISH)); + return switch (scheme) { + case OLD, NEW -> curator.lock(pathFactory.apply(scheme), timeout); + case BOTH -> new MultiplePathsLock(curator.lock(pathFactory.apply(LockScheme.OLD), timeout), + curator.lock(pathFactory.apply(LockScheme.NEW), timeout)); + }; + } + public Mutex lock(TenantName name) { - return curator.lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); + return lock(scheme -> lockPath(name, scheme.isLegacy()), defaultLockTimeout.multipliedBy(2)); } public Mutex lock(TenantAndApplicationId id) { - return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + return lock(scheme -> lockPath(id, scheme.isLegacy()), defaultLockTimeout.multipliedBy(2)); } public Mutex lockForDeployment(ApplicationId id, ZoneId zone) { - return curator.lock(lockPath(id, zone), deployLockTimeout); + return lock(scheme -> lockPath(id, zone, scheme.isLegacy()), deployLockTimeout); } public Mutex lock(ApplicationId id, JobType type) { - return curator.lock(lockPath(id, type), defaultLockTimeout); + return lock(scheme -> lockPath(id, type, scheme.isLegacy()), defaultLockTimeout); } public Mutex lock(ApplicationId id, JobType type, Step step) throws TimeoutException { - return tryLock(lockPath(id, type, step)); + try { + // TODO(mpolden): Revert to tryLock once lock scheme is removed + return lock(scheme -> lockPath(id, type, step, scheme.isLegacy()), tryLockTimeout); + } catch (UncheckedTimeoutException e) { + throw new TimeoutException(e.getMessage()); + } } public Mutex lockRotations() { @@ -387,7 +415,7 @@ public class CuratorDb { return curator.getChildren(applicationRoot).stream() .map(TenantAndApplicationId::fromSerialized) .sorted() - .collect(toUnmodifiableList()); + .toList(); } public void removeApplication(TenantAndApplicationId id) { @@ -620,8 +648,8 @@ public class CuratorDb { public List<TenantName> listTenantsWithNotifications() { return curator.getChildren(notificationsRoot).stream() - .map(TenantName::from) - .collect(Collectors.toUnmodifiableList()); + .map(TenantName::from) + .toList(); } public void writeNotifications(TenantName tenantName, List<Notification> notifications) { @@ -638,7 +666,6 @@ public class CuratorDb { return readSlime(supportAccessPath(deploymentId)).map(SupportAccessSerializer::fromSlime).orElse(SupportAccess.DISALLOWED_NO_HISTORY); } - /** Take lock before reading before writing */ public void writeSupportAccess(DeploymentId deploymentId, SupportAccess supportAccess) { curator.set(supportAccessPath(deploymentId), asJson(SupportAccessSerializer.toSlime(supportAccess))); } @@ -655,31 +682,34 @@ public class CuratorDb { // -------------- Paths --------------------------------------------------- - private Path lockPath(TenantName tenant) { - return lockRoot - .append(tenant.value()); + private Path lockPath(TenantName tenant, boolean legacy) { + Path currentRoot = legacy ? lockRoot : lockRoot.append("tenants"); + return currentRoot.append(tenant.value()); } - private Path lockPath(TenantAndApplicationId application) { - return lockRoot.append(application.tenant().value() + ":" + application.application().value()); + private Path lockPath(TenantAndApplicationId application, boolean legacy) { + Path currentRoot = legacy ? lockRoot : lockRoot.append("applications"); + return currentRoot.append(application.tenant().value() + ":" + application.application().value()); } - private Path lockPath(ApplicationId instance, ZoneId zone) { - return lockRoot.append(instance.serializedForm() + ":" + zone.environment().value() + ":" + zone.region().value()); + private Path lockPath(ApplicationId instance, ZoneId zone, boolean legacy) { + Path currentRoot = legacy ? lockRoot : lockRoot.append("instances"); + return currentRoot.append(instance.serializedForm() + ":" + zone.environment().value() + ":" + zone.region().value()); } - private Path lockPath(ApplicationId instance, JobType type) { - return lockRoot.append(instance.serializedForm() + ":" + type.jobName()); + private Path lockPath(ApplicationId instance, JobType type, boolean legacy) { + Path currentRoot = legacy ? lockRoot : lockRoot.append("jobs"); + return currentRoot.append(instance.serializedForm() + ":" + type.jobName()); } - private Path lockPath(ApplicationId instance, JobType type, Step step) { - return lockRoot.append(instance.serializedForm() + ":" + type.jobName() + ":" + step.name()); + private Path lockPath(ApplicationId instance, JobType type, Step step, boolean legacy) { + Path currentRoot = legacy ? lockRoot : lockRoot.append("steps"); + return currentRoot.append(instance.serializedForm() + ":" + type.jobName() + ":" + step.name()); } private Path lockPath(String provisionId) { - return lockRoot - .append(provisionStatePath()) - .append(provisionId); + return lockRoot.append(provisionStatePath()) + .append(provisionId); } private static Path upgradesPerMinutePath() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java index 8f36daf9756..85feb97f39b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java @@ -5,6 +5,8 @@ import com.yahoo.component.annotation.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.provision.SystemName; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; + import java.time.Duration; /** @@ -19,20 +21,19 @@ public class MockCuratorDb extends CuratorDb { @Inject public MockCuratorDb(ConfigserverConfig config) { - this("test-controller:2222", SystemName.from(config.system())); + this("test-controller:2222"); } public MockCuratorDb(SystemName system) { - this("test-controller:2222", system); + this("test-controller:2222"); } - public MockCuratorDb(String zooKeeperEnsembleConnectionSpec, SystemName system) { - this(new MockCurator() { @Override public String zooKeeperEnsembleConnectionSpec() { return zooKeeperEnsembleConnectionSpec; } }, - system); + public MockCuratorDb(String zooKeeperEnsembleConnectionSpec) { + this(new MockCurator() { @Override public String zooKeeperEnsembleConnectionSpec() { return zooKeeperEnsembleConnectionSpec; } }); } - public MockCuratorDb(MockCurator curator, SystemName system) { - super(curator, Duration.ofMillis(100), system); + public MockCuratorDb(MockCurator curator) { + super(curator, Duration.ofMillis(100), new InMemoryFlagSource()); this.curator = curator; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java index 99687a9893c..beda8942fc2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/NotificationsSerializer.java @@ -69,18 +69,16 @@ public class NotificationsSerializer { public List<Notification> fromSlime(TenantName tenantName, Slime slime) { return SlimeUtils.entriesStream(slime.get().field(notificationsFieldName)) - .filter(inspector -> { // TODO: remove in summer. - if ( ! inspector.field(jobTypeField).valid()) return true; - try { - JobType.ofSerialized(inspector.field(jobTypeField).asString()); - return true; - } - catch (RuntimeException e) { - return false; - } - }) - .map(inspector -> fromInspector(tenantName, inspector)) - .collect(Collectors.toUnmodifiableList()); + .filter(inspector -> { // TODO: remove in summer. + if (!inspector.field(jobTypeField).valid()) return true; + try { + JobType.ofSerialized(inspector.field(jobTypeField).asString()); + return true; + } catch (RuntimeException e) { + return false; + } + }) + .map(inspector -> fromInspector(tenantName, inspector)).toList(); } private Notification fromInspector(TenantName tenantName, Inspector inspector) { @@ -96,49 +94,49 @@ public class NotificationsSerializer { SlimeUtils.optionalString(inspector.field(clusterIdField)).map(ClusterSpec.Id::from), SlimeUtils.optionalString(inspector.field(jobTypeField)).map(jobName -> JobType.ofSerialized(jobName)), SlimeUtils.optionalLong(inspector.field(runNumberField))), - SlimeUtils.entriesStream(inspector.field(messagesField)).map(Inspector::asString).collect(Collectors.toUnmodifiableList())); + SlimeUtils.entriesStream(inspector.field(messagesField)).map(Inspector::asString).toList()); } private static String asString(Notification.Type type) { - switch (type) { - case applicationPackage: return "applicationPackage"; - case submission: return "submission"; - case testPackage: return "testPackage"; - case deployment: return "deployment"; - case feedBlock: return "feedBlock"; - case reindex: return "reindex"; - default: throw new IllegalArgumentException("No serialization defined for notification type " + type); - } + return switch (type) { + case applicationPackage -> "applicationPackage"; + case submission -> "submission"; + case testPackage -> "testPackage"; + case deployment -> "deployment"; + case feedBlock -> "feedBlock"; + case reindex -> "reindex"; + default -> throw new IllegalArgumentException("No serialization defined for notification type " + type); + }; } private static Notification.Type typeFrom(Inspector field) { - switch (field.asString()) { - case "applicationPackage": return Notification.Type.applicationPackage; - case "submission": return Notification.Type.submission; - case "testPackage": return Notification.Type.testPackage; - case "deployment": return Notification.Type.deployment; - case "feedBlock": return Notification.Type.feedBlock; - case "reindex": return Notification.Type.reindex; - default: throw new IllegalArgumentException("Unknown serialized notification type value '" + field.asString() + "'"); - } + return switch (field.asString()) { + case "applicationPackage" -> Notification.Type.applicationPackage; + case "submission" -> Notification.Type.submission; + case "testPackage" -> Notification.Type.testPackage; + case "deployment" -> Notification.Type.deployment; + case "feedBlock" -> Notification.Type.feedBlock; + case "reindex" -> Notification.Type.reindex; + default -> throw new IllegalArgumentException("Unknown serialized notification type value '" + field.asString() + "'"); + }; } private static String asString(Notification.Level level) { - switch (level) { - case info: return "info"; - case warning: return "warning"; - case error: return "error"; - default: throw new IllegalArgumentException("No serialization defined for notification level " + level); - } + return switch (level) { + case info -> "info"; + case warning -> "warning"; + case error -> "error"; + default -> throw new IllegalArgumentException("No serialization defined for notification level " + level); + }; } private static Notification.Level levelFrom(Inspector field) { - switch (field.asString()) { - case "info": return Notification.Level.info; - case "warning": return Notification.Level.warning; - case "error": return Notification.Level.error; - default: throw new IllegalArgumentException("Unknown serialized notification level value '" + field.asString() + "'"); - } + return switch (field.asString()) { + case "info" -> Notification.Level.info; + case "warning" -> Notification.Level.warning; + case "error" -> Notification.Level.error; + default -> throw new IllegalArgumentException("Unknown serialized notification level value '" + field.asString() + "'"); + }; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 6a50f4c52b4..341cba60519 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -2073,7 +2073,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (pin) change = change.withPin(); - controller.applications().deploymentTrigger().forceChange(id, change); + controller.applications().deploymentTrigger().forceChange(id, change, isOperator(request)); response.append("Triggered ").append(change).append(" for ").append(id); }); return new MessageResponse(response.toString()); @@ -2090,7 +2090,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { RevisionId revision = build == -1 ? application.get().revisions().last().get().id() : getRevision(application.get(), build); Change change = Change.of(revision); - controller.applications().deploymentTrigger().forceChange(id, change); + controller.applications().deploymentTrigger().forceChange(id, change, isOperator(request)); response.append("Triggered ").append(change).append(" for ").append(id); }); return new MessageResponse(response.toString()); @@ -2112,6 +2112,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { RevisionId revision = RevisionId.forProduction(Long.parseLong(build)); controller.applications().lockApplicationOrThrow(id, application -> { controller.applications().store(application.withRevisions(revisions -> revisions.with(revisions.get(revision).skipped()))); + for (Instance instance : application.get().instances().values()) + if (instance.change().revision().equals(Optional.of(revision))) + controller.applications().deploymentTrigger().cancelChange(instance.id(), ChangesToCancel.APPLICATION); }); return new MessageResponse("Marked build '" + build + "' as non-deployable"); } @@ -2278,7 +2281,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .map(Boolean::valueOf) .orElse(false); - controller.jobController().deploy(id, type, version, applicationPackage, dryRun); + controller.jobController().deploy(id, type, version, applicationPackage, dryRun, isOperator(request)); RunId runId = controller.jobController().last(id, type).get().id(); Slime slime = new Slime(); Cursor rootObject = slime.setObject(); 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 1bf2f78f866..76925241c1e 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 @@ -14,6 +14,7 @@ import com.yahoo.restapi.UriBuilder; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; @@ -174,6 +175,8 @@ public class DeploymentApiHandler extends ThreadedHttpRequestHandler { instanceObject.setString("upgradePolicy", toString(status.application().deploymentSpec().instance(instance.instance()) .map(DeploymentInstanceSpec::upgradePolicy) .orElse(DeploymentSpec.UpgradePolicy.defaultPolicy))); + status.application().revisions().last().flatMap(ApplicationVersion::compileVersion) + .ifPresent(compiled -> instanceObject.setString("compileVersion", compiled.toFullString())); Cursor jobsArray = instanceObject.setArray("jobs"); status.jobSteps().forEach((job, jobStatus) -> { if ( ! job.application().equals(instance)) return; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java index 2211e83c51a..5953c51782a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/horizon/TsdbQueryRewriter.java @@ -40,7 +40,12 @@ public class TsdbQueryRewriter { JsonNode execution = executionGraph.get(i); // Will be handled by rewriteFilters() - if (execution.has("filterId") && filterExists(root, execution.get("filterId").asText())) continue; + if (execution.has("filterId")) { + if (filterExists(root, execution.get("filterId").asText())) + continue; + else + throw new IllegalArgumentException("Invalid filterId: " + execution.get("filterId").asText()); + } rewriteFilter((ObjectNode) execution, tenantNames, operator, systemName); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index 9cced2b8159..893befb57a2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -101,6 +101,7 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse handleGET(Path path, HttpRequest request) { if (path.matches("/user/v1/user")) return userMetadata(request); + if (path.matches("/user/v1/find")) return findUser(request); if (path.matches("/user/v1/tenant/{tenant}")) return listTenantRoleMembers(path.get("tenant")); if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return listApplicationRoleMembers(path.get("tenant"), path.get("application")); @@ -133,6 +134,45 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { RoleDefinition.hostedSupporter, RoleDefinition.hostedAccountant); + private HttpResponse findUser(HttpRequest request) { + var email = request.getProperty("email"); + var query = request.getProperty("query"); + if (email != null) return userMetadataFromUserId(email); + if (query != null) return userMetadataQuery(query); + return ErrorResponse.badRequest("Need 'email' or 'query' parameter"); + } + + private HttpResponse userMetadataFromUserId(String email) { + var maybeUser = users.findUser(email); + + var slime = new Slime(); + var root = slime.setObject(); + var usersRoot = root.setArray("users"); + + if (maybeUser.isPresent()) { + var user = maybeUser.get(); + var roles = users.listRoles(new UserId(user.email())); + renderUserMetaData(usersRoot.addObject(), user, Set.copyOf(roles)); + } + + return new SlimeJsonResponse(slime); + } + + private HttpResponse userMetadataQuery(String query) { + var userList = users.findUsers(query); + + var slime = new Slime(); + var root = slime.setObject(); + var userSlime = root.setArray("users"); + + for (var user : userList) { + var roles = users.listRoles(new UserId((user.email()))); + renderUserMetaData(userSlime.addObject(), user, Set.copyOf(roles)); + } + + return new SlimeJsonResponse(slime); + } + private HttpResponse userMetadata(HttpRequest request) { User user; if (request.getJDiscRequest().context().get(User.ATTRIBUTE_NAME) instanceof User) { @@ -146,6 +186,12 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { Set<Role> roles = getAttribute(request, SecurityContext.ATTRIBUTE_NAME, SecurityContext.class).roles(); + var slime = new Slime(); + renderUserMetaData(slime.setObject(), user, roles); + return new SlimeJsonResponse(slime); + } + + private void renderUserMetaData(Cursor root, User user, Set<Role> roles) { Map<TenantName, List<TenantRole>> tenantRolesByTenantName = roles.stream() .flatMap(role -> filterTenantRoles(role).stream()) .distinct() @@ -156,10 +202,7 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { List<Role> operatorRoles = roles.stream() .filter(role -> hostedOperators.contains(role.definition())) .sorted(Comparator.comparing(Role::definition)) - .collect(Collectors.toList()); - - Slime slime = new Slime(); - Cursor root = slime.setObject(); + .toList(); root.setBool("isPublic", controller.system().isPublic()); root.setBool("isCd", controller.system().isCd()); @@ -185,8 +228,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { } UserFlagsSerializer.toSlime(root, flagsDb.getAllFlagData(), tenantRolesByTenantName.keySet(), !operatorRoles.isEmpty(), user.email()); - - return new SlimeJsonResponse(slime); } private HttpResponse listTenantRoleMembers(String tenantName) { @@ -256,7 +297,7 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { var user = new UserId(require("user", Inspector::asString, requestObject)); var roles = SlimeStream.fromArray(requestObject.field("roles"), Inspector::asString) .map(roleName -> Roles.toRole(tenant, roleName)) - .collect(Collectors.toUnmodifiableList()); + .toList(); users.addToRoles(user, roles); return new MessageResponse(user + " is now a member of " + roles.stream().map(Role::toString).collect(Collectors.joining(", "))); @@ -268,7 +309,7 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { var user = new UserId(require("user", Inspector::asString, requestObject)); var roles = SlimeStream.fromArray(requestObject.field("roles"), Inspector::asString) .map(roleName -> Roles.toRole(tenant, roleName)) - .collect(Collectors.toUnmodifiableList()); + .toList(); enforceLastAdminOfTenant(tenant, user, roles); removeDeveloperKey(tenant, user, roles); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingId.java index 67eafe6235d..21c8b5aeb87 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingId.java @@ -12,42 +12,14 @@ import java.util.Objects; * * @author mpolden */ -public class RoutingId { +public record RoutingId(ApplicationId instance, + EndpointId endpointId, + TenantAndApplicationId application) { - private final TenantAndApplicationId application; - private final ApplicationId instance; - private final EndpointId endpointId; - - private RoutingId(ApplicationId instance, EndpointId endpointId) { - this.instance = Objects.requireNonNull(instance, "application must be non-null"); - this.endpointId = Objects.requireNonNull(endpointId, "endpointId must be non-null"); - - application = TenantAndApplicationId.from(instance); - } - - public TenantAndApplicationId application() { - return application; - } - - public ApplicationId instance() { - return instance; - } - - public EndpointId endpointId() { - return endpointId; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RoutingId routingId = (RoutingId) o; - return application.equals(routingId.application) && instance.equals(routingId.instance) && endpointId.equals(routingId.endpointId); - } - - @Override - public int hashCode() { - return Objects.hash(application, instance, endpointId); + public RoutingId { + Objects.requireNonNull(instance, "application must be non-null"); + Objects.requireNonNull(endpointId, "endpointId must be non-null"); + Objects.requireNonNull(application, "application must be non-null"); } @Override @@ -56,7 +28,7 @@ public class RoutingId { } public static RoutingId of(ApplicationId instance, EndpointId endpoint) { - return new RoutingId(instance, endpoint); + return new RoutingId(instance, endpoint, TenantAndApplicationId.from(instance)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java index fa6741ec8b8..585cda65e66 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java @@ -23,14 +23,12 @@ import java.util.Set; * @author mortent * @author mpolden */ -public class RoutingPolicy { - - private final RoutingPolicyId id; - private final DomainName canonicalName; - private final Optional<String> dnsZone; - private final Set<EndpointId> instanceEndpoints; - private final Set<EndpointId> applicationEndpoints; - private final Status status; +public record RoutingPolicy(RoutingPolicyId id, + DomainName canonicalName, + Optional<String> dnsZone, + Set<EndpointId> instanceEndpoints, + Set<EndpointId> applicationEndpoints, + Status status) { /** DO NOT USE. Public for serialization purposes */ public RoutingPolicy(RoutingPolicyId id, DomainName canonicalName, Optional<String> dnsZone, @@ -123,15 +121,11 @@ public class RoutingPolicy { } /** The status of a routing policy */ - public static class Status { - - private final boolean active; - private final RoutingStatus routingStatus; + public record Status(boolean active, RoutingStatus routingStatus) { /** DO NOT USE. Public for serialization purposes */ - public Status(boolean active, RoutingStatus routingStatus) { - this.active = active; - this.routingStatus = Objects.requireNonNull(routingStatus, "globalRouting must be non-null"); + public Status { + Objects.requireNonNull(routingStatus, "routingStatus must be non-null"); } /** Returns whether this is considered active according to the load balancer status */ @@ -149,20 +143,6 @@ public class RoutingPolicy { return new Status(active, routingStatus); } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Status status = (Status) o; - return active == status.active && - routingStatus.equals(status.routingStatus); - } - - @Override - public int hashCode() { - return Objects.hash(active, routingStatus); - } - } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicyId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicyId.java index d64241b1239..1c0b41155fd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicyId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicyId.java @@ -13,26 +13,12 @@ import java.util.Objects; * * @author mpolden */ -public class RoutingPolicyId { +public record RoutingPolicyId(ApplicationId owner, ClusterSpec.Id cluster, ZoneId zone) { - private final ApplicationId owner; - private final ClusterSpec.Id cluster; - private final ZoneId zone; - - public RoutingPolicyId(ApplicationId owner, ClusterSpec.Id cluster, ZoneId zone) { - this.owner = Objects.requireNonNull(owner, "owner must be non-null"); - this.cluster = Objects.requireNonNull(cluster, "cluster must be non-null"); - this.zone = Objects.requireNonNull(zone, "zone must be non-null"); - } - - /** The application owning this */ - public ApplicationId owner() { - return owner; - } - - /** The zone this applies to */ - public ZoneId zone() { - return zone; + public RoutingPolicyId { + Objects.requireNonNull(owner, "owner must be non-null"); + Objects.requireNonNull(cluster, "cluster must be non-null"); + Objects.requireNonNull(zone, "zone must be non-null"); } /** The deployment this applies to */ @@ -40,26 +26,6 @@ public class RoutingPolicyId { return new DeploymentId(owner, zone); } - /** The cluster this applies to */ - public ClusterSpec.Id cluster() { - return cluster; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RoutingPolicyId that = (RoutingPolicyId) o; - return owner.equals(that.owner) && - cluster.equals(that.cluster) && - zone.equals(that.zone); - } - - @Override - public int hashCode() { - return Objects.hash(owner, cluster, zone); - } - @Override public String toString() { return "routing policy for " + cluster + ", in " + zone + ", owned by " + owner; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingStatus.java index 58f0005d488..de16089e735 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingStatus.java @@ -13,19 +13,15 @@ import java.util.Objects; * * @author mpolden */ -public class RoutingStatus { +public record RoutingStatus(Value value, Agent agent, Instant changedAt) { public static final RoutingStatus DEFAULT = new RoutingStatus(Value.in, Agent.system, Instant.EPOCH); - private final Value value; - private final Agent agent; - private final Instant changedAt; - /** DO NOT USE. Public for serialization purposes */ - public RoutingStatus(Value value, Agent agent, Instant changedAt) { - this.value = Objects.requireNonNull(value, "value must be non-null"); - this.agent = Objects.requireNonNull(agent, "agent must be non-null"); - this.changedAt = Objects.requireNonNull(changedAt, "changedAt must be non-null"); + public RoutingStatus { + Objects.requireNonNull(value, "value must be non-null"); + Objects.requireNonNull(agent, "agent must be non-null"); + Objects.requireNonNull(changedAt, "changedAt must be non-null"); } /** @@ -47,21 +43,6 @@ public class RoutingStatus { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RoutingStatus that = (RoutingStatus) o; - return value == that.value && - agent == that.agent && - changedAt.equals(that.changedAt); - } - - @Override - public int hashCode() { - return Objects.hash(value, agent, changedAt); - } - - @Override public String toString() { return "status " + value + ", changed by " + agent + " @ " + changedAt; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/ZoneRoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/ZoneRoutingPolicy.java index 60605df1002..a404be76507 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/ZoneRoutingPolicy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/ZoneRoutingPolicy.java @@ -13,38 +13,11 @@ import java.util.Objects; * * @author mpolden */ -public class ZoneRoutingPolicy { +public record ZoneRoutingPolicy(ZoneId zone, RoutingStatus routingStatus) { - private final ZoneId zone; - private final RoutingStatus routingStatus; - - public ZoneRoutingPolicy(ZoneId zone, RoutingStatus routingStatus) { - this.zone = Objects.requireNonNull(zone, "zone must be non-null"); - this.routingStatus = Objects.requireNonNull(routingStatus, "globalRouting must be non-null"); - } - - /** The zone this applies to */ - public ZoneId zone() { - return zone; - } - - /** Routing status of this policy */ - public RoutingStatus routingStatus() { - return routingStatus; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ZoneRoutingPolicy that = (ZoneRoutingPolicy) o; - return zone.equals(that.zone) && - routingStatus.equals(that.routingStatus); - } - - @Override - public int hashCode() { - return Objects.hash(zone, routingStatus); + public ZoneRoutingPolicy { + Objects.requireNonNull(zone, "zone must be non-null"); + Objects.requireNonNull(routingStatus, "globalRouting must be non-null"); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/Rotation.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/Rotation.java index 8eeab8c20e3..ea97b1da4de 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/Rotation.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/Rotation.java @@ -10,14 +10,11 @@ import java.util.Objects; * * @author mpolden */ -public class Rotation { +public record Rotation(RotationId id, String name) { - private final RotationId id; - private final String name; - - public Rotation(RotationId id, String name) { - this.id = Objects.requireNonNull(id); - this.name = Objects.requireNonNull(name); + public Rotation { + Objects.requireNonNull(id); + Objects.requireNonNull(name); } /** The ID of the allocated rotation. This value is generated by global routing system */ @@ -31,19 +28,6 @@ public class Rotation { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof Rotation)) return false; - final Rotation rotation = (Rotation) o; - return id().equals(rotation.id()) && name().equals(rotation.name()); - } - - @Override - public int hashCode() { - return Objects.hash(id(), name()); - } - - @Override public String toString() { return Text.format("rotation %s -> %s", id().asString(), name()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationId.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationId.java index 4d97962a40a..95cebf7ea78 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationId.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationId.java @@ -1,20 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.routing.rotation; -import java.util.Objects; - /** * ID of a global rotation. * * @author mpolden */ -public class RotationId { - - private final String id; - - public RotationId(String id) { - this.id = id; - } +public record RotationId(String id) { /** Rotation ID, e.g. rotation-42.vespa.global.routing */ public String asString() { @@ -22,19 +14,6 @@ public class RotationId { } @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RotationId that = (RotationId) o; - return Objects.equals(id, that.id); - } - - @Override - public int hashCode() { - return Objects.hash(id); - } - - @Override public String toString() { return "rotation ID " + id; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationStatus.java index 6d95ad9a230..89247ca2a31 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/rotation/RotationStatus.java @@ -13,13 +13,11 @@ import java.util.Objects; * * @author mpolden */ -public class RotationStatus { +public record RotationStatus(Map<RotationId, Targets> status) { public static final RotationStatus EMPTY = new RotationStatus(Map.of()); - private final Map<RotationId, Targets> status; - - private RotationStatus(Map<RotationId, Targets> status) { + public RotationStatus(Map<RotationId, Targets> status) { this.status = Map.copyOf(Objects.requireNonNull(status)); } @@ -46,31 +44,15 @@ public class RotationStatus { return "rotation status " + status; } - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - RotationStatus that = (RotationStatus) o; - return status.equals(that.status); - } - - @Override - public int hashCode() { - return Objects.hash(status); - } - public static RotationStatus from(Map<RotationId, Targets> targets) { return targets.isEmpty() ? EMPTY : new RotationStatus(targets); } /** Targets of a rotation */ - public static class Targets { + public record Targets(Map<ZoneId, RotationState> targets, Instant lastUpdated) { public static final Targets NONE = new Targets(Map.of(), Instant.EPOCH); - private final Map<ZoneId, RotationState> targets; - private final Instant lastUpdated; - public Targets(Map<ZoneId, RotationState> targets, Instant lastUpdated) { this.targets = Map.copyOf(Objects.requireNonNull(targets, "states must be non-null")); this.lastUpdated = Objects.requireNonNull(lastUpdated, "lastUpdated must be non-null"); @@ -80,24 +62,6 @@ public class RotationStatus { return targets; } - public Instant lastUpdated() { - return lastUpdated; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Targets targets1 = (Targets) o; - return targets.equals(targets1.targets) && - lastUpdated.equals(targets1.lastUpdated); - } - - @Override - public int hashCode() { - return Objects.hash(targets, lastUpdated); - } - } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index add95c2f7b2..b0a37474af7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.versions; +import ai.vespa.validation.Validation; +import com.yahoo.collections.Iterables; import com.yahoo.component.Version; import com.yahoo.config.provision.HostName; import com.yahoo.vespa.hosted.controller.Controller; @@ -9,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.SystemApplication; import com.yahoo.vespa.hosted.controller.maintenance.SystemUpgrader; +import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.util.ArrayList; import java.util.Collections; @@ -225,14 +228,14 @@ public record VersionStatus(List<VespaVersion> versions) { if (!confidenceIsOverridden) { // Always compute confidence for system and controller if (isSystemVersion || isControllerVersion) { - confidence = VespaVersion.confidenceFrom(statistics, controller); + confidence = VespaVersion.confidenceFrom(statistics, controller, versionStatus); } else { - // This is an older version so we preserve the existing confidence, if any + // This is an older version, so we preserve the existing confidence, if any confidence = versionStatus.versions().stream() .filter(v -> statistics.version().equals(v.versionNumber())) .map(VespaVersion::confidence) .findFirst() - .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller)); + .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller, versionStatus)); } } @@ -262,4 +265,14 @@ public record VersionStatus(List<VespaVersion> versions) { confidence); } + /** Whether no version on a newer major, with high confidence, can be deployed. */ + public boolean isOnCurrentMajor(Version version) { + for (VespaVersion available : deployableVersions()) + if ( available.confidence().equalOrHigherThan(Confidence.high) + && available.versionNumber().getMajor() > version.getMajor()) + return false; + + return true; + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java index 4cb10aa6ba9..0814e9ea6ec 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java @@ -29,11 +29,11 @@ public record VespaVersion(Version version, List<NodeVersion> nodeVersions, Confidence confidence) implements Comparable<VespaVersion> { - public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { + public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller, VersionStatus versionStatus) { int thisMajorVersion = statistics.version().getMajor(); InstanceList all = InstanceList.from(controller.jobController().deploymentStatuses(ApplicationList.from(controller.applications().asList()) .withProductionDeployment())) - .allowingMajorVersion(thisMajorVersion); + .allowingMajorVersion(thisMajorVersion, versionStatus); // 'production on this': All production deployment jobs upgrading to this version have completed without failure InstanceList productionOnThis = all.matching(instance -> statistics.productionSuccesses().stream().anyMatch(run -> run.id().application().equals(instance))) .not().failingUpgrade() @@ -123,6 +123,9 @@ public record VespaVersion(Version version, /** We don't have sufficient evidence that this version is working */ low, + + /** This version works, but we want users to stop using it */ + legacy, /** We have sufficient evidence that this version is working */ normal, |