Use gerrit.instanceId instead of replication.instanceLabel
Pull-replication uses replication.instanceLabel to identify source
node. Gerrit 3.4 introduce gerrit.instanceId property. To avoid having
multiple properties which contains similar information use
gerrit.instanceId instead of replication.instanceLabel.
For backward compatibility replication.instanceLabel is still used
if present if not gerrit.instanceId will be used.
Bug: Issue 15636
Change-Id: I324f25bc38007a2a48c1068980a31c637f2cd9d1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
index f2f57cd..f1d486d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClient.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.Url;
+import com.google.gerrit.server.config.GerritInstanceId;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.inject.Inject;
@@ -66,7 +67,7 @@
private final CredentialsFactory credentials;
private final SourceHttpClient.Factory httpClientFactory;
private final Source source;
- private final String instanceLabel;
+ private final String instanceId;
private final String pluginName;
private final SyncRefsFilter syncRefsFilter;
@@ -77,18 +78,22 @@
ReplicationConfig replicationConfig,
SyncRefsFilter syncRefsFilter,
@PluginName String pluginName,
+ @Nullable @GerritInstanceId String instanceId,
@Assisted Source source) {
this.credentials = credentials;
this.httpClientFactory = httpClientFactory;
this.source = source;
this.pluginName = pluginName;
this.syncRefsFilter = syncRefsFilter;
- this.instanceLabel =
- Strings.nullToEmpty(
+ this.instanceId =
+ Optional.ofNullable(
replicationConfig.getConfig().getString("replication", null, "instanceLabel"))
+ .orElse(instanceId)
.trim();
+
requireNonNull(
- Strings.emptyToNull(instanceLabel), "replication.instanceLabel cannot be null or empty");
+ Strings.emptyToNull(this.instanceId),
+ "gerrit.instanceId or replication.instanceLabel must be set");
}
/* (non-Javadoc)
@@ -107,7 +112,7 @@
new StringEntity(
String.format(
"{\"label\":\"%s\", \"ref_name\": \"%s\", \"async\":%s}",
- instanceLabel, refName, callAsync),
+ instanceId, refName, callAsync),
StandardCharsets.UTF_8));
post.addHeader(new BasicHeader("Content-Type", "application/json"));
return httpClientFactory.create(source).execute(post, this, getContext(targetUri));
@@ -172,7 +177,7 @@
} else {
requireNull(revisionData, "DELETE ref-updates cannot be associated with a RevisionData");
}
- RevisionInput input = new RevisionInput(instanceLabel, refName, revisionData);
+ RevisionInput input = new RevisionInput(instanceId, refName, revisionData);
String url =
String.format(
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index b2470a4..53541ba 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -31,8 +31,13 @@
threads = 3
authGroup = Public Mirror Group
authGroup = Second Public Mirror Group
- [replication]
- instanceLabel = host-one
+```
+
+And make sure that instanceId is setup in `$site_path/etc/gerrit.config`:
+
+```
+[gerrit]
+ instanceId = host-one
```
Then reload the replication plugin to pick up the new configuration:
@@ -121,6 +126,10 @@
provided in the remote configuration section which name is equal
to instanceLabel.
+ Deprecated: This property is kept for backward compatibility and
+ will be removed in the future release. Use [gerrit.instanceId](https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#gerrit.instanceId)
+ instead.
+
replication.maxConnectionsPerRoute
: Maximum number of HTTP connections per one HTTP route.
@@ -441,7 +450,6 @@
autoReload = true
replicateOnStartup = false
[replication]
- instanceLabel = host-one
lockErrorMaxRetries = 5
maxRetries = 5
```
@@ -477,7 +485,6 @@
autoReload = true
replicateOnStartup = false
[replication]
- instanceLabel = host-one
lockErrorMaxRetries = 5
maxRetries = 5
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
index cb5af42..ee5876f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.SkipProjectClone;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -95,6 +96,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewChangeRef() throws Exception {
testRepo = cloneProject(createTestProject(project + TEST_REPLICATION_SUFFIX));
@@ -122,6 +124,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewChangeRefAfterConfigReloaded() throws Exception {
testRepo = cloneProject(createTestProject(project + TEST_REPLICATION_SUFFIX));
@@ -157,6 +160,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewBranch() throws Exception {
String testProjectName = project + TEST_REPLICATION_SUFFIX;
createTestProject(testProjectName);
@@ -190,6 +194,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldAutoReloadConfiguration() throws Exception {
SourcesCollection sources = getInstance(SourcesCollection.class);
AutoReloadConfigDecorator autoReloadConfigDecorator =
@@ -202,6 +207,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldAutoReloadConfigurationWhenRemoteConfigAdded() throws Exception {
FileBasedConfig newRemoteConfig =
new FileBasedConfig(
@@ -224,6 +230,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldAutoReloadConfigurationWhenRemoteConfigDeleted() throws Exception {
SourcesCollection sources = getInstance(SourcesCollection.class);
AutoReloadConfigDecorator autoReloadConfigDecorator =
@@ -259,7 +266,6 @@
}
private void setReplicationSource(String remoteName) throws IOException {
- config.setString("replication", null, "instanceLabel", remoteName);
config.setBoolean("gerrit", null, "autoReload", true);
config.save();
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java
index cf94086..13b3460 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.acceptance.SkipProjectClone;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
@@ -115,6 +116,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewChangeRef() throws Exception {
testRepo = cloneProject(createTestProject(project + TEST_REPLICATION_SUFFIX));
@@ -142,6 +144,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewBranch() throws Exception {
String testProjectName = project + TEST_REPLICATION_SUFFIX;
createTestProject(testProjectName);
@@ -176,6 +179,7 @@
@Test
@UseLocalDisk
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateForceUpdatedBranch() throws Exception {
boolean forcedPush = true;
String testProjectName = project + TEST_REPLICATION_SUFFIX;
@@ -245,6 +249,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewChangeRefCGitClient() throws Exception {
AutoReloadConfigDecorator autoReloadConfigDecorator =
getInstance(AutoReloadConfigDecorator.class);
@@ -280,6 +285,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateNewBranchCGitClient() throws Exception {
AutoReloadConfigDecorator autoReloadConfigDecorator =
getInstance(AutoReloadConfigDecorator.class);
@@ -321,6 +327,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateProjectDeletion() throws Exception {
String projectToDelete = project.get();
setReplicationSource(TEST_REPLICATION_REMOTE, "", Optional.of(projectToDelete));
@@ -349,6 +356,7 @@
}
@Test
+ @GerritConfig(name = "gerrit.instanceId", value = TEST_REPLICATION_REMOTE)
public void shouldReplicateHeadUpdate() throws Exception {
String testProjectName = project.get();
setReplicationSource(TEST_REPLICATION_REMOTE, "", Optional.of(testProjectName));
@@ -407,7 +415,6 @@
replicaSuffixes.stream()
.map(suffix -> gitPath.resolve("${name}" + suffix + ".git").toString())
.collect(toList());
- config.setString("replication", null, "instanceLabel", remoteName);
config.setStringList("remote", remoteName, "url", replicaUrls);
config.setString("remote", remoteName, "apiUrl", adminRestSession.url());
config.setString("remote", remoteName, "fetch", "+refs/*:refs/*");
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java
index cef1051..e6f788a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/ActionITBase.java
@@ -199,7 +199,6 @@
replicaSuffixes.stream()
.map(suffix -> gitPath.resolve("${name}" + suffix + ".git").toString())
.collect(toList());
- config.setString("replication", null, "instanceLabel", remoteName);
config.setStringList("remote", remoteName, "url", replicaUrls);
config.setString("remote", remoteName, "apiUrl", adminRestSession.url());
config.setString("remote", remoteName, "fetch", "+refs/tags/*:refs/tags/*");
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientTest.java
index a1c2172..f0b8b99 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/FetchRestApiClientTest.java
@@ -79,7 +79,7 @@
String api = "http://gerrit-host";
String pluginName = "pull-replication";
- String label = "Replication";
+ String instanceId = "Replication";
String refName = RefNames.REFS_HEADS + "master";
String expectedPayload =
@@ -159,7 +159,6 @@
when(replicationConfig.getConfig()).thenReturn(config);
when(config.getStringList("replication", null, "syncRefs")).thenReturn(new String[0]);
when(source.getRemoteConfigName()).thenReturn("Replication");
- when(config.getString("replication", null, "instanceLabel")).thenReturn(label);
HttpResult httpResult = new HttpResult(SC_CREATED, Optional.of("result message"));
when(httpClient.execute(any(HttpPost.class), any(), any())).thenReturn(httpResult);
@@ -167,7 +166,13 @@
syncRefsFilter = new SyncRefsFilter(replicationConfig);
objectUnderTest =
new FetchRestApiClient(
- credentials, httpClientFactory, replicationConfig, syncRefsFilter, pluginName, source);
+ credentials,
+ httpClientFactory,
+ replicationConfig,
+ syncRefsFilter,
+ pluginName,
+ instanceId,
+ source);
}
@Test
@@ -191,7 +196,13 @@
syncRefsFilter = new SyncRefsFilter(replicationConfig);
objectUnderTest =
new FetchRestApiClient(
- credentials, httpClientFactory, replicationConfig, syncRefsFilter, pluginName, source);
+ credentials,
+ httpClientFactory,
+ replicationConfig,
+ syncRefsFilter,
+ pluginName,
+ instanceId,
+ source);
objectUnderTest.callFetch(Project.nameKey("test_repo"), refName, new URIish(api));
@@ -210,7 +221,13 @@
syncRefsFilter = new SyncRefsFilter(replicationConfig);
objectUnderTest =
new FetchRestApiClient(
- credentials, httpClientFactory, replicationConfig, syncRefsFilter, pluginName, source);
+ credentials,
+ httpClientFactory,
+ replicationConfig,
+ syncRefsFilter,
+ pluginName,
+ instanceId,
+ source);
objectUnderTest.callFetch(Project.nameKey("test_repo"), refName, new URIish(api));
@@ -232,7 +249,13 @@
syncRefsFilter = new SyncRefsFilter(replicationConfig);
objectUnderTest =
new FetchRestApiClient(
- credentials, httpClientFactory, replicationConfig, syncRefsFilter, pluginName, source);
+ credentials,
+ httpClientFactory,
+ replicationConfig,
+ syncRefsFilter,
+ pluginName,
+ instanceId,
+ source);
objectUnderTest.callFetch(Project.nameKey("test_repo"), refName, new URIish(api));
verify(httpClient, times(1)).execute(httpPostCaptor.capture(), any(), any());
@@ -321,7 +344,6 @@
@Test
public void shouldThrowExceptionWhenInstanceLabelIsNull() {
- when(config.getString("replication", null, "instanceLabel")).thenReturn(null);
assertThrows(
NullPointerException.class,
() ->
@@ -331,12 +353,12 @@
replicationConfig,
syncRefsFilter,
pluginName,
+ null,
source));
}
@Test
public void shouldTrimInstanceLabel() {
- when(config.getString("replication", null, "instanceLabel")).thenReturn(" ");
assertThrows(
NullPointerException.class,
() ->
@@ -346,12 +368,12 @@
replicationConfig,
syncRefsFilter,
pluginName,
+ " ",
source));
}
@Test
public void shouldThrowExceptionWhenInstanceLabelIsEmpty() {
- when(config.getString("replication", null, "instanceLabel")).thenReturn("");
assertThrows(
NullPointerException.class,
() ->
@@ -361,10 +383,32 @@
replicationConfig,
syncRefsFilter,
pluginName,
+ "",
source));
}
@Test
+ public void shouldUseReplicationLabelWhenProvided()
+ throws ClientProtocolException, IOException, URISyntaxException {
+ when(config.getString("replication", null, "instanceLabel")).thenReturn(instanceId);
+ FetchRestApiClient objectUnderTest =
+ new FetchRestApiClient(
+ credentials,
+ httpClientFactory,
+ replicationConfig,
+ syncRefsFilter,
+ pluginName,
+ "",
+ source);
+ objectUnderTest.callFetch(Project.nameKey("test_repo"), refName, new URIish(api));
+
+ verify(httpClient, times(1)).execute(httpPostCaptor.capture(), any(), any());
+
+ HttpPost httpPost = httpPostCaptor.getValue();
+ assertThat(readPayload(httpPost)).isEqualTo(expectedPayload);
+ }
+
+ @Test
public void shouldCallInitProjectEndpoint() throws IOException, URISyntaxException {
objectUnderTest.initProject(Project.nameKey("test_repo"), new URIish(api));
