Skip to content
Permalink
Browse files
[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>
  • Loading branch information
jacob-keller committed Nov 15, 2017
1 parent 5838c3c commit d74750819e5a2ed5397763954d0a32d22d062f6d
@@ -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()) {
@@ -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"));
}
}
}

0 comments on commit d747508

Please sign in to comment.