Closes #1237 - Always hard-reset on initialization when pulling from …

…and pushing to the same branch (#1255)
inspectIT · Dec 22, 2021 · 9a04641 · 9a04641
1 parent 90e7636
commit 9a04641
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 29 deletions.
diff --git a/...urationserver/src/main/java/rocks/inspectit/ocelot/file/versioning/VersioningManager.java b/...urationserver/src/main/java/rocks/inspectit/ocelot/file/versioning/VersioningManager.java
@@ -34,9 +34,7 @@
 import rocks.inspectit.ocelot.file.versioning.model.WorkspaceVersion;
 
 import java.io.IOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Duration;
 import java.util.*;
 import java.util.function.Supplier;
@@ -224,8 +222,8 @@ private void setCurrentBranchToTarget() throws GitAPIException, IOException {
 
         // this is true for a fresh start of a config server instance connected to a remote git for backup
         // --> prevent unnecessary commits at startup that don't change any files (particularly when frequently restarting in, e.g., Kubernetes)
-        // otherwise, we need to properly merge files from local and two remotes
-        boolean hardReset = isWorkingDirectoryEmpty() && remoteSettings.isInitialConfigurationSync() && remoteSettings.isAutoPromotion() && areRemotesEqual(sourceRepository, targetRepository);
+        // otherwise, we need to properly merge files from two remotes
+        boolean hardReset = remoteSettings.isInitialConfigurationSync() && remoteSettings.isAutoPromotion() && areRemotesEqual(sourceRepository, targetRepository);
 
         log.info("{}-resetting current branch to '{}'.", (hardReset ? "Hard" : "Soft"), targetRepository.getBranchName());
         git.reset()
@@ -249,17 +247,6 @@ private boolean areRemotesEqual(RemoteRepositorySettings sourceRepository, Remot
         return ObjectId.isEqual(sourceId, targetId);
     }
 
-    private boolean isWorkingDirectoryEmpty() throws IOException {
-        Path filesPath = Paths.get(AbstractFileAccessor.CONFIGURATION_FILES_SUBFOLDER);
-        boolean filesEmpty = !Files.exists(filesPath) || (Files.isDirectory(filesPath) && Files.list(filesPath)
-                .count() == 0);
-
-        Path agentMappingPath = Paths.get(AbstractFileAccessor.AGENT_MAPPINGS_FILE_NAME);
-        boolean agentMappingMissing = !Files.exists(agentMappingPath);
-
-        return filesEmpty && agentMappingMissing;
-    }
-
     /**
      * Closes the {@link #git} instance of this manager.
      */

diff --git a/...ionserver/src/test/java/rocks/inspectit/ocelot/file/versioning/VersioningManagerTest.java b/...ionserver/src/test/java/rocks/inspectit/ocelot/file/versioning/VersioningManagerTest.java
@@ -1185,7 +1185,42 @@ public void mergeBranch() throws URISyntaxException, GitAPIException, IOExceptio
         }
 
         @Test
-        public void initialConfigurationSyncOneRemote() throws URISyntaxException, GitAPIException, IOException {
+        public void initialConfigurationSyncOneRemoteWithoutLocalFiles() throws URISyntaxException, GitAPIException, IOException {
+            // ////////////////////////
+            // test without local files --> should only reset to remote
+            doInitialConfigurationSyncOneRemote(false);
+
+            ObjectId targetCommitIdBeforeInitialization = repositoryOne.getRepository()
+                    .exactRef("refs/heads/remote")
+                    .getObjectId();
+            ObjectId liveCommitId = versioningManager.getLatestCommit(Branch.LIVE).get().getId();
+
+            assertThat(liveCommitId).isEqualTo(targetCommitIdBeforeInitialization); // live should be set to remote repo
+            assertThat(Files.list(tempDirectory.resolve(AbstractFileAccessor.CONFIGURATION_FILES_SUBFOLDER))).extracting(Path::getFileName)
+                    .extracting(Path::toString)
+                    .containsExactlyInAnyOrder("from_remote.yml"); // should exactly contain content of remote repo
+        }
+
+        @Test
+        public void initialConfigurationSyncOneRemoteWithLocalFiles() throws URISyntaxException, GitAPIException, IOException {
+            // ////////////////////////
+            // test with local files --> should overwrite those local files that exist in remote and keep others
+            doInitialConfigurationSyncOneRemote(true);
+
+            ObjectId targetCommitIdBeforeInitialization = repositoryOne.getRepository()
+                    .exactRef("refs/heads/remote")
+                    .getObjectId();
+            RevCommit liveCommit = versioningManager.getLatestCommit(Branch.LIVE).get();
+            ObjectId parentCommitId = liveCommit.getParent(0).getId();
+
+            assertThat(parentCommitId).isEqualTo(targetCommitIdBeforeInitialization); // live should add one commit to remote repo
+            assertThat(Files.list(tempDirectory.resolve(AbstractFileAccessor.CONFIGURATION_FILES_SUBFOLDER))).extracting(Path::getFileName)
+                    .extracting(Path::toString)
+                    .containsExactlyInAnyOrder("from_remote.yml", "from_local.yml"); // should contain content of remote repo + from_local.yml
+            assertThat(new String(Files.readAllBytes(tempDirectory.resolve("files/from_remote.yml")))).isEmpty(); // file from remote should have remote content (empty)
+        }
+
+        private void doInitialConfigurationSyncOneRemote(boolean localFiles) throws URISyntaxException, GitAPIException, IOException {
             RemoteRepositorySettings remoteRepositorySettings = RemoteRepositorySettings.builder()
                     .branchName("remote")
                     .remoteName("remote")
@@ -1208,22 +1243,13 @@ public void initialConfigurationSyncOneRemote() throws URISyntaxException, GitAP
             repositoryOne.branchCreate().setName("remote").call();
             repositoryOne.checkout().setName("remote").call();
 
-            ObjectId targetCommitIdBeforeInitialization = repositoryOne.getRepository()
-                    .exactRef("refs/heads/remote")
-                    .getObjectId();
+            if (localFiles) {
+                // add some local files that should (not) be overridden by initial synchronization
+                createTestFiles("files/from_local.yml=local", "files/from_remote.yml=local");
+            }
 
             // initialize Git
             versioningManager.initialize();
-            Git git = (Git) ReflectionTestUtils.getField(versioningManager, "git");
-
-            // ////////////////////////
-            // check state after initial synchronization
-            ObjectId liveCommitId = versioningManager.getLatestCommit(Branch.LIVE).get().getId();
-
-            assertThat(liveCommitId).isEqualTo(targetCommitIdBeforeInitialization); // live should be set to remote repo
-            assertThat(Files.list(tempDirectory.resolve(AbstractFileAccessor.CONFIGURATION_FILES_SUBFOLDER))).extracting(Path::getFileName)
-                    .extracting(Path::toString)
-                    .containsExactlyInAnyOrder("from_remote.yml"); // should exactly contain content of remote repo
         }
 
         @Test