bugfix for RackawareEnsemblePlacementPolicyImpl.isEnsembleAdheringToPlacementPolicy

.github/workflows/backward-compat-tests.yml

@@ -0,0 +1,58 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+#
+
+name: Backward compatibility tests
+
+on:
+  push:
+  pull_request:
+    branches:
+      - master
+      - branch-*
+    paths-ignore:
+      - 'site/**'
+    workflow_dispatch:
+
+env:
+  MAVEN_OPTS: -Dmaven.wagon.httpconnectionManager.ttlSeconds=25 -Dmaven.wagon.http.retryHandler.count=3
+
+jobs:
+  test:
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Tune Runner VM
+        uses: ./.github/actions/tune-runner-vm
+
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      - name: Build
+        run: ./gradlew stream:server:build -x test
+      - name: Test current server with old clients
+        run: ./gradlew :tests:backward-compat:current-server-old-clients:test || (tail -n +1 tests/backward-compat/current-server-old-clients/build/reports/tests/test/classes/* && tail -n +1 tests/backward-compat/current-server-old-clients/build/container-logs/**/* && exit 1)
+      - name: Test progressive upgrade
+        run: ./gradlew :tests:backward-compat:upgrade:test || (tail -n +1 tests/backward-compat/upgrade/build/reports/tests/test/classes/* && tail -n +1 tests/backward-compat/upgrade/build/container-logs/**/* && exit 1)
+      - name: Other tests
+        run: ./gradlew :tests:backward-compat:test -x tests:backward-compat:upgrade:test -x :tests:backward-compat:current-server-old-clients:test

bin/common_gradle.sh

@@ -139,8 +139,8 @@ CLI_GC_OPTS=${CLI_GC_OPTS:-"${DEFAULT_CLI_GC_OPTS}"}
 CLI_GC_LOGGING_OPTS=${CLI_GC_LOGGING_OPTS:-"${DEFAULT_CLI_GC_LOGGING_OPTS}"}
 
 # module names
-BOOKIE_SERVER_MODULE_NAME="bookkeeper-server"
-TABLE_SERVICE_MODULE_NAME="stream-storage-server"
+BOOKIE_SERVER_MODULE_NAME="(org.apache.bookkeeper-)?bookkeeper-server"
+TABLE_SERVICE_MODULE_NAME="(org.apache.bookkeeper-)?stream-storage-server"
 
 is_released_binary() {
   if [ -d ${BK_HOME}/lib ]; then

bookkeeper-server/src/main/java/org/apache/bookkeeper/client/RackawareEnsemblePlacementPolicyImpl.java

@@ -1035,8 +1035,8 @@ public PlacementPolicyAdherence isEnsembleAdheringToPlacementPolicy(List<BookieI
                     LOG.warn("Received exception while trying to get network location of bookie: {}", bookie, e);
                 }
             }
-            if ((racksInQuorum.size() < minNumRacksPerWriteQuorumForThisEnsemble)
-                    || (enforceMinNumRacksPerWriteQuorum && racksInQuorum.contains(getDefaultRack()))) {
+            if (enforceMinNumRacksPerWriteQuorum && (racksInQuorum.size() < minNumRacksPerWriteQuorumForThisEnsemble
+                    || racksInQuorum.contains(getDefaultRack()))) {
                 return PlacementPolicyAdherence.FAIL;
             }
         }

build.gradle

