From 44c104df2abc0d26288ba36a0e066adff4c1d33c Mon Sep 17 00:00:00 2001 From: nhtzr Date: Mon, 11 Jan 2021 16:08:01 -0600 Subject: [PATCH] Add index regen if config server has force-pull (#1593) * Add index regen if config server has force-pull jGit may fail with 'JGitInternalException: Short read of block.' Due to a truncated git index, which may happen due to external reasons. The index can be regenerated easily by deleting it and resetting git working directory. For a pull force, resetting to HEAD is more convenient since the info that may have been lost in the index is the diff in a dirty copy that we are discarding in any way. This commits adds automates those steps when git-status fails and the force-pull flag is set. Please see: - https://github.com/spinnaker/spinnaker/issues/5153 - https://bugs.eclipse.org/bugs/show_bug.cgi?id=531807 - https://www.devguerrilla.com/notes/2015/07/fuse-for-the-idle-fellow-short-read-of-block-errors-when-starting-a-container/ * Add tests Co-authored-by: Ezequiel Rosas --- .../JGitEnvironmentRepository.java | 28 +++- .../JGitEnvironmentRepositoryTests.java | 133 ++++++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java index 8b958d16..17fa6f8d 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java @@ -43,6 +43,7 @@ import org.eclipse.jgit.api.StatusCommand; import org.eclipse.jgit.api.TransportCommand; import org.eclipse.jgit.api.TransportConfigCallback; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.errors.NoRemoteRepositoryException; import org.eclipse.jgit.lib.BranchTrackingStatus; @@ -425,7 +426,15 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return false; } - Status gitStatus = git.status().call(); + Status gitStatus; + try { + gitStatus = git.status().call(); + } + catch (JGitInternalException e) { + onPullInvalidIndex(git, e); + gitStatus = git.status().call(); + } + boolean isWorkingTreeClean = gitStatus.isClean(); String originUrl = git.getRepository().getConfig().getString("remote", "origin", "url"); @@ -444,6 +453,23 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return shouldPull; } + protected void onPullInvalidIndex(Git git, JGitInternalException e) { + if (!e.getMessage().contains("Short read of block.")) { + throw e; + } + if (!this.forcePull) { + throw e; + } + try { + new File(getWorkingDirectory(), ".git/index").delete(); + git.reset().setMode(ResetType.HARD).setRef("HEAD").call(); + } + catch (GitAPIException ex) { + e.addSuppressed(ex); + throw e; + } + } + @SuppressWarnings("unchecked") private void logDirty(Status status) { Set dirties = dirties(status.getAdded(), status.getChanged(), diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java index 16a4e1f5..1765ba07 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.List; import org.apache.commons.logging.Log; +import org.assertj.core.api.Assertions; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.DeleteBranchCommand; @@ -39,6 +40,7 @@ import org.eclipse.jgit.api.StatusCommand; import org.eclipse.jgit.api.TransportConfigCallback; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.InvalidRemoteException; +import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.api.errors.NotMergedException; import org.eclipse.jgit.api.errors.TransportException; import org.eclipse.jgit.attributes.AttributesNodeProvider; @@ -330,6 +332,137 @@ public class JGitEnvironmentRepositoryTests { assertThat(shouldPull).as("shouldPull was true").isFalse(); } + @Test + public void shouldPullTruncatedIndexForcePull() throws Exception { + Git git = mock(Git.class); + StatusCommand statusCommand = mock(StatusCommand.class); + Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); + ResetCommand resetCommand = mock(ResetCommand.class); + + when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")) + .thenReturn("http://example/git"); + when(statusCommand.call()) + .thenThrow(new JGitInternalException("Short read of block.")) + .thenReturn(status); + when(status.isClean()).thenReturn(true); + when(git.reset()).thenReturn(resetCommand); + when(resetCommand.setMode(any())).thenReturn(resetCommand); + when(resetCommand.setRef(any())).thenReturn(resetCommand); + + JGitEnvironmentRepository repo = new JGitEnvironmentRepository(this.environment, + new JGitEnvironmentProperties()); + repo.setUri(""); + repo.setForcePull(true); + + boolean shouldPull = repo.shouldPull(git); + + assertThat(shouldPull).as("shouldPull was false").isTrue(); + } + + @Test + public void shouldPullTruncatedIndexNotForcePull() throws Exception { + Git git = mock(Git.class); + StatusCommand statusCommand = mock(StatusCommand.class); + Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); + + when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")) + .thenReturn("http://example/git"); + when(statusCommand.call()) + .thenThrow(new JGitInternalException("Short read of block.")); + when(status.isClean()).thenReturn(true); + + JGitEnvironmentRepository repo = new JGitEnvironmentRepository(this.environment, + new JGitEnvironmentProperties()); + repo.setForcePull(false); + + try { + final boolean shouldPull = repo.shouldPull(git); + assertThat(shouldPull).as("shouldPull did not fail").isFalse(); + } + catch (JGitInternalException e) { + assertThat(e.getMessage()).as("shouldPull did not fail as expected") + .isEqualTo("Short read of block."); + } + } + + @Test + public void shouldPullTruncatedIndexResetFail() throws Exception { + final String mockThrownMessage = "__mock_thrown__"; + + Git git = mock(Git.class); + StatusCommand statusCommand = mock(StatusCommand.class); + Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); + ResetCommand resetCommand = mock(ResetCommand.class); + + when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")) + .thenReturn("http://example/git"); + when(statusCommand.call()).thenThrow(new JGitInternalException(mockThrownMessage)) + .thenReturn(status); + when(status.isClean()).thenReturn(true); + when(git.reset()).thenReturn(resetCommand); + when(resetCommand.call()).thenThrow(new GitAPIException("") { + }); + + JGitEnvironmentRepository repo = new JGitEnvironmentRepository(this.environment, + new JGitEnvironmentProperties()); + + try { + repo.shouldPull(git); + Assertions.fail("shouldPull did not fail"); + } + catch (JGitInternalException e) { + assertThat(e.getMessage()).as("shouldPull did not fail as expected") + .isEqualTo(mockThrownMessage); + } + } + + @Test + public void shouldPullStatusFail() throws Exception { + final String mockThrownMessage = "__mock_thrown__"; + + Git git = mock(Git.class); + StatusCommand statusCommand = mock(StatusCommand.class); + Status status = mock(Status.class); + Repository repository = mock(Repository.class); + StoredConfig storedConfig = mock(StoredConfig.class); + + when(git.status()).thenReturn(statusCommand); + when(git.getRepository()).thenReturn(repository); + when(repository.getConfig()).thenReturn(storedConfig); + when(storedConfig.getString("remote", "origin", "url")) + .thenReturn("http://example/git"); + when(statusCommand.call()) + .thenThrow(new JGitInternalException(mockThrownMessage)); + when(status.isClean()).thenReturn(true); + + JGitEnvironmentRepository repo = new JGitEnvironmentRepository(this.environment, + new JGitEnvironmentProperties()); + + try { + repo.shouldPull(git); + Assertions.fail("shouldPull did not fail"); + } + catch (JGitInternalException e) { + assertThat(e.getMessage()).as("shouldPull did not fail as expected") + .isEqualTo(mockThrownMessage); + } + } + @Test public void shouldPullClean() throws Exception { Git git = mock(Git.class);