TW-57690: now when we load mirrors mapping file we ignore non existing directories...
authorPavel Sher <pavel.sher@jetbrains.com>
Fri, 4 Jan 2019 15:16:13 +0000 (16:16 +0100)
committerPavel Sher <pavel.sher@jetbrains.com>
Fri, 4 Jan 2019 15:16:27 +0000 (16:16 +0100)
git-agent/src/jetbrains/buildServer/buildTriggers/vcs/git/agent/AgentMirrorCleaner.java
git-common/src/jetbrains/buildServer/buildTriggers/vcs/git/MirrorManager.java
git-common/src/jetbrains/buildServer/buildTriggers/vcs/git/MirrorManagerImpl.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/RepositoryManagerImpl.java
git-tests/src/jetbrains/buildServer/buildTriggers/vcs/git/tests/AgentMirrorCleanerTest.java
git-tests/src/jetbrains/buildServer/buildTriggers/vcs/git/tests/MirrorManagerTest.java

index 6fa57b06f8eb464a6be5c8518cc7f43363eab9dc..fcb40254b537872b41e420638df1a3e1f8901074 100644 (file)
@@ -38,7 +38,7 @@ import java.util.Set;
 
 public class AgentMirrorCleaner implements DirectoryCleanersProvider {
 
-  private final static Logger ourLog = Logger.getInstance(AgentMirrorCleaner.class.getName());
+  private final static Logger LOG = Logger.getInstance(AgentMirrorCleaner.class.getName());
   private final MirrorManager myMirrorManager;
 
   public AgentMirrorCleaner(@NotNull MirrorManager mirrorManager) {
@@ -53,15 +53,23 @@ public class AgentMirrorCleaner implements DirectoryCleanersProvider {
   public void registerDirectoryCleaners(@NotNull DirectoryCleanersProviderContext context,
                                         @NotNull DirectoryCleanersRegistry registry) {
     Set<String> repositoriesUsedInBuild = getRunningBuildRepositories(context);
-    for (Map.Entry<String, File> entry : myMirrorManager.getMappings().entrySet()) {
+    final Map<String, File> mappings = myMirrorManager.getMappings();
+    for (Map.Entry<String, File> entry : mappings.entrySet()) {
       String repository = entry.getKey();
       File mirror = entry.getValue();
       if (!repositoriesUsedInBuild.contains(repository)) {
+
+        if (!mirror.isDirectory()) {
+          myMirrorManager.removeMirrorDir(mirror);
+          LOG.debug("Found non existing mirror directory: " + mirror.getAbsolutePath() + ", removed it from the list of mirrors");
+          continue;
+        }
+
         if (isCleanupEnabled(mirror)) {
-          ourLog.debug("Register cleaner for mirror " + mirror.getAbsolutePath());
+          LOG.debug("Register cleaner for mirror " + mirror.getAbsolutePath());
           registry.addCleaner(mirror, new Date(myMirrorManager.getLastUsedTime(mirror)));
         } else {
-          ourLog.debug("Clean-up is disabled in " + repository + " (" + mirror.getName() + ")");
+          LOG.debug("Clean-up is disabled in " + repository + " (" + mirror.getName() + ")");
         }
       }
     }
@@ -76,10 +84,10 @@ public class AgentMirrorCleaner implements DirectoryCleanersProvider {
       try {
         GitVcsRoot gitRoot = new GitVcsRoot(myMirrorManager, root);
         String repositoryUrl = gitRoot.getRepositoryFetchURL().toString();
-        ourLog.debug("Repository " + repositoryUrl + " is used in the build, its mirror won't be cleaned");
+        LOG.debug("Repository " + repositoryUrl + " is used in the build, its mirror won't be cleaned");
         repositories.add(gitRoot.getRepositoryFetchURL().toString());
       } catch (VcsException e) {
-        ourLog.warn("Error while creating git root " + root.getName() + ". If the root has a mirror on agent, the mirror might be cleaned", e);
+        LOG.warn("Error while creating git root " + root.getName() + ". If the root has a mirror on agent, the mirror might be cleaned", e);
       }
     }
     return repositories;
index e88a19d825305555384abb4284f88534c6313a85..cc6afb76f94795fef38506dcdbdd563279ffb37a 100644 (file)
@@ -49,6 +49,12 @@ public interface MirrorManager {
    */
   void invalidate(@NotNull File dir);
 
+  /**
+   * Removes mirror directory
+   * @param dir directory to remove
+   */
+  void removeMirrorDir(@NotNull final File dir);
+
 
   @NotNull
   Map<String, File> getMappings();
index 061c6634c8a34fda96dc87ea1c8fc2866fde7556..236ec7f27b6fb03eb4f2a0e537f46e6f8ce79877 100644 (file)
@@ -80,6 +80,16 @@ public class MirrorManagerImpl implements MirrorManager {
     }
   }
 
+  public void removeMirrorDir(@NotNull final File dir) {
+    synchronized (myLock) {
+      List<String> urlsMappedToDir = getUrlsMappedToDir(dir);
+      for (String url : urlsMappedToDir) {
+        myMirrorMap.remove(url);
+      }
+      saveMappingToFile();
+      FileUtil.delete(dir);
+    }
+  }
 
   @NotNull
   public Map<String, File> getMappings() {
@@ -250,6 +260,8 @@ public class MirrorManagerImpl implements MirrorManager {
 
   private void readMappings() {
     synchronized (myLock) {
+      boolean mappingsFileHasObsoleteDirs = false;
+
       for (String line : readLines(myMapFile)) {
         int separatorIndex = line.lastIndexOf(" = ");
         if (separatorIndex == -1) {
@@ -258,6 +270,13 @@ public class MirrorManagerImpl implements MirrorManager {
         } else {
           String url = line.substring(0, separatorIndex);
           String dirName = line.substring(separatorIndex + 3);
+
+          if (!new File(myBaseMirrorsDir, dirName).isDirectory()) {
+            LOG.info("Skip mapping " + line + ": " + dirName + " because the specified directory does not exist");
+            mappingsFileHasObsoleteDirs = true;
+            continue;
+          }
+
           if (myMirrorMap.values().contains(dirName)) {
             LOG.error("Skip mapping " + line + ": " + dirName + " is used for url other than " + url);
           } else {
@@ -265,6 +284,10 @@ public class MirrorManagerImpl implements MirrorManager {
           }
         }
       }
+
+      if (mappingsFileHasObsoleteDirs) {
+        saveMappingToFile();
+      }
     }
   }
 
@@ -335,11 +358,12 @@ public class MirrorManagerImpl implements MirrorManager {
 
   @NotNull
   private File[] findRepositoryDirs() {
-    return myBaseMirrorsDir.listFiles(new FileFilter() {
+    final File[] dirs = myBaseMirrorsDir.listFiles(new FileFilter() {
       public boolean accept(File f) {
         return f.isDirectory() && new File(f, "config").exists();
       }
     });
+    return dirs != null ? dirs : new File[0];
   }
 
 
index dac9175853fd149264da5c8a908284a3fb7ebd3e..5009224dc01c31b7b9e7853f17e0101fb872d06d 100644 (file)
@@ -100,6 +100,11 @@ public final class RepositoryManagerImpl implements RepositoryManager {
     myMirrorManager.invalidate(dir);
   }
 
+  @Override
+  public void removeMirrorDir(@NotNull final File dir) {
+    myMirrorManager.removeMirrorDir(dir);
+  }
+
 
   @NotNull
   public Map<String, File> getMappings() {
index ac5a4f590c838dc91dc7c10a1ef3df4e4e43826e..00cef2fcb7ac8195d7bc0742bd1b65e6538f64b7 100644 (file)
@@ -16,6 +16,7 @@
 
 package jetbrains.buildServer.buildTriggers.vcs.git.tests;
 
+import jetbrains.buildServer.TempFiles;
 import jetbrains.buildServer.agent.AgentRunningBuild;
 import jetbrains.buildServer.agent.DirectoryCleanersProviderContext;
 import jetbrains.buildServer.agent.DirectoryCleanersRegistry;
@@ -31,6 +32,7 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
@@ -52,25 +54,30 @@ public class AgentMirrorCleanerTest {
   }
 
 
-  public void should_register_mirrors_not_used_in_current_build() {
-    final DirectoryCleanersRegistry registry = myContext.mock(DirectoryCleanersRegistry.class);
-    final File r3mirror = new File("r3");
-    final File r4mirror = new File("r4");
-    final Date r3lastAccess = Dates.makeDate(2012, 10, 29);
-    final Date r4lastAccess = Dates.makeDate(2012, 10, 27);
-    List<String> repositoriesInBuild = asList("git://some.org/r1", "git://some.org/r2");
-    myContext.checking(new Expectations() {{
-      one(myMirrorManager).getMappings(); will(returnValue(map("git://some.org/r1", new File("r1"),
-                                                               "git://some.org/r2", new File("r2"),
-                                                               "git://some.org/r3", r3mirror,
-                                                               "git://some.org/r4", r4mirror)));
-      one(myMirrorManager).getLastUsedTime(r3mirror); will(returnValue(r3lastAccess.getTime()));
-      one(myMirrorManager).getLastUsedTime(r4mirror); will(returnValue(r4lastAccess.getTime()));
+  public void should_register_mirrors_not_used_in_current_build() throws IOException {
+    TempFiles tmpFiles = new TempFiles();
+    try {
+      final DirectoryCleanersRegistry registry = myContext.mock(DirectoryCleanersRegistry.class);
+      final File r3mirror = tmpFiles.createTempDir();
+      final File r4mirror = tmpFiles.createTempDir();
+      final Date r3lastAccess = Dates.makeDate(2012, 10, 29);
+      final Date r4lastAccess = Dates.makeDate(2012, 10, 27);
+      List<String> repositoriesInBuild = asList("git://some.org/r1", "git://some.org/r2");
+      myContext.checking(new Expectations() {{
+        one(myMirrorManager).getMappings(); will(returnValue(map("git://some.org/r1", tmpFiles.createTempDir(),
+                                                                 "git://some.org/r2", tmpFiles.createTempDir(),
+                                                                 "git://some.org/r3", r3mirror,
+                                                                 "git://some.org/r4", r4mirror)));
+        one(myMirrorManager).getLastUsedTime(r3mirror); will(returnValue(r3lastAccess.getTime()));
+        one(myMirrorManager).getLastUsedTime(r4mirror); will(returnValue(r4lastAccess.getTime()));
 
-      one(registry).addCleaner(r3mirror, r3lastAccess);
-      one(registry).addCleaner(r4mirror, r4lastAccess);
-    }});
-    myAgentMirrorCleaner.registerDirectoryCleaners(createCleanerContext(repositoriesInBuild), registry);
+        one(registry).addCleaner(r3mirror, r3lastAccess);
+        one(registry).addCleaner(r4mirror, r4lastAccess);
+      }});
+      myAgentMirrorCleaner.registerDirectoryCleaners(createCleanerContext(repositoriesInBuild), registry);
+    } finally {
+      tmpFiles.cleanup();
+    }
   }
 
 
index 46be8aba2b3cd3ce90d50991fd3fb92f2acd6548..47042e9b26d9ac3232701c7f9e48bd911131fc49 100644 (file)
@@ -19,6 +19,7 @@ package jetbrains.buildServer.buildTriggers.vcs.git.tests;
 import jetbrains.buildServer.TempFiles;
 import jetbrains.buildServer.buildTriggers.vcs.git.*;
 import jetbrains.buildServer.serverSide.ServerPaths;
+import jetbrains.buildServer.util.FileUtil;
 import org.eclipse.jgit.transport.URIish;
 import org.jetbrains.annotations.NotNull;
 import org.testng.annotations.AfterMethod;
@@ -93,6 +94,23 @@ public class MirrorManagerTest {
     }
   }
 
+  public void should_ignore_non_existing_directories() throws Exception {
+    File baseMirrorsDir = myConfig.getCachesDir();
+    File map = new File(baseMirrorsDir, "map");
+
+    FileUtil.writeFileAndReportErrors(map, "git://some.org/repository1.git = git-11111111.git\n" +
+                                           "git://some.org/repository2.git = git-22222222.git");
+
+    getRepository(new File(baseMirrorsDir, "git-22222222.git"), new URIish("git://some.org/repository2.git"));
+
+    MirrorManager mirrorManager = new MirrorManagerImpl(myConfig, new HashCalculatorImpl());
+    assertEquals(1, mirrorManager.getMappings().size());
+    assertTrue(mirrorManager.getMappings().containsKey("git://some.org/repository2.git"));
+
+    String mapping = FileUtil.readText(map);
+    assertFalse(mapping.contains("git-11111111.git"));
+  }
+
 
   public void should_give_different_dirs_for_same_url_if_dir_was_invalidated() {
     MirrorManager mirrorManager = new MirrorManagerImpl(myConfig, new HashCalculatorImpl());