@@ -148,6 +148,7 @@ allprojects {
 
         plugins.withType(DistributionPlugin) {
             distTar {
+                archiveClassifier = "bin"
                 compression = Compression.GZIP
                 archiveExtension = 'tar.gz'
             }
@@ -184,7 +185,6 @@ allprojects {
         signing {
             def skipSigning = project.hasProperty('skipSigning') && skipSigning.toBoolean()
             def shouldSign = !skipSigning
-
             if (shouldSign) {
                 if (project.hasProperty("singingKey")) {
                     // The following allow the secretKey and password to be specified using env var
@@ -227,6 +227,12 @@ allprojects {
                     });
                 }
             }
+            systemProperty "gradle.buildDirectory", project.buildDir
+            systemProperty "currentVersion", project.rootProject.version
+            testLogging {
+                outputs.upToDateWhen {false}
+                showStandardStreams = true
+            }
         }
     }
 

settings.gradle

@@ -78,6 +78,7 @@ include(':bookkeeper-benchmark',
         'stream:storage:api',
         'stream:storage:impl',
         'stream:tests-common',
+        'tests:docker-images',
         'tests:backward-compat',
         'tests:backward-compat:bc-non-fips',
         'tests:backward-compat:current-server-old-clients',

tests/backward-compat/build.gradle

@@ -38,6 +38,15 @@ subprojects {
         testImplementation depLibs.junit
         testImplementation depLibs.slf4j
         testImplementation depLibs.arquillianCubeDocker
+        testImplementation depLibs.zookeeper
     }
 
+    test {
+        dependsOn(":tests:docker-images:buildImages")
+    }
+
+}
+
+test {
+    subprojects.each { dependsOn("${it.name}:test") }
 }

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/AbstractClientFencingTest.groovy

@@ -0,0 +1,97 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you 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 org.apache.bookkeeper.tests.backwardcompat
+
+import com.github.dockerjava.api.DockerClient
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.MavenClassLoader
+import org.jboss.arquillian.test.api.ArquillianResource
+import org.junit.Assert
+import org.junit.Before
+
+abstract class AbstractClientFencingTest {
+    protected static byte[] PASSWD = "foobar".getBytes()
+
+    @ArquillianResource
+    protected DockerClient docker
+
+    protected String currentVersion = BookKeeperClusterUtils.CURRENT_VERSION
+
+    @Before
+    public void before() throws Exception {
+        Assert.assertTrue(BookKeeperClusterUtils.stopAllBookies(docker))
+        // First test to run, formats metadata and bookies
+        if (BookKeeperClusterUtils.metadataFormatIfNeeded(docker, currentVersion)) {
+            BookKeeperClusterUtils.formatAllBookies(docker, currentVersion)
+        }
+        // If already started, this has no effect
+        Assert.assertTrue(BookKeeperClusterUtils.startAllBookiesWithVersion(docker, currentVersion))
+    }
+
+    protected void testFencingOldClient(String oldClientVersion, String fencingVersion) {
+        String zookeeper = BookKeeperClusterUtils.zookeeperConnectString(docker)
+
+        def oldCL = MavenClassLoader.forBookKeeperVersion(oldClientVersion)
+        def oldBK = oldCL.newBookKeeper(zookeeper)
+        def fencingCL = MavenClassLoader.forBookKeeperVersion(fencingVersion)
+        def fencingBK = fencingCL.newBookKeeper(zookeeper)
+
+        try {
+            def numEntries = 5
+            def ledger0 = oldBK.createLedger(3, 2,
+                    oldCL.digestType("CRC32"),
+                    PASSWD)
+            for (int i = 0; i < numEntries; i++) {
+                ledger0.addEntry(("foobar" + i).getBytes())
+            }
+            ledger0.close()
+
+
+            def ledger1 = fencingBK.openLedger(ledger0.getId(), fencingCL.digestType("CRC32"), PASSWD)
+
+            // cannot write any more
+            try {
+                ledger0.addEntry("shouldn't work".getBytes())
+                Assert.fail("Shouldn't have been able to add any more")
+            } catch (Exception e) {
+                Assert.assertEquals(e.getClass().getName(),
+                        "org.apache.bookkeeper.client.BKException\$BKLedgerClosedException")
+            }
+
+            // should be able to open it and read it back
+            def ledger2 = oldBK.openLedger(ledger0.getId(), oldCL.digestType("CRC32"), PASSWD)
+            def entries = ledger2.readEntries(0, ledger2.getLastAddConfirmed())
+            Assert.assertEquals(numEntries, ledger2.getLastAddConfirmed() + 1 /* counts from 0 */)
+            int j = 0
+            while (entries.hasMoreElements()) {
+                def e = entries.nextElement()
+                Assert.assertEquals(new String(e.getEntry()), "foobar"+ j)
+                j++
+            }
+            ledger2.close()
+        } catch(Throwable tt) {
+            throw new RuntimeException("Got exception with oldClientVersion: " + oldClientVersion + " and fencingVersion: " + fencingVersion, tt);
+        } finally {
+            oldBK.close()
+            oldCL.close()
+
+            fencingBK.close()
+            fencingCL.close()
+        }
+    }
+}

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatNewClientFencesOldClient.groovy

@@ -0,0 +1,37 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you 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 org.apache.bookkeeper.tests.backwardcompat
+
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.ThreadReaper
+import org.jboss.arquillian.junit.Arquillian
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(Arquillian.class)
+class TestCompatNewClientFencesOldClient extends AbstractClientFencingTest {
+
+    @Test
+    public void testNewClientFencesOldClient() throws Exception {
+        BookKeeperClusterUtils.OLD_CLIENT_VERSIONS.each{
+            def version = it
+            ThreadReaper.runWithReaper({ testFencingOldClient(version, currentVersion) })
+        }
+    }
+
+}

tests/backward-compat/current-server-old-clients/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatOldClientFencesOldClient.groovy

@@ -0,0 +1,40 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you 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 org.apache.bookkeeper.tests.backwardcompat
+
+import com.github.dockerjava.api.DockerClient
+import org.apache.bookkeeper.tests.integration.utils.BookKeeperClusterUtils
+import org.apache.bookkeeper.tests.integration.utils.ThreadReaper
+import org.jboss.arquillian.junit.Arquillian
+import org.jboss.arquillian.test.api.ArquillianResource
+import org.junit.Assert
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(Arquillian.class)
+class TestCompatOldClientFencesOldClient extends AbstractClientFencingTest {
+
+    @Test
+    public void testOldClientFencesOldClient() throws Exception {
+        BookKeeperClusterUtils.OLD_CLIENT_VERSIONS.each{
+            def version = it
+            ThreadReaper.runWithReaper({ testFencingOldClient(version, version) })
+        }
+    }
+}