Issue 2795: Bookkeeper upgrade using Bookie ID may fail due to cookie…

… mismatch (#2796)
apache · Oct 13, 2021 · 594a056 · 594a056
1 parent 354cf37
commit 594a056
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 18 deletions.
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java
@@ -286,26 +286,27 @@ static List<BookieId> possibleBookieIds(ServerConfiguration conf)
         // we are checking all possibilities here, so we don't need to fail if we can only get
         // loopback address. it will fail anyway when the bookie attempts to listen on loopback address.
         try {
-            // ip address
-            addresses.add(getBookieAddress(
-                new ServerConfiguration(conf)
-                    .setUseHostNameAsBookieID(false)
-                    .setAdvertisedAddress(null)
-                    .setAllowLoopback(true)
-            ).toBookieId());
-            // host name
-            addresses.add(getBookieAddress(
-                new ServerConfiguration(conf)
-                    .setUseHostNameAsBookieID(true)
-                    .setAdvertisedAddress(null)
-                    .setAllowLoopback(true)
-            ).toBookieId());
-            // advertised address
-            if (null != conf.getAdvertisedAddress()) {
-                addresses.add(getBookieAddress(conf).toBookieId());
-            }
             if (null != conf.getBookieId()) {
                 addresses.add(BookieId.parse(conf.getBookieId()));
+            } else {
+                // ip address
+                addresses.add(getBookieAddress(
+                    new ServerConfiguration(conf)
+                        .setUseHostNameAsBookieID(false)
+                        .setAdvertisedAddress(null)
+                        .setAllowLoopback(true)
+                ).toBookieId());
+                // host name
+                addresses.add(getBookieAddress(
+                    new ServerConfiguration(conf)
+                        .setUseHostNameAsBookieID(true)
+                        .setAdvertisedAddress(null)
+                        .setAllowLoopback(true)
+                ).toBookieId());
+                // advertised address
+                if (null != conf.getAdvertisedAddress()) {
+                    addresses.add(getBookieAddress(conf).toBookieId());
+                }
             }
         } catch (UnknownHostException e) {
             throw new UnknownBookieIdException(e);

diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
@@ -1614,4 +1614,40 @@ private void testInvalidServiceMetadataURICase(String uri) throws Exception {
             // ok
         }
     }
+
+    @Test
+    public void testBookieIdSetting() throws Exception {
+        String customBookieId = "customId";
+        // If BookieID is set, it takes precedence over network info.
+        final ServerConfiguration conf = newServerConfiguration().setBookieId(customBookieId);
+        BookieServer server = new MockBookieServer(conf);
+        server.start();
+        assertEquals(customBookieId, server.getBookieId().toString());
+        server.shutdown();
+    }
+
+    @Test
+    public void testBookieIdChange() throws Exception {
+        // By default, network info is set as Bookie Id and it is stored in the Cookie.
+        final ServerConfiguration conf = newServerConfiguration();
+        BookieServer server = new MockBookieServer(conf);
+        server.start();
+        assertNotNull(server.getBookieId().toString());
+        server.shutdown();
+
+        // If BookieID is set, it takes precedence over network info. Because of that, the new Bookie start
+        // should fail with an InvalidCookieException, as now the custom BookieID takes precedence.
+        String customBookieId = "customId";
+        conf.setBookieId(customBookieId);
+        try {
+            server = new MockBookieServer(conf);
+        } catch (BookieException.InvalidCookieException e) {
+            // This is the expected case, as the customBookieId prevails over the default one.
+        } catch (Exception e) {
+            // Unexpected exception, failing.
+            Assert.fail();
+        }
+        assertEquals(customBookieId, server.getBookieId().toString());
+        server.shutdown();
+    }
 }