Merge pull request #20264 from vespa-engine/hmusum/cleanup

Hmusum/cleanup [run-systemtest]

5 files changed, 28 insertions, 57 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
index 78ffc8667fe..dd4706461ad 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
@@ -22,6 +22,7 @@ public interface FileDistribution {
      */
     void startDownload(String hostName, int port, Set<FileReference> fileReferences);
 
-    File getFileReferencesDir();
+    // TODO: Remove when 7.508 is latest version in use
+    default File getFileReferencesDir() {return null; }
 
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionFactory.java
index 3e3c7066927..1027cc6a237 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionFactory.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionFactory.java
@@ -32,7 +32,7 @@ public class FileDistributionFactory implements AutoCloseable {
     }
 
     public FileDistribution createFileDistribution() {
-        return new FileDistributionImpl(getFileReferencesDir(), supervisor);
+        return new FileDistributionImpl(supervisor);
     }
 
     public AddFileInterface createFileManager(File applicationDir) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
index 605f5924e68..abb8a3e8487 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
@@ -10,7 +10,6 @@ import com.yahoo.jrt.StringArray;
 import com.yahoo.jrt.Supervisor;
 import com.yahoo.jrt.Target;
 
-import java.io.File;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -24,26 +23,21 @@ public class FileDistributionImpl implements FileDistribution, RequestWaiter {
     private final static double rpcTimeout = 1.0;
 
     private final Supervisor supervisor;
-    private final File fileReferencesDir;
 
-    public FileDistributionImpl(File fileReferencesDir, Supervisor supervisor) {
-        this.fileReferencesDir = fileReferencesDir;
+    public FileDistributionImpl(Supervisor supervisor) {
         this.supervisor = supervisor;
     }
 
+    /**
+     * Notifies client which file references it should start downloading. It's OK if the call does not succeed,
+     * as this is just a hint to the client to start downloading. Currently the only client is the config server
+     *
+     * @param hostName       host which should be notified about file references to download
+     * @param port           port which should be used when notifying
+     * @param fileReferences set of file references to start downloading
+     */
     @Override
     public void startDownload(String hostName, int port, Set<FileReference> fileReferences) {
-         startDownloadingFileReferences(hostName, port, fileReferences);
-    }
-
-    @Override
-    public File getFileReferencesDir() {
-        return fileReferencesDir;
-    }
-
-    // Notifies client which file references it should start downloading. It's OK if the call does not succeed,
-    // as this is just a hint to the client to start downloading. Currently the only client is the config server
-    private void startDownloadingFileReferences(String hostName, int port, Set<FileReference> fileReferences) {
         Target target = supervisor.connect(new Spec(hostName, port));
         Request request = new Request("filedistribution.setFileReferencesToDownload");
         request.setContext(target);
@@ -52,7 +46,6 @@ public class FileDistributionImpl implements FileDistribution, RequestWaiter {
         target.invokeAsync(request, rpcTimeout, this);
     }
 
-
     @Override
     public void handleRequestDone(Request req) {
         Target target = (Target) req.getContext();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistribution.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistribution.java
deleted file mode 100644
index 99889f38bb9..00000000000
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistribution.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.config.server.filedistribution;
-
-import com.yahoo.config.FileReference;
-import com.yahoo.config.model.api.FileDistribution;
-
-import java.io.File;
-import java.util.Set;
-
-/**
- * @author Ulf Lilleengen
- */
-public class MockFileDistribution implements FileDistribution {
-    private final File fileReferencesDir;
-
-    MockFileDistribution(File fileReferencesDir) {
-        this.fileReferencesDir = fileReferencesDir;
-    }
-
-    @Override
-    public void startDownload(String hostName, int port, Set<FileReference> fileReferences) {}
-
-    @Override
-    public File getFileReferencesDir() { return fileReferencesDir; }
-
-}
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
index ce592c3282a..aaacc9f69e0 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
@@ -127,7 +127,8 @@ public class SessionPreparer {
             AllocatedHosts allocatedHosts = preparation.buildModels(now);
             preparation.makeResult(allocatedHosts);
             if ( ! params.isDryRun()) {
-                preparation.writeStateZK();
+                FileReference fileReference = preparation.startDistributionOfApplicationPackage();
+                preparation.writeStateZK(fileReference);
                 preparation.writeEndpointCertificateMetadataZK();
                 preparation.writeContainerEndpointsZK();
             }
@@ -226,16 +227,18 @@ public class SessionPreparer {
             }
         }
 
-        Optional<FileReference> distributedApplicationPackage() {
+        FileReference startDistributionOfApplicationPackage() {
             FileReference fileReference = fileRegistry.addApplicationPackage();
             FileDistribution fileDistribution = fileDistributionFactory.createFileDistribution();
-            log.log(Level.FINE, () -> "Distribute application package for " + applicationId + " ("  + fileReference + ") to other config servers");
-            properties.configServerSpecs().stream()
-                    .filter(spec -> ! spec.getHostName().equals(HostName.getLocalhost()))
-                    .forEach(spec -> fileDistribution.startDownload(spec.getHostName(), spec.getConfigServerPort(), Set.of(fileReference)));
-
-            checkTimeout("distributeApplicationPackage");
-            return Optional.of(fileReference);
+            log.log(Level.FINE, () -> "Ask other config servers to download application package for " +
+                    applicationId + " (" + fileReference + ")");
+            properties.configServerSpecs()
+                      .stream()
+                      .filter(spec -> !spec.getHostName().equals(HostName.getLocalhost()))
+                      .forEach(spec -> fileDistribution.startDownload(spec.getHostName(), spec.getConfigServerPort(), Set.of(fileReference)));
+
+            checkTimeout("startDistributionOfApplicationPackage");
+            return fileReference;
         }
 
         void preprocess() {
@@ -261,12 +264,12 @@ public class SessionPreparer {
             checkTimeout("making result from models");
         }
 
-        void writeStateZK() {
+        void writeStateZK(FileReference filereference) {
             log.log(Level.FINE, "Writing application package state to zookeeper");
             writeStateToZooKeeper(sessionZooKeeperClient,
                                   preprocessedApplicationPackage,
                                   applicationId,
-                                  distributedApplicationPackage(),
+                                  filereference,
                                   dockerImageRepository,
                                   vespaVersion,
                                   logger,
@@ -306,7 +309,7 @@ public class SessionPreparer {
     private void writeStateToZooKeeper(SessionZooKeeperClient zooKeeperClient,
                                        ApplicationPackage applicationPackage,
                                        ApplicationId applicationId,
-                                       Optional<FileReference> distributedApplicationPackage,
+                                       FileReference fileReference,
                                        Optional<DockerImage> dockerImageRepository,
                                        Version vespaVersion,
                                        DeployLogger deployLogger,
@@ -321,7 +324,7 @@ public class SessionPreparer {
             zkDeployer.deploy(applicationPackage, fileRegistryMap, allocatedHosts);
             // Note: When changing the below you need to also change similar calls in SessionRepository.createSessionFromExisting()
             zooKeeperClient.writeApplicationId(applicationId);
-            zooKeeperClient.writeApplicationPackageReference(distributedApplicationPackage);
+            zooKeeperClient.writeApplicationPackageReference(Optional.of(fileReference));
             zooKeeperClient.writeVespaVersion(vespaVersion);
             zooKeeperClient.writeDockerImageRepository(dockerImageRepository);
             zooKeeperClient.writeAthenzDomain(athenzDomain);