Consistently handle remote repository creation failures
Failure to create remote repository was handled inconsistently in the
three existing AdminApi implementations. This issue was present in the
code even before the refactoring done in
Ie760bf3e143b1d143b6e81ac6cfa816ef1f8d016. For example, creation of
remote repository over SSH [1] didn't return a status back to the
caller, while the Gerrit+SSH implementation [2] did return a boolean
flag which was afterwards ignored by the caller [3].
Not handling remote repository creation failures had an ugly effect:
we log the "Missing repository created; ..." [4], even if the repository
creation failed.
Let all three implementations of the AdminApi return a flag indicating
the success/failure and make sure to handle it properly.
Further, remove some redundant and potentially confusing parts of the
log messages as they assume that failure to create missing repository
can only be caused by using wrong protocol.
[1] https://gerrit.googlesource.com/plugins/replication/+/d557ccc642c59a55750f560ce0d98870e1550d65/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java#290
[2] https://gerrit.googlesource.com/plugins/replication/+/d557ccc642c59a55750f560ce0d98870e1550d65/src/main/java/com/googlesource/gerrit/plugins/replication/GerritSshApi.java#43
[3] https://gerrit.googlesource.com/plugins/replication/+/d557ccc642c59a55750f560ce0d98870e1550d65/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java#259
[4] https://gerrit.googlesource.com/plugins/replication/+/092792edacf9c29732a560a30967b92664cd65f9/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java#400
Change-Id: I6566f867138ab73c81ff7a67630772c3734e501b
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApi.java
index ef7e353..79362fd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApi.java
@@ -17,7 +17,7 @@
import com.google.gerrit.reviewdb.client.Project;
public interface AdminApi {
- public void createProject(Project.NameKey project, String head);
+ public boolean createProject(Project.NameKey project, String head);
public void deleteProject(Project.NameKey project);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritSshApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritSshApi.java
index 85b17d0..0b439cc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritSshApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritSshApi.java
@@ -41,14 +41,16 @@
}
@Override
- public void createProject(Project.NameKey projectName, String head) {
+ public boolean createProject(Project.NameKey projectName, String head) {
OutputStream errStream = sshHelper.newErrorBufferStream();
String cmd = "gerrit create-project --branch " + head + " " + projectName.get();
try {
execute(uri, cmd, errStream);
} catch (IOException e) {
logError("creating", uri, errStream, cmd, e);
+ return false;
}
+ return true;
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/LocalFS.java b/src/main/java/com/googlesource/gerrit/plugins/replication/LocalFS.java
index 1012cd7..908e7e0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/LocalFS.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/LocalFS.java
@@ -34,7 +34,7 @@
}
@Override
- public void createProject(Project.NameKey project, String head) {
+ public boolean createProject(Project.NameKey project, String head) {
try (Repository repo = new FileRepository(uri.getPath())) {
repo.create(true /* bare */);
@@ -46,7 +46,9 @@
repLog.info("Created local repository: {}", uri);
} catch (IOException e) {
repLog.error("Error creating local repository {}", uri.getPath(), e);
+ return false;
}
+ return true;
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
index 1245e4b..241ca49 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -401,11 +401,7 @@
repLog.warn("Missing repository created; retry replication to {}", uri);
pool.reschedule(this, Destination.RetryReason.REPOSITORY_MISSING);
} else {
- repLog.warn(
- "Missing repository could not be created when replicating {}. "
- + "You can only create missing repositories locally, over SSH or when "
- + "using adminUrl in replication.config. See documentation for more information.",
- uri);
+ repLog.warn("Missing repository could not be created when replicating {}", uri);
}
} catch (IOException ioe) {
stateLog.error(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
index dad1b0b..6dfa117 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteSsh.java
@@ -33,7 +33,7 @@
}
@Override
- public void createProject(Project.NameKey project, String head) {
+ public boolean createProject(Project.NameKey project, String head) {
String quotedPath = QuotedString.BOURNE.quote(uri.getPath());
String cmd = "mkdir -p " + quotedPath + " && cd " + quotedPath + " && git init --bare";
if (head != null) {
@@ -54,7 +54,9 @@
cmd,
errStream,
e);
+ return false;
}
+ return true;
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
index ae06f43..db20282 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -249,8 +249,7 @@
private boolean createProject(URIish replicateURI, Project.NameKey projectName, String head) {
Optional<AdminApi> adminApi = adminApiFactory.create(replicateURI);
- if (adminApi.isPresent()) {
- adminApi.get().createProject(projectName, head);
+ if (adminApi.isPresent() && adminApi.get().createProject(projectName, head)) {
return true;
}
@@ -279,10 +278,6 @@
}
private void warnCannotPerform(String op, URIish uri) {
- repLog.warn(
- "Cannot {} on remote site {}."
- + "Only local paths and SSH URLs are supported for this operation",
- op,
- uri);
+ repLog.warn("Cannot {} on remote site {}.", op, uri);
}
}
