Merge pull request #24553 from vespa-engine/jonmv/controller-code-cleanup

Jonmv/controller code cleanup

9 files changed, 101 insertions, 206 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 d8234e4f269..7e284a8aef5 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
@@ -37,6 +37,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
@@ -48,9 +50,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.RestartFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore;
-import com.yahoo.vespa.hosted.controller.application.ActivateResult;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
+import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics.Warning;
 import com.yahoo.vespa.hosted.controller.application.DeploymentQuotaCalculator;
 import com.yahoo.vespa.hosted.controller.application.QuotaUsage;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
@@ -108,6 +110,9 @@ import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence
 import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.low;
 import static com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence.normal;
 import static java.util.Comparator.naturalOrder;
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.counting;
+import static java.util.stream.Collectors.groupingBy;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toMap;
@@ -475,7 +480,7 @@ public class ApplicationController {
     }
 
     /** Deploys an application package for an existing application instance. */
-    public ActivateResult deploy(JobId job, boolean deploySourceVersions, Consumer<String> deployLogger) {
+    public DeploymentResult deploy(JobId job, boolean deploySourceVersions, Consumer<String> deployLogger) {
         if (job.application().instance().isTester())
             throw new IllegalArgumentException("'" + job.application() + "' is a tester application!");
 
@@ -515,8 +520,8 @@ public class ApplicationController {
             } // Release application lock while doing the deployment, which is a lengthy task.
 
             // Carry out deployment without holding the application lock.
-            ActivateResult result = deploy(job.application(), instance.tags(), applicationPackage, zone, platform, containerEndpoints,
-                                           endpointCertificateMetadata, run.isDryRun());
+            DeploymentResult result = deploy(job.application(), instance.tags(), applicationPackage, zone, platform, containerEndpoints,
+                                             endpointCertificateMetadata, run.isDryRun());
 
             endpointCertificateMetadata.ifPresent(e -> deployLogger.accept("Using CA signed certificate version %s".formatted(e.version())));
 
@@ -530,12 +535,11 @@ public class ApplicationController {
                                         ? NotificationSource.from(deployment)
                                         : revision.equals(lastRevision.get()) ? NotificationSource.from(applicationId) : null;
             if (source != null) {
-                @SuppressWarnings("deprecation")
-                List<String> warnings = Optional.ofNullable(result.prepareResponse().log)
+                List<String> warnings = Optional.ofNullable(result.log())
                                                 .map(logs -> logs.stream()
-                                                                 .filter(log -> log.applicationPackage)
-                                                                 .filter(log -> LogLevel.parse(log.level).intValue() >= Level.WARNING.intValue())
-                                                                 .map(log -> log.message)
+                                                                 .filter(LogEntry::concernsPackage)
+                                                                 .filter(log -> log.level().intValue() >= Level.WARNING.intValue())
+                                                                 .map(LogEntry::message)
                                                                  .sorted()
                                                                  .distinct()
                                                                  .collect(Collectors.toList()))
@@ -547,7 +551,7 @@ public class ApplicationController {
             lockApplicationOrThrow(applicationId, application ->
                     store(application.with(job.application().instance(),
                                            i -> i.withNewDeployment(zone, revision, platform,
-                                                                    clock.instant(), warningsFrom(result),
+                                                                    clock.instant(), warningsFrom(result.log()),
                                                                     quotaUsage))));
             return result;
         }
@@ -600,7 +604,7 @@ public class ApplicationController {
     }
 
     /** Deploy a system application to given zone */
-    public ActivateResult deploySystemApplicationPackage(SystemApplication application, ZoneId zone, Version version) {
+    public DeploymentResult deploySystemApplicationPackage(SystemApplication application, ZoneId zone, Version version) {
         if (application.hasApplicationPackage()) {
             ApplicationPackage applicationPackage = new ApplicationPackage(
                     artifactRepository.getSystemApplicationPackage(application.id(), zone, version)
@@ -612,14 +616,14 @@ public class ApplicationController {
     }
 
     /** Deploys the given tester application to the given zone. */
-    public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, Version platform) {
+    public DeploymentResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, Version platform) {
         return deploy(tester.id(), Tags.empty(), applicationPackage, zone, platform, Set.of(), /* No application cert for tester*/ Optional.empty(), false);
     }
 
-    private ActivateResult deploy(ApplicationId application, Tags tags, ApplicationPackage applicationPackage,
-                                  ZoneId zone, Version platform, Set<ContainerEndpoint> endpoints,
-                                  Optional<EndpointCertificateMetadata> endpointCertificateMetadata,
-                                  boolean dryRun) {
+    private DeploymentResult deploy(ApplicationId application, Tags tags, ApplicationPackage applicationPackage,
+                                    ZoneId zone, Version platform, Set<ContainerEndpoint> endpoints,
+                                    Optional<EndpointCertificateMetadata> endpointCertificateMetadata,
+                                    boolean dryRun) {
         DeploymentId deployment = new DeploymentId(application, zone);
         try {
             Optional<DockerImage> dockerImageRepo = Optional.ofNullable(
@@ -657,8 +661,7 @@ public class ApplicationController {
                                                            deploymentQuota, tenantSecretStores, operatorCertificates,
                                                            cloudAccount, dryRun));
 
-            return new ActivateResult(new com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(),
-                                      applicationPackage.zippedContent().length);
+            return preparedApplication.deploymentResult();
         } finally {
             // Even if prepare fails, routing configuration may need to be updated
             if ( ! application.instance().isTester()) {
@@ -1020,16 +1023,13 @@ public class ApplicationController {
     }
 
     /** Extract deployment warnings metric from deployment result */
-    private static Map<DeploymentMetrics.Warning, Integer> warningsFrom(ActivateResult result) {
-        if (result.prepareResponse().log == null) return Map.of();
-        Map<DeploymentMetrics.Warning, Integer> warnings = new HashMap<>();
-        for (Log log : result.prepareResponse().log) {
-            // TODO: Categorize warnings. Response from config server should be updated to include the appropriate
-            //  category and typed log level
-            if (!"warn".equalsIgnoreCase(log.level) && !"warning".equalsIgnoreCase(log.level)) continue;
-            warnings.merge(DeploymentMetrics.Warning.all, 1, Integer::sum);
-        }
-        return Map.copyOf(warnings);
+    private static Map<DeploymentMetrics.Warning, Integer> warningsFrom(List<DeploymentResult.LogEntry> log) {
+        return log.stream()
+                  .filter(entry -> entry.level().intValue() >= Level.WARNING.intValue())
+                  // TODO: Categorize warnings. Response from config server should be updated to include the appropriate
+                  //  category and typed log level
+                  .collect(groupingBy(__ -> Warning.all,
+                                      collectingAndThen(counting(), Long::intValue)));
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java
deleted file mode 100644
index 989a0de0f83..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ActivateResult.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.application;
-
-import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
-
-/**
- * @author Oyvind Gronnesby
- */
-public class ActivateResult {
-    // TODO jvenstad: Replace this class with just the PrepareResponse.
-
-    private final RevisionId revisionId;
-    private final PrepareResponse prepareResponse;
-    private final long applicationZipSizeBytes;
-
-    public ActivateResult(RevisionId revisionId, PrepareResponse prepareResponse, long applicationZipSizeBytes) {
-        this.revisionId = revisionId;
-        this.prepareResponse = prepareResponse;
-        this.applicationZipSizeBytes = applicationZipSizeBytes;
-    }
-
-    public long applicationZipSizeBytes() {
-        return applicationZipSizeBytes;
-    }
-
-    public RevisionId revisionId() {
-        return revisionId;
-    }
-
-    public PrepareResponse prepareResponse() {
-        return prepareResponse;
-    }
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java
index c2c95a0c4bf..b99d825a779 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.application.pkg;
 
 import com.google.common.hash.Funnel;
+import com.google.common.hash.Hasher;
 import com.google.common.hash.Hashing;
 import com.yahoo.component.Version;
 import com.yahoo.compress.ArchiveStreamReader;
@@ -71,8 +72,8 @@ public class ApplicationPackage {
     static final String deploymentFile = "deployment.xml";
     private static final String validationOverridesFile = "validation-overrides.xml";
     static final String servicesFile = "services.xml";
+    private static Hasher hasher() { return Hashing.murmur3_128().newHasher(); }
 
-    private final String contentHash;
     private final String bundleHash;
     private final byte[] zippedContent;
     private final DeploymentSpec deploymentSpec;
@@ -98,10 +99,8 @@ public class ApplicationPackage {
      * it must not be further changed by the caller.
      * If 'requireFiles' is true, files needed by deployment orchestration must be present.
      */
-    @SuppressWarnings("deprecation") // for Hashing.sha1()
     public ApplicationPackage(byte[] zippedContent, boolean requireFiles) {
         this.zippedContent = Objects.requireNonNull(zippedContent, "The application package content cannot be null");
-        this.contentHash = Hashing.sha1().hashBytes(zippedContent).toString();
         this.files = new ZipArchiveCache(zippedContent, Set.of(deploymentFile, validationOverridesFile, servicesFile, buildMetaFile, trustedCertificatesFile));
 
         Optional<DeploymentSpec> deploymentSpec = files.get(deploymentFile).map(bytes -> new String(bytes, UTF_8)).map(DeploymentSpec::fromXml);
@@ -134,9 +133,6 @@ public class ApplicationPackage {
         return new ApplicationPackage(modified.toByteArray());
     }
 
-    /** Returns a hash of the content of this package */
-    public String hash() { return contentHash; }
-
     /** Hash of all files and settings that influence what is deployed to config servers. */
     public String bundleHash() {
         return bundleHash;
@@ -242,7 +238,6 @@ public class ApplicationPackage {
     }
 
     // Hashes all files and settings that require a deployment to be forwarded to configservers
-    @SuppressWarnings("deprecation") // for Hashing.sha1()
     private String calculateBundleHash(byte[] zippedContent) {
         Predicate<String> entryMatcher = name -> ! name.endsWith(deploymentFile) && ! name.endsWith(buildMetaFile);
         SortedMap<String, Long> crcByEntry = new TreeMap<>();
@@ -258,17 +253,14 @@ public class ApplicationPackage {
             into.putBytes(key.getBytes());
             into.putLong(value);
         });
-        return Hashing.sha1().newHasher()
-                      .putObject(crcByEntry, funnel)
-                      .putInt(deploymentSpec.deployableHashCode())
-                      .hash().toString();
+        return hasher().putObject(crcByEntry, funnel)
+                       .putInt(deploymentSpec.deployableHashCode())
+                       .hash().toString();
     }
 
-    @SuppressWarnings("deprecation") // for Hashing.sha1()
     public static String calculateHash(byte[] bytes) {
-        return Hashing.sha1().newHasher()
-                      .putBytes(bytes)
-                      .hash().toString();
+        return hasher().putBytes(bytes)
+                       .hash().toString();
     }
 
 
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 dcdfea6e594..7a459e4f29f 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
@@ -13,7 +13,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.Instance;
@@ -21,9 +20,9 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateException;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId;
@@ -31,7 +30,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentFailureMails;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail;
-import com.yahoo.vespa.hosted.controller.application.ActivateResult;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
@@ -205,21 +203,18 @@ public class InternalStepRunner implements StepRunner {
                       logger);
     }
 
-    @SuppressWarnings("deprecation")
-    private Optional<RunStatus> deploy(Supplier<ActivateResult> deployment, Instant startTime, DualLogger logger) {
+    private Optional<RunStatus> deploy(Supplier<DeploymentResult> deployment, Instant startTime, DualLogger logger) {
         try {
-            PrepareResponse prepareResponse = deployment.get().prepareResponse();
-            if (prepareResponse.log != null)
-                logger.logAll(prepareResponse.log.stream()
-                                                 .map(entry -> new LogEntry(0, // Sequenced by BufferedLogStore.
-                                                                            Instant.ofEpochMilli(entry.time),
-                                                                            LogEntry.typeOf(LogLevel.parse(entry.level)),
-                                                                            entry.message))
-                                                 .collect(toList()));
+            DeploymentResult result = deployment.get();
+                logger.logAll(result.log().stream()
+                                    .map(entry -> new LogEntry(0, // Sequenced by BufferedLogStore.
+                                                               Instant.ofEpochMilli(entry.epochMillis()),
+                                                               LogEntry.typeOf(entry.level()),
+                                                               entry.message()))
+                                    .collect(toList()));
 
             logger.log("Deployment successful.");
-            if (prepareResponse.message != null)
-                logger.log(prepareResponse.message);
+            logger.log(result.message());
 
             return Optional.of(running);
         }
@@ -231,38 +226,42 @@ public class InternalStepRunner implements StepRunner {
                 logger.log(WARNING, "Deployment failed for one hour; giving up now!");
 
             switch (e.code()) {
-                case CERTIFICATE_NOT_READY:
+                case CERTIFICATE_NOT_READY -> {
                     logger.log("No valid CA signed certificate for app available to config server");
                     if (startTime.plus(timeouts.endpointCertificate()).isBefore(controller.clock().instant())) {
                         logger.log(WARNING, "CA signed certificate for app not available to config server within " + timeouts.endpointCertificate());
                         return Optional.of(RunStatus.endpointCertificateTimeout);
                     }
                     return result;
-                case ACTIVATION_CONFLICT:
-                case APPLICATION_LOCK_FAILURE:
-                case CONFIG_NOT_CONVERGED:
+                }
+                case ACTIVATION_CONFLICT, APPLICATION_LOCK_FAILURE, CONFIG_NOT_CONVERGED -> {
                     logger.log("Deployment failed with possibly transient error " + e.code() +
                                ", will retry: " + e.getMessage());
                     return result;
-                case INTERNAL_SERVER_ERROR:
+                }
+                case INTERNAL_SERVER_ERROR -> {
                     // Log only error code, to avoid exposing internal data in error message
                     logger.log("Deployment failed with possibly transient error " + e.code() + ", will retry");
                     return result;
-                case LOAD_BALANCER_NOT_READY:
-                case PARENT_HOST_NOT_READY:
+                }
+                case LOAD_BALANCER_NOT_READY, PARENT_HOST_NOT_READY -> {
                     logger.log(e.message()); // Consider splitting these messages in summary and details, on config server.
                     return result;
-                case NODE_ALLOCATION_FAILURE:
+                }
+                case NODE_ALLOCATION_FAILURE -> {
                     logger.log(e.message());
                     return controller.system().isCd() && startTime.plus(timeouts.capacity()).isAfter(controller.clock().instant())
                            ? result
                            : Optional.of(nodeAllocationFailure);
-                case INVALID_APPLICATION_PACKAGE:
+                }
+                case INVALID_APPLICATION_PACKAGE -> {
                     logger.log(WARNING, e.getMessage());
                     return Optional.of(invalidApplication);
-                case BAD_REQUEST:
+                }
+                case BAD_REQUEST -> {
                     logger.log(WARNING, e.getMessage());
                     return Optional.of(deploymentFailed);
+                }
             }
 
             throw e;
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 81fb72e19fd..35483309cae 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
@@ -54,16 +54,14 @@ import com.yahoo.vespa.hosted.controller.LockedTenant;
 import com.yahoo.vespa.hosted.controller.NotExistsException;
 import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ProtonMetrics;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RestartAction;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ServiceInfo;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.integration.aws.TenantRoles;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Cluster;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeRepository;
@@ -75,11 +73,9 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.RestartFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore;
-import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 import com.yahoo.vespa.hosted.controller.api.role.Role;
 import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
 import com.yahoo.vespa.hosted.controller.api.role.SecurityContext;
-import com.yahoo.vespa.hosted.controller.application.ActivateResult;
 import com.yahoo.vespa.hosted.controller.application.AssignedRotation;
 import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
@@ -331,16 +327,16 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/submit")) return submit(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return trigger(appIdFromPath(path), jobTypeFromPath(path), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/pause")) return pause(appIdFromPath(path), jobTypeFromPath(path));
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/deploy")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindex")) return reindex(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/reindexing")) return enableReindexing(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/restart")) return restart(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/suspend")) return suspend(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), true);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/access/support")) return allowSupportAccess(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/node/{node}/service-dump")) return requestServiceDump(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("node"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/deploy")) return deploy(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
+        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/deploy")) return deploySystemApplication(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); // legacy synonym of the above
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/restart")) return restart(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         return ErrorResponse.notFoundError("Nothing at " + path);
     }
@@ -2283,7 +2279,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
-    private HttpResponse deploy(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
+    private HttpResponse deploySystemApplication(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationId applicationId = ApplicationId.from(tenantName, applicationName, instanceName);
         ZoneId zone = requireZone(environment, region);
 
@@ -2295,18 +2291,18 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
 
         // Resolve system application
         Optional<SystemApplication> systemApplication = SystemApplication.matching(applicationId);
-        if (systemApplication.isEmpty() || !systemApplication.get().hasApplicationPackage()) {
+        if (systemApplication.isEmpty() || ! systemApplication.get().hasApplicationPackage()) {
             return ErrorResponse.badRequest("Deployment of " + applicationId + " is not supported through this API");
         }
 
         // Make it explicit that version is not yet supported here
         String vespaVersion = deployOptions.field("vespaVersion").asString();
-        if (!vespaVersion.isEmpty() && !vespaVersion.equals("null")) {
+        if ( ! vespaVersion.isEmpty()) {
             return ErrorResponse.badRequest("Specifying version for " + applicationId + " is not permitted");
         }
 
-        // To avoid second guessing the orchestrated upgrades of system applications
-        // we don't allow to deploy these during an system upgrade (i.e when new vespa is being rolled out)
+        // To avoid second guessing the orchestrated upgrades of system applications we don't allow
+        // deploying these during a system upgrade, i.e., when a new Vespa version is being rolled out
         VersionStatus versionStatus = controller.readVersionStatus();
         if (versionStatus.isUpgrading()) {
             throw new IllegalArgumentException("Deployment of system applications during a system upgrade is not allowed");
@@ -2315,14 +2311,26 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
         if (systemVersion.isEmpty()) {
             throw new IllegalArgumentException("Deployment of system applications is not permitted until system version is determined");
         }
-        ActivateResult result = controller.applications()
-                                          .deploySystemApplicationPackage(systemApplication.get(), zone, systemVersion.get().versionNumber());
-        return new SlimeJsonResponse(toSlime(result));
+        DeploymentResult result = controller.applications()
+                                            .deploySystemApplicationPackage(systemApplication.get(), zone, systemVersion.get().versionNumber());
+        Slime slime = new Slime();
+        Cursor root = slime.setObject();
+        root.setString("message", "Deployed " + systemApplication.get() + " in " + zone + " on " + systemVersion.get().versionNumber());
+
+        Cursor logArray = root.setArray("prepareMessages");
+        for (LogEntry logMessage : result.log()) {
+            Cursor logObject = logArray.addObject();
+            logObject.setLong("time", logMessage.epochMillis());
+            logObject.setString("level", logMessage.level().getName());
+            logObject.setString("message", logMessage.message());
+        }
+
+        return new SlimeJsonResponse(slime);
     }
 
     private HttpResponse deleteTenant(String tenantName, HttpRequest request) {
         boolean forget = request.getBooleanProperty("forget");
-        if (forget && !isOperator(request))
+        if (forget && ! isOperator(request))
             return ErrorResponse.forbidden("Only operators can forget a tenant");
 
         controller.tenants().delete(TenantName.from(tenantName),
@@ -2777,55 +2785,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler {
                                          request.getUri()).toString());
     }
 
-    private Slime toSlime(ActivateResult result) {
-        Slime slime = new Slime();
-        Cursor object = slime.setObject();
-        object.setString("revisionId", result.revisionId().id());
-        object.setLong("applicationZipSize", result.applicationZipSizeBytes());
-        Cursor logArray = object.setArray("prepareMessages");
-        if (result.prepareResponse().log != null) {
-            for (Log logMessage : result.prepareResponse().log) {
-                Cursor logObject = logArray.addObject();
-                logObject.setLong("time", logMessage.time);
-                logObject.setString("level", logMessage.level);
-                logObject.setString("message", logMessage.message);
-            }
-        }
-
-        Cursor changeObject = object.setObject("configChangeActions");
-
-        Cursor restartActionsArray = changeObject.setArray("restart");
-        for (RestartAction restartAction : result.prepareResponse().configChangeActions.restartActions) {
-            Cursor restartActionObject = restartActionsArray.addObject();
-            restartActionObject.setString("clusterName", restartAction.clusterName);
-            restartActionObject.setString("clusterType", restartAction.clusterType);
-            restartActionObject.setString("serviceType", restartAction.serviceType);
-            serviceInfosToSlime(restartAction.services, restartActionObject.setArray("services"));
-            stringsToSlime(restartAction.messages, restartActionObject.setArray("messages"));
-        }
-
-        Cursor refeedActionsArray = changeObject.setArray("refeed");
-        for (RefeedAction refeedAction : result.prepareResponse().configChangeActions.refeedActions) {
-            Cursor refeedActionObject = refeedActionsArray.addObject();
-            refeedActionObject.setString("name", refeedAction.name);
-            refeedActionObject.setString("documentType", refeedAction.documentType);
-            refeedActionObject.setString("clusterName", refeedAction.clusterName);
-            serviceInfosToSlime(refeedAction.services, refeedActionObject.setArray("services"));
-            stringsToSlime(refeedAction.messages, refeedActionObject.setArray("messages"));
-        }
-        return slime;
-    }
-
-    private void serviceInfosToSlime(List<ServiceInfo> serviceInfoList, Cursor array) {
-        for (ServiceInfo serviceInfo : serviceInfoList) {
-            Cursor serviceInfoObject = array.addObject();
-            serviceInfoObject.setString("serviceName", serviceInfo.serviceName);
-            serviceInfoObject.setString("serviceType", serviceInfo.serviceType);
-            serviceInfoObject.setString("configId", serviceInfo.configId);
-            serviceInfoObject.setString("hostName", serviceInfo.hostName);
-        }
-    }
-
     private void stringsToSlime(List<String> strings, Cursor array) {
         for (String string : strings)
             array.addString(string);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
index ce7fa5784e8..8cf861ff963 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
@@ -124,15 +124,12 @@ public class ApplicationPackageTest {
 
         // services.xml is changed -> different bundle hash
         assertNotEquals(originalPackage.bundleHash(), changedServices.bundleHash());
-        assertNotEquals(originalPackage.hash(), changedServices.hash());
 
         // deployment.xml is changed, with real changes -> different bundle hash
         assertNotEquals(originalPackage.bundleHash(), changedDeploymentXml.bundleHash());
-        assertNotEquals(originalPackage.hash(), changedDeploymentXml.hash());
 
         // deployment.xml is changed, but only deployment orchestration settings -> same bundle hash
         assertEquals(originalPackage.bundleHash(), similarDeploymentXml.bundleHash());
-        assertNotEquals(originalPackage.hash(), similarDeploymentXml.hash());
     }
 
     private static Map<String, String> unzip(byte[] zip) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index 8810e09dd40..7687a1561a4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -412,8 +412,6 @@ public class InternalStepRunnerTest {
         tester.runner().run(); // Job run order determined by JobType enum order per application.
         tester.configServer().convergeServices(app.instanceId(), zone);
         assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.installReal));
-        assertEquals(applicationPackage().hash(), tester.configServer().application(app.instanceId(), zone).get().applicationPackage().hash());
-        assertEquals(otherPackage.hash(), tester.configServer().application(app.instanceId(), DeploymentContext.perfUsEast3.zone()).get().applicationPackage().hash());
 
         tester.configServer().setVersion(version, app.instanceId(), zone);
         tester.runner().run();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
index 5a8d6a45e43..5b141716eaa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java
@@ -22,20 +22,17 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ProtonMetrics;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.integration.LogEntry;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ApplicationReindexing.Status;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Cluster;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint;
+import com.yahoo.vespa.hosted.controller.api.integration.configserver.DeploymentResult;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.NodeFilter;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ProxyResponse;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.QuotaUsage;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence;
@@ -51,7 +48,6 @@ import java.io.InputStream;
 import java.net.URI;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -67,6 +63,7 @@ import java.util.function.Consumer;
 import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow;
 import static com.yahoo.config.provision.NodeResources.StorageType.remote;
@@ -89,7 +86,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     private final Set<DeploymentId> suspendedApplications = new HashSet<>();
     private final Map<ZoneId, Set<LoadBalancer>> loadBalancers = new HashMap<>();
     private final Set<Environment> deferLoadBalancerProvisioning = new HashSet<>();
-    private final Map<DeploymentId, List<Log>> warnings = new HashMap<>();
+    private final Map<DeploymentId, List<DeploymentResult.LogEntry>> warnings = new HashMap<>();
     private final Map<DeploymentId, Set<ContainerEndpoint>> containerEndpoints = new HashMap<>();
     private final Map<DeploymentId, List<ClusterMetrics>> clusterMetrics = new HashMap<>();
     private final Map<DeploymentId, TestReport> testReport = new HashMap<>();
@@ -266,15 +263,13 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
     }
 
     public void generateWarnings(DeploymentId deployment, int count) {
-        List<Log> logs = new ArrayList<>(count);
-        for (int i = 0; i < count; i++) {
-            Log log = new Log();
-            log.time = Instant.now().toEpochMilli();
-            log.level = Level.WARNING.getName();
-            log.message = "log message " + (count + 1) + " generated by unit test";
-            logs.add(log);
-        }
-        warnings.put(deployment, List.copyOf(logs));
+        warnings.put(deployment,
+                     IntStream.rangeClosed(1, count)
+                              .mapToObj(i -> new DeploymentResult.LogEntry(Instant.now().toEpochMilli(),
+                                                                           "log message " + i + " generated by unit test",
+                                                                           Level.WARNING,
+                                                                           false))
+                              .toList());
     }
 
     public Map<DeploymentId, Set<ContainerEndpoint>> containerEndpoints() {
@@ -407,6 +402,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
                                                                    Optional.of("dns-zone-1"))));
         }
 
+        // TODO jonmv: compute on deploy, not when getting the result.
         return () -> {
             Application application = applications.get(id);
             application.activate();
@@ -428,12 +424,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
                                                                                                                  1))
                                                                       .collect(Collectors.toList())));
 
-            PrepareResponse prepareResponse = new PrepareResponse();
-            prepareResponse.message = "foo";
-            prepareResponse.configChangeActions = new ConfigChangeActions(List.of(), List.of(), List.of());
-            prepareResponse.tenant = new TenantId("tenant");
-            prepareResponse.log = warnings.getOrDefault(id, Collections.emptyList());
-            return prepareResponse;
+            return new DeploymentResult("foo", warnings.getOrDefault(id, List.of()));
         };
     }
 
@@ -549,9 +540,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
 
     @Override
     public QuotaUsage getQuotaUsage(DeploymentId deploymentId) {
-        var q = new QuotaUsage();
-        q.rate = 42.42;
-        return q;
+        return new QuotaUsage(42);
     }
 
     @Override
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
index f2736102968..5a037ad8201 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deploy-result.json
@@ -1,9 +1,4 @@
 {
-  "revisionId": "da39a3ee5e6b4b0d3255bfef95601890afd80709",
-  "applicationZipSize": 0,
-  "prepareMessages": [ ],
-  "configChangeActions": {
-    "restart": [ ],
-    "refeed": [ ]
-  }
+  "message": "Deployed system application hosted-vespa.routing of type proxy in prod.us-central-1 on 6.2",
+  "prepareMessages": [ ]
 }