Merge branch 'stable-3.6'
* stable-3.6:
Add explicit testcontainers deps for testing
Bump global-refdb to v3.4.0
Introduce zookeeper-refdb init step
Bump zookeeper and netty versions
Bazel: Format BUILD file with buildifier
Change-Id: If3554d5189509aeb565dfbe993fec5bf578fd0cb
diff --git a/BUILD b/BUILD
index a640fc1..0751458 100644
--- a/BUILD
+++ b/BUILD
@@ -22,7 +22,7 @@
"@curator-framework//jar",
"@curator-recipes//jar",
"@global-refdb//jar",
- "@netty-all_3.5//jar",
+ "@netty-all//jar",
"@zookeeper-jute_3.5//jar",
"@zookeeper_3.5//jar",
],
@@ -34,6 +34,7 @@
manifest_entries = [
"Gerrit-PluginName: zookeeper-refdb",
"Gerrit-Module: com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.ZkValidationModule",
+ "Gerrit-InitStep: com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.ZkInit",
"Implementation-Title: zookeeper ref-db plugin",
"Implementation-URL: https://review.gerrithub.io/admin/repos/GerritForge/plugins_zookeeper",
],
@@ -73,10 +74,10 @@
"@jackson-annotations//jar",
"@jna//jar",
"@visible-assertions//jar",
- "@testcontainers//jar",
"@docker-java-api//jar",
"@docker-java-transport//jar",
"@duct-tape//jar",
+ "@testcontainers//jar",
"@testcontainer-localstack//jar",
"@jackson-dataformat-cbor//jar",
"@jackson-databind//jar",
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index d63f76d..1a3fc2b 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -29,20 +29,20 @@
maven_jar(
name = "zookeeper_3.5",
- artifact = "org.apache.zookeeper:zookeeper:3.5.7",
- sha1 = "12bdf55ba8be7fc891996319d37f35eaad7e63ea",
+ artifact = "org.apache.zookeeper:zookeeper:3.5.8",
+ sha1 = "fc0d02657ed5b26029daa50d7f98b9806a0b13af",
)
maven_jar(
name = "zookeeper-jute_3.5",
- artifact = "org.apache.zookeeper:zookeeper-jute:3.5.7",
- sha1 = "1270f80b08904499a6839a2ee1800da687ad96b4",
+ artifact = "org.apache.zookeeper:zookeeper-jute:3.5.8",
+ sha1 = "b399078f6ccfd6c258e42054091052e8f3e05824",
)
maven_jar(
- name = "netty-all_3.5",
- artifact = "io.netty:netty-all:4.1.45.Final",
- sha1 = "e830eae36d22f2bba3118a3bc08e17f15263a01d",
+ name = "netty-all",
+ artifact = "io.netty:netty-all:4.1.48.Final",
+ sha1 = "ebb3666ba4883ba81920cec8ccb1a3adcc827eb1",
)
maven_jar(
@@ -91,6 +91,12 @@
sha1 = "95c6cfde71c2209f0c29cb14e432471e0b111880",
)
+ maven_jar(
+ name = "testcontainer-localstack",
+ artifact = "org.testcontainers:localstack:" + TESTCONTAINERS_VERSION,
+ sha1 = "7aa69995bdaafb4b06e69fdab9bd98c4fddee43d",
+ )
+
DOCKER_JAVA_VERS = "3.2.8"
maven_jar(
@@ -122,9 +128,3 @@
artifact = "net.java.dev.jna:jna:5.5.0",
sha1 = "0e0845217c4907822403912ad6828d8e0b256208",
)
-
- maven_jar(
- name = "testcontainer-localstack",
- artifact = "org.testcontainers:localstack:" + TESTCONTAINERS_VERSION,
- sha1 = "7aa69995bdaafb4b06e69fdab9bd98c4fddee43d",
- )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java
new file mode 100644
index 0000000..893698e
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkInit.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper;
+
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.schema.NoteDbSchemaVersionManager;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration.ZkMigrations;
+import java.io.IOException;
+import org.apache.curator.framework.CuratorFramework;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+
+@Singleton
+public class ZkInit implements InitStep {
+
+ private final ConsoleUI ui;
+ private final FileBasedConfig config;
+
+ @Inject(optional = true)
+ private NoteDbSchemaVersionManager versionManager;
+
+ @Inject(optional = true)
+ private ZkMigrations zkMigrations;
+
+ private final String pluginName;
+ private final Injector initInjector;
+
+ @Inject
+ ZkInit(ConsoleUI ui, SitePaths site, @PluginName String pluginName, Injector initInjector)
+ throws IOException, ConfigInvalidException {
+ this.ui = ui;
+ this.pluginName = pluginName;
+ this.initInjector = initInjector;
+
+ config =
+ new FileBasedConfig(site.etc_dir.resolve(pluginName + ".config").toFile(), FS.DETECTED);
+ config.load();
+ }
+
+ @Override
+ public void run() throws Exception {
+ if (config.getSections().isEmpty()) {
+ ui.message("%s configuration not found: global-refdb migration skipped\n", pluginName);
+ return;
+ }
+
+ Injector injector = getInjector(initInjector);
+ injector.injectMembers(this);
+
+ try {
+ try (CuratorFramework curator = new ZookeeperConfig(config).buildCurator()) {
+ zkMigrations.migrate(injector, curator, versionManager.read());
+ }
+ } catch (StorageException e) {
+ // No version information: skip migration as it is most likely a new site
+ }
+ }
+
+ public static Injector getInjector(Injector parentInjector) {
+ return parentInjector.createChildInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(AllProjectsName.class).toProvider(AllProjectsNameProvider.class);
+ bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
+ }
+ });
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
index 7def5f2..0aa0c9f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZkSharedRefDatabase.java
@@ -168,7 +168,7 @@
String message =
String.format(
"Error trying to perform CAS of generic value at path %s", pathFor(project, refName));
- logger.atWarning().withCause(e).log(message);
+ logger.atWarning().withCause(e).log("%s", message);
throw new GlobalRefDbSystemError(message, e);
}
}
@@ -204,7 +204,7 @@
return client.checkExists().forPath(pathFor(projectName, oldRef)) == null;
}
- static String pathFor(Project.NameKey projectName, String refName) {
+ public static String pathFor(Project.NameKey projectName, String refName) {
return "/" + projectName + "/" + refName;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
index 9abc196..117dead 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/ZookeeperConfig.java
@@ -94,7 +94,10 @@
@Inject
public ZookeeperConfig(PluginConfigFactory cfgFactory, @PluginName String pluginName) {
- Config zkConfig = cfgFactory.getGlobalPluginConfig(pluginName);
+ this(cfgFactory.getGlobalPluginConfig(pluginName));
+ }
+
+ public ZookeeperConfig(Config zkConfig) {
connectionString =
getString(zkConfig, SECTION, SUBSECTION, KEY_CONNECT_STRING, DEFAULT_ZK_CONNECT);
root = getString(zkConfig, SECTION, SUBSECTION, KEY_ROOT_NODE, "gerrit/multi-site");
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration.java
new file mode 100644
index 0000000..0b91c8b
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration.java
@@ -0,0 +1,22 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration;
+
+import org.apache.curator.framework.CuratorFramework;
+
+public interface ZkMigration {
+
+ void run(CuratorFramework curator) throws Exception;
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_182.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_182.java
new file mode 100644
index 0000000..8f0d497
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_182.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration;
+
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.ZkSharedRefDatabase;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+
+class ZkMigration_Schema_182 implements ZkMigration {
+ private final AllUsersName allUsers;
+
+ @Inject
+ ZkMigration_Schema_182(AllUsersName allUsers) {
+ this.allUsers = allUsers;
+ }
+
+ @Override
+ public void run(CuratorFramework curator) throws Exception {
+ try {
+ curator
+ .delete()
+ .deletingChildrenIfNeeded()
+ .forPath(ZkSharedRefDatabase.pathFor(allUsers, "refs/draft-comments"));
+ } catch (NoNodeException e) {
+ // Nothing to do: the node did not exist
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_183.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_183.java
new file mode 100644
index 0000000..6e0a96e
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigration_Schema_183.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration;
+
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.ZkSharedRefDatabase;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+
+class ZkMigration_Schema_183 implements ZkMigration {
+ private final AllProjectsName allProjects;
+
+ @Inject
+ ZkMigration_Schema_183(AllProjectsName allProjects) {
+ this.allProjects = allProjects;
+ }
+
+ @Override
+ public void run(CuratorFramework curator) throws Exception {
+ try {
+ curator.delete().forPath(ZkSharedRefDatabase.pathFor(allProjects, RefNames.REFS_CONFIG));
+ } catch (NoNodeException e) {
+ // Nothing to do: the node did not exist
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrations.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrations.java
new file mode 100644
index 0000000..6af0e30
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrations.java
@@ -0,0 +1,66 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration;
+
+import static com.google.gerrit.server.schema.NoteDbSchemaVersions.LATEST;
+
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import org.apache.curator.framework.CuratorFramework;
+
+public class ZkMigrations {
+ private final ConsoleUI ui;
+ private final String pluginName;
+
+ @Inject
+ public ZkMigrations(ConsoleUI ui, @PluginName String pluginName) {
+ this.ui = ui;
+ this.pluginName = pluginName;
+ }
+
+ @SuppressWarnings("unchecked")
+ public void migrate(Injector injector, CuratorFramework curator, int currentVersion)
+ throws Exception {
+ if (currentVersion <= 0 || currentVersion >= LATEST) {
+ return;
+ }
+
+ boolean headerDisplayed = false;
+
+ for (int version = currentVersion + 1; version <= LATEST; version++) {
+ try {
+ Class<ZkMigration> migration =
+ (Class<ZkMigration>) Class.forName(ZkMigration.class.getName() + "_Schema_" + version);
+
+ if (!headerDisplayed) {
+ ui.header("%s migration", pluginName);
+ headerDisplayed = true;
+ }
+
+ ui.message("Migrating %s data to schema %d ... ", pluginName, version);
+ injector.getInstance(migration).run(curator);
+ ui.message("DONE\n");
+ } catch (ClassNotFoundException e) {
+ // No migration found for the schema
+ }
+ }
+
+ if (headerDisplayed) {
+ ui.message("\n%s migration completed successfully\n\n", pluginName);
+ }
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrationsTest.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrationsTest.java
new file mode 100644
index 0000000..3ba2eba
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/zookeeper/migration/ZkMigrationsTest.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.migration;
+
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.gerrit.server.schema.NoteDbSchemaVersions;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.api.BackgroundVersionable;
+import org.apache.curator.framework.api.DeleteBuilder;
+import org.junit.Before;
+import org.junit.Test;
+
+@TestPlugin(
+ name = "foo",
+ sysModule = "com.googlesource.gerrit.plugins.validation.dfsrefdb.zookeeper.ZkValidationModule")
+public class ZkMigrationsTest extends LightweightPluginDaemonTest {
+
+ private CuratorFramework curatorMock;
+ private ConsoleUI uiMock;
+ private ZkMigrations migrations;
+ private DeleteBuilder deleteBuilderMock;
+ private BackgroundVersionable backgroundVersionableMock;
+
+ @Before
+ public void setup() {
+ curatorMock = mock(CuratorFramework.class, RETURNS_DEEP_STUBS);
+ deleteBuilderMock = mock(DeleteBuilder.class);
+ backgroundVersionableMock = mock(BackgroundVersionable.class);
+ when(curatorMock.delete()).thenReturn(deleteBuilderMock);
+ when(deleteBuilderMock.deletingChildrenIfNeeded()).thenReturn(backgroundVersionableMock);
+
+ uiMock = mock(ConsoleUI.class);
+ migrations = new ZkMigrations(uiMock, "foo");
+ }
+
+ @Test
+ public void shouldNotMigrateAnythingForNewSites() throws Exception {
+ migrations.migrate(plugin.getSysInjector(), curatorMock, 0);
+ verifyZeroInteractions(curatorMock);
+ }
+
+ @Test
+ public void shouldNotMigrateAnythingForSchema184() throws Exception {
+ migrations.migrate(plugin.getSysInjector(), curatorMock, 184);
+ verifyZeroInteractions(curatorMock);
+ }
+
+ @Test
+ public void shouldCallCuratorDeleteForMigrationFromSchema182PlusOneToLatest() throws Exception {
+ migrations.migrate(plugin.getSysInjector(), curatorMock, 182);
+
+ verify(curatorMock).delete();
+ verify(deleteBuilderMock).forPath(eq("/All-Projects/" + RefNames.REFS_CONFIG));
+
+ verifyNoMoreInteractions(curatorMock);
+ verifyNoMoreInteractions(deleteBuilderMock);
+ verifyZeroInteractions(backgroundVersionableMock);
+ }
+
+ @Test
+ public void shouldNotCallCuratorDeleteIfAlreadyOnLatestVersion() throws Exception {
+ migrations.migrate(plugin.getSysInjector(), curatorMock, NoteDbSchemaVersions.LATEST);
+ verifyZeroInteractions(curatorMock);
+ }
+
+ @Test
+ public void shouldNotCallCuratorDeleteIfOverLatestVersion() throws Exception {
+ migrations.migrate(plugin.getSysInjector(), curatorMock, NoteDbSchemaVersions.LATEST + 1);
+ verifyZeroInteractions(curatorMock);
+ }
+}