Use FileDirectory when deleting unused file references

7 files changed, 47 insertions, 35 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index 29590beed19..3ae083d6f6b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -57,6 +57,7 @@ import com.yahoo.vespa.config.server.configchange.RestartActions;
 import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger;
 import com.yahoo.vespa.config.server.deploy.Deployment;
 import com.yahoo.vespa.config.server.deploy.InfraDeployerProvider;
+import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.config.server.http.InternalServerException;
 import com.yahoo.vespa.config.server.http.LogRetriever;
 import com.yahoo.vespa.config.server.http.SecretStoreValidator;
@@ -594,22 +595,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return fileDistributionStatus.status(getApplication(applicationId), timeout);
     }
 
-    public List<String> deleteUnusedFileDistributionReferences(File fileReferencesPath, Duration keepFileReferencesDuration) {
-        if (!fileReferencesPath.isDirectory()) throw new RuntimeException(fileReferencesPath + " is not a directory");
-
+    public List<String> deleteUnusedFileDistributionReferences(FileDirectory fileDirectory, Duration keepFileReferencesDuration) {
         Set<String> fileReferencesInUse = getFileReferencesInUse();
         log.log(Level.FINE, () -> "File references in use : " + fileReferencesInUse);
         Instant instant = clock.instant().minus(keepFileReferencesDuration);
         log.log(Level.FINE, () -> "Remove unused file references last modified before " + instant);
 
-        List<String> fileReferencesToDelete = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, instant);
+        List<String> fileReferencesToDelete = sortedUnusedFileReferences(fileDirectory.getRoot(), fileReferencesInUse, instant);
         if (fileReferencesToDelete.size() > 0) {
             log.log(Level.FINE, () -> "Will delete file references not in use: " + fileReferencesToDelete);
-            fileReferencesToDelete.forEach(fileReference -> {
-                File file = new File(fileReferencesPath, fileReference);
-                if ( ! IOUtils.recursiveDeleteDir(file))
-                    log.log(Level.WARNING, "Could not delete " + file.getAbsolutePath());
-            });
+            fileReferencesToDelete.forEach(fileReference -> fileDirectory.delete(new FileReference(fileReference)));
         }
         return fileReferencesToDelete;
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
index 5eb0f830ef6..372b7a44a0b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java
@@ -10,6 +10,7 @@ import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.TransientException;
 import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.jdisc.state.StateMonitor;
+import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.config.server.maintenance.ConfigServerMaintenance;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
@@ -71,14 +72,16 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
     @SuppressWarnings("unused") //  Injected component
     @Inject
     public ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
-                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus) {
-        this(applicationRepository, server, versionState, stateMonitor, vipStatus, EXIT_JVM, vipStatusMode(applicationRepository));
+                                 VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
+                                 FileDirectory fileDirectory) {
+        this(applicationRepository, server, versionState, stateMonitor, vipStatus, EXIT_JVM,
+             vipStatusMode(applicationRepository), fileDirectory);
     }
 
     protected ConfigServerBootstrap(ApplicationRepository applicationRepository, RpcServer server,
                                     VersionState versionState, StateMonitor stateMonitor, VipStatus vipStatus,
                                     RedeployingApplicationsFails exitIfRedeployingApplicationsFails,
-                                    VipStatusMode vipStatusMode) {
+                                    VipStatusMode vipStatusMode, FileDirectory fileDirectory) {
         this.applicationRepository = applicationRepository;
         this.server = server;
         this.versionState = versionState;
@@ -90,7 +93,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable
         this.exitIfRedeployingApplicationsFails = exitIfRedeployingApplicationsFails;
         this.clock = applicationRepository.clock();
         rpcServerExecutor = Executors.newSingleThreadExecutor(new DaemonThreadFactory("config server RPC server"));
-        configServerMaintenance = new ConfigServerMaintenance(applicationRepository);
+        configServerMaintenance = new ConfigServerMaintenance(applicationRepository, fileDirectory);
         configServerMaintenance.startBeforeBootstrap();
         log.log(Level.FINE, () -> "VIP status mode: " + vipStatusMode);
         initializing(vipStatusMode);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
index 020a28b9ecd..c7c907b6acf 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java
@@ -90,9 +90,7 @@ public class FileDirectory extends AbstractComponent {
         return files[0];
     }
 
-    File getRoot() {
-        return root;
-    }
+    public File getRoot() { return root; }
 
     private Long computeHash(File file) throws IOException {
         XXHash64 hasher = XXHashFactory.fastestInstance().hash64();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
index a4cc11bd22e..3adda08b9aa 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
@@ -6,6 +6,7 @@ import com.yahoo.concurrent.maintenance.Maintainer;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.ConfigServerBootstrap;
 import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker;
+import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.flags.FlagSource;
 
@@ -30,13 +31,15 @@ public class ConfigServerMaintenance {
     private final Curator curator;
     private final FlagSource flagSource;
     private final ConfigConvergenceChecker convergenceChecker;
+    private final FileDirectory fileDirectory;
 
-    public ConfigServerMaintenance(ApplicationRepository applicationRepository) {
+    public ConfigServerMaintenance(ApplicationRepository applicationRepository, FileDirectory fileDirectory) {
         this.configserverConfig = applicationRepository.configserverConfig();
         this.applicationRepository = applicationRepository;
         this.curator = applicationRepository.tenantRepository().getCurator();
         this.flagSource = applicationRepository.flagSource();
         this.convergenceChecker = applicationRepository.configConvergenceChecker();
+        this.fileDirectory = fileDirectory;
     }
 
     public void startBeforeBootstrap() {
@@ -46,8 +49,11 @@ public class ConfigServerMaintenance {
     }
 
     public void startAfterBootstrap() {
-        maintainers.add(new FileDistributionMaintainer(applicationRepository, curator,
-                                                       new DefaultTimes(configserverConfig).defaultInterval, flagSource));
+        maintainers.add(new FileDistributionMaintainer(applicationRepository,
+                                                       curator,
+                                                       new DefaultTimes(configserverConfig).defaultInterval,
+                                                       flagSource,
+                                                       fileDirectory));
         maintainers.add(new SessionsMaintainer(applicationRepository, curator, Duration.ofSeconds(30), flagSource));
         maintainers.add(new ReindexingMaintainer(applicationRepository, curator, flagSource,
                                                  Duration.ofMinutes(3), convergenceChecker, Clock.systemUTC()));
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
index 5542d24253b..0187f35cce0 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
@@ -3,11 +3,9 @@ package com.yahoo.vespa.config.server.maintenance;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.vespa.config.server.ApplicationRepository;
+import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.curator.Curator;
-import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.flags.FlagSource;
-
-import java.io.File;
 import java.time.Duration;
 
 /**
@@ -21,23 +19,24 @@ import java.time.Duration;
 public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
     private final ApplicationRepository applicationRepository;
-    private final File fileReferencesDir;
+    private final FileDirectory fileDirectory;
     private final Duration maxUnusedFileReferenceAge;
 
     FileDistributionMaintainer(ApplicationRepository applicationRepository,
                                Curator curator,
                                Duration interval,
-                               FlagSource flagSource) {
+                               FlagSource flagSource,
+                               FileDirectory fileDirectory) {
         super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval, false);
         this.applicationRepository = applicationRepository;
         ConfigserverConfig configserverConfig = applicationRepository.configserverConfig();
         this.maxUnusedFileReferenceAge = Duration.ofMinutes(configserverConfig.keepUnusedFileReferencesMinutes());
-        this.fileReferencesDir = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir()));
+        this.fileDirectory = fileDirectory;
     }
 
     @Override
     protected double maintain() {
-        applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge);
+        applicationRepository.deleteUnusedFileDistributionReferences(fileDirectory, maxUnusedFileReferenceAge);
         return 1.0;
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index 754781e16f5..74ec5e81da3 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -105,6 +105,7 @@ public class ApplicationRepositoryTest {
     private TimeoutBudget timeoutBudget;
     private Curator curator;
     private ConfigserverConfig configserverConfig;
+    private FileDirectory fileDirectory;
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -123,12 +124,13 @@ public class ApplicationRepositoryTest {
                 .fileReferencesDir(temporaryFolder.newFolder().getAbsolutePath())
                 .build();
         InMemoryFlagSource flagSource = new InMemoryFlagSource();
+        fileDirectory = new FileDirectory(configserverConfig, flagSource);
         tenantRepository = new TestTenantRepository.Builder()
                 .withClock(clock)
                 .withConfigserverConfig(configserverConfig)
                 .withCurator(curator)
                 .withFileDistributionFactory(
-                        new MockFileDistributionFactory(configserverConfig, new FileDirectory(configserverConfig, flagSource)))
+                        new MockFileDistributionFactory(configserverConfig, fileDirectory))
                 .withFlagSource(flagSource)
                 .build();
         tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT);
@@ -263,8 +265,8 @@ public class ApplicationRepositoryTest {
     }
 
     @Test
-    public void deleteUnusedFileReferences() throws IOException {
-        File fileReferencesDir = temporaryFolder.newFolder();
+    public void deleteUnusedFileReferences() {
+        File fileReferencesDir = new File(configserverConfig.fileReferencesDir());
         Duration keepFileReferencesDuration = Duration.ofSeconds(4);
 
         // Add file reference that is not in use and should be deleted (older than 'keepFileReferencesDuration')
@@ -289,10 +291,10 @@ public class ApplicationRepositoryTest {
                 .build();
 
         // TODO: Deploy an app with a bundle or file that will be a file reference, too much missing in test setup to get this working now
-        PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
-        deployApp(new File("src/test/apps/app"), prepareParams);
+        // PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
+        // deployApp(new File("src/test/apps/app"), prepareParams);
 
-        List<String> deleted = applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, keepFileReferencesDuration);
+        List<String> deleted = applicationRepository.deleteUnusedFileDistributionReferences(fileDirectory, keepFileReferencesDuration);
         Collections.sort(deleted);
         List<String> expected = new ArrayList<>(List.of("bar", "baz0", "baz1"));
         Collections.sort(expected);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
index 9b6ef7ce197..dad029e909a 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
@@ -20,10 +20,12 @@ import com.yahoo.docproc.jdisc.metric.NullMetric;
 import com.yahoo.path.Path;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.config.server.deploy.DeployTester;
+import com.yahoo.vespa.config.server.filedistribution.FileDirectory;
 import com.yahoo.vespa.config.server.rpc.RpcServer;
 import com.yahoo.vespa.config.server.version.VersionState;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -181,7 +183,13 @@ public class ConfigServerBootstrapTest {
 
         StateMonitor stateMonitor = StateMonitor.createForTesting();
         VipStatus vipStatus = createVipStatus(stateMonitor);
-        return new Bootstrapper(tester.applicationRepository(), rpcServer, versionState, stateMonitor, vipStatus, vipStatusMode);
+        return new Bootstrapper(tester.applicationRepository(),
+                                rpcServer,
+                                versionState,
+                                stateMonitor,
+                                vipStatus,
+                                vipStatusMode,
+                                new FileDirectory(tester.applicationRepository().configserverConfig(), new InMemoryFlagSource()));
     }
 
     private void waitUntil(BooleanSupplier booleanSupplier, String messageIfWaitingFails) throws InterruptedException {
@@ -269,8 +277,9 @@ public class ConfigServerBootstrapTest {
                             VersionState versionState,
                             StateMonitor stateMonitor,
                             VipStatus vipStatus,
-                            VipStatusMode vipStatusMode) {
-            super(applicationRepository, server, versionState, stateMonitor, vipStatus, CONTINUE, vipStatusMode);
+                            VipStatusMode vipStatusMode,
+                            FileDirectory fileDirectory) {
+            super(applicationRepository, server, versionState, stateMonitor, vipStatus, CONTINUE, vipStatusMode, fileDirectory);
         }
 
         @Override