HBASE-29248 HBASE-28529 made an incompatible change to hbase.zookeeper.property handling (#6896)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit 26b026c)

Author: stoty

hbase-common/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKConfig.java

@@ -34,14 +34,16 @@
  * Utility methods for reading, and building the ZooKeeper configuration. The order and priority for
  * reading the config are as follows:
  * <ol>
- * <li>Property with "hbase.zookeeper.property." prefix from HBase XML.</li>
+ * <li>Property with "hbase.zookeeper.property." prefix from HBase XML is added with "zookeeper."
+ * prefix</li>
  * <li>other zookeeper related properties in HBASE XML</li>
  * </ol>
  */
 @InterfaceAudience.Private
 public final class ZKConfig {
 
   private static final String VARIABLE_START = "${";
+  private static final String ZOOKEEPER_JAVA_PROPERTY_PREFIX = "zookeeper.";
 
   private ZKConfig() {
   }
@@ -53,14 +55,29 @@ private ZKConfig() {
    * @return Properties holding mappings representing ZooKeeper config file.
    */
   public static Properties makeZKProps(Configuration conf) {
-    return makeZKPropsFromHbaseConfig(conf);
+    return makeZKServerPropsFromHBaseConfig(conf);
+  }
+
+  private static Properties extractZKClientPropsFromHBaseConfig(final Configuration conf) {
+    return extractZKPropsFromHBaseConfig(conf, ZOOKEEPER_JAVA_PROPERTY_PREFIX);
+  }
+
+  // This is only used for the in-process ZK Quorums used mainly for testing, not for
+  // deployments with an external Zookeeper.
+  private static Properties extractZKServerPropsFromHBaseConfig(final Configuration conf) {
+    return extractZKPropsFromHBaseConfig(conf, "");
   }
 
   /**
-   * Directly map all the hbase.zookeeper.property.KEY properties. Synchronize on conf so no loading
-   * of configs while we iterate
+   * Map all hbase.zookeeper.property.KEY properties to targetPrefix.KEY. Synchronize on conf so no
+   * loading of configs while we iterate This is rather messy, as we use the same prefix for both
+   * ZKClientConfig and for the HQuorum properties. ZKClientConfig props all have the zookeeper.
+   * prefix, while the HQuorum server props don't, and ZK automagically sets a system property
+   * adding a zookeeper. prefix to the non HQuorum properties, so we need to add the "zookeeper."
+   * prefix for ZKClientConfig but not for the HQuorum props.
    */
-  private static Properties extractZKPropsFromHBaseConfig(final Configuration conf) {
+  private static Properties extractZKPropsFromHBaseConfig(final Configuration conf,
+    final String targetPrefix) {
     Properties zkProperties = new Properties();
 
     synchronized (conf) {
@@ -73,7 +90,7 @@ private static Properties extractZKPropsFromHBaseConfig(final Configuration conf
           if (value.contains(VARIABLE_START)) {
             value = conf.get(key);
           }
-          zkProperties.setProperty(zkKey, value);
+          zkProperties.setProperty(targetPrefix + zkKey, value);
         }
       }
     }
@@ -82,13 +99,14 @@ private static Properties extractZKPropsFromHBaseConfig(final Configuration conf
   }
 
   /**
-   * Make a Properties object holding ZooKeeper config. Parses the corresponding config options from
-   * the HBase XML configs and generates the appropriate ZooKeeper properties.
+   * Make a Properties object holding ZooKeeper config for the optional in-process ZK Quorum
+   * servers. Parses the corresponding config options from the HBase XML configs and generates the
+   * appropriate ZooKeeper properties.
    * @param conf Configuration to read from.
    * @return Properties holding mappings representing ZooKeeper config file.
    */
-  private static Properties makeZKPropsFromHbaseConfig(Configuration conf) {
-    Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
+  private static Properties makeZKServerPropsFromHBaseConfig(Configuration conf) {
+    Properties zkProperties = extractZKServerPropsFromHBaseConfig(conf);
 
     // If clientPort is not set, assign the default.
     if (zkProperties.getProperty(HConstants.CLIENT_PORT_STR) == null) {
@@ -325,7 +343,7 @@ public String getZnodeParent() {
   }
 
   public static ZKClientConfig getZKClientConfig(Configuration conf) {
-    Properties zkProperties = extractZKPropsFromHBaseConfig(conf);
+    Properties zkProperties = extractZKClientPropsFromHBaseConfig(conf);
     ZKClientConfig zkClientConfig = new ZKClientConfig();
     zkProperties.forEach((k, v) -> zkClientConfig.setProperty(k.toString(), v.toString()));
     return zkClientConfig;

hbase-common/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKConfig.java

@@ -106,14 +106,36 @@ public void testZooKeeperTlsProperties() {
     Configuration conf = HBaseConfiguration.create();
     for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
       conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + p, p);
+      String zkprop = "zookeeper." + p;
+      System.clearProperty(zkprop);
     }
 
     // Act
     ZKClientConfig zkClientConfig = ZKConfig.getZKClientConfig(conf);
 
     // Assert
     for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
-      assertEquals("Invalid or unset system property: " + p, p, zkClientConfig.getProperty(p));
+      assertEquals("Invalid or unset system property: " + p, p,
+        zkClientConfig.getProperty("zookeeper." + p));
+    }
+  }
+
+  @Test
+  public void testZooKeeperTlsPropertiesHQuorumPeer() {
+    // Arrange
+    Configuration conf = HBaseConfiguration.create();
+    for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
+      conf.set(HConstants.ZK_CFG_PROPERTY_PREFIX + p, p);
+      String zkprop = "zookeeper." + p;
+      System.clearProperty(zkprop);
+    }
+
+    // Act
+    Properties zkProps = ZKConfig.makeZKProps(conf);
+
+    // Assert
+    for (String p : ZOOKEEPER_CLIENT_TLS_PROPERTIES) {
+      assertEquals("Invalid or unset system property: " + p, p, zkProps.getProperty(p));
     }
   }