[JENKINS-31934] throw IOException around submodule update failures

Updating the submodule can fail in a similar manner to git checkout of the parent project. If we simply throw the GitException, then the generic retry loop will not kick in. For now, the only exceptions we'll catch is the GitException which should be generated if the launchCommand fails. Without this change, intermittent failures when attempting to pull submodule data from a git remote will not properly allow the retry loop to kick in. Add a new test case to verify that we actually do throw the IOException. Because this exception case is likely due to an intermittent error, we'll use mocks to setup a case where we always throw the exception. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
jenkinsci · Nov 15, 2017 · d747508 · d747508
1 parent 5838c3c
commit d747508
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 11 deletions.
diff --git a/src/main/java/hudson/plugins/git/extensions/impl/SubmoduleOption.java b/src/main/java/hudson/plugins/git/extensions/impl/SubmoduleOption.java
@@ -94,17 +94,23 @@ public void onClean(GitSCM scm, GitClient git) throws IOException, InterruptedEx
     public void onCheckoutCompleted(GitSCM scm, Run<?, ?> build, GitClient git, TaskListener listener) throws IOException, InterruptedException, GitException {
         BuildData revToBuild = scm.getBuildData(build);
 
-        if (!disableSubmodules && git.hasGitModules()) {
-            // This ensures we don't miss changes to submodule paths and allows
-            // seamless use of bare and non-bare superproject repositories.
-            git.setupSubmoduleUrls(revToBuild.lastBuild.getRevision(), listener);
-            git.submoduleUpdate()
-                    .recursive(recursiveSubmodules)
-                    .remoteTracking(trackingSubmodules)
-                    .parentCredentials(parentCredentials)
-                    .ref(build.getEnvironment(listener).expand(reference))
-                    .timeout(timeout)
-                    .execute();
+        try {
+            if (!disableSubmodules && git.hasGitModules()) {
+                // This ensures we don't miss changes to submodule paths and allows
+                // seamless use of bare and non-bare superproject repositories.
+                git.setupSubmoduleUrls(revToBuild.lastBuild.getRevision(), listener);
+                git.submoduleUpdate()
+                        .recursive(recursiveSubmodules)
+                        .remoteTracking(trackingSubmodules)
+                        .parentCredentials(parentCredentials)
+                        .ref(build.getEnvironment(listener).expand(reference))
+                        .timeout(timeout)
+                        .execute();
+            }
+        } catch (GitException e) {
+            // Re-throw as an IOException in order to allow generic retry
+            // logic to kick in properly.
+            throw new IOException("Could not perform submodule update", e);
         }
 
         if (scm.isDoGenerateSubmoduleConfigurations()) {

diff --git a/src/test/java/hudson/plugins/git/extensions/impl/SubmoduleOptionTest.java b/src/test/java/hudson/plugins/git/extensions/impl/SubmoduleOptionTest.java
@@ -0,0 +1,83 @@
+package hudson.plugins.git.extensions.impl;
+
+import hudson.plugins.git.extensions.GitSCMExtension;
+import hudson.plugins.git.extensions.impl.*;
+
+import hudson.plugins.git.GitSCM;
+import org.jenkinsci.plugins.gitclient.*;
+
+import jenkins.security.MasterToSlaveCallable;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.jenkinsci.plugins.gitclient.*;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.TestExtension;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ObjectStreamException;
+import java.io.Serializable;
+import java.net.URL;
+import java.text.MessageFormat;
+import java.util.*;
+import org.eclipse.jgit.transport.RemoteConfig;
+import static org.hamcrest.Matchers.*;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import static org.junit.Assert.*;
+import org.junit.Before;
+import org.junit.BeforeClass;
+
+import hudson.model.Run;
+import hudson.plugins.git.GitException;
+import hudson.model.TaskListener;
+import hudson.plugins.git.util.BuildData;
+import hudson.plugins.git.util.Build;
+
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+
+public class SubmoduleOptionTest {
+
+    @Issue("JENKINS-31934")
+    @Test
+    public void testSubmoduleUpdateThrowsIOException() throws Exception {
+        SubmoduleOption submoduleOption = new SubmoduleOption(false, false, false, null, null, false);
+
+        // In order to verify that the submodule option correctly converts
+        // GitExceptions into IOExceptions, setup a SubmoduleOption, and run
+        // it's onCheckoutCOmpleted extension point with a mocked git client
+        // that always throws an exception.
+        BuildData buildData = Mockito.mock(BuildData.class);
+        Build lastBuild = Mockito.mock(Build.class);
+        GitSCM scm = Mockito.mock(GitSCM.class);
+        Run<?, ?> build = Mockito.mock(Run.class);
+        GitClient client = Mockito.mock(GitClient.class);
+        TaskListener listener = Mockito.mock(TaskListener.class);
+        buildData.lastBuild = lastBuild;
+        Mockito.when(scm.getBuildData(build)).thenReturn(buildData);
+        Mockito.when(client.hasGitModules()).thenReturn(true);
+        Mockito.when(client.submoduleUpdate()).thenThrow(new GitException("a git exception"));
+
+        try {
+            submoduleOption.onCheckoutCompleted(scm, build, client, listener);
+            fail("Expected IOException to be thrown");
+        } catch (IOException e) {
+            assertThat(e.getMessage(), is("Could not perform submodule update"));
+        }
+    }
+}