HBASE-4072 Deprecate/disable and remove support for reading ZooKeeper zoo.cfg files from the classpath

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1418775 13f79535-47bb-0310-9956-ffa450edef68

hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java

@@ -723,6 +723,10 @@ public final class HConstants {
   public static final byte [] NO_NEXT_INDEXED_KEY = toBytes("NO_NEXT_INDEXED_KEY");
   /** delimiter used between portions of a region name */
   public static final int DELIMITER = ',';
+  public static final String HBASE_CONFIG_READ_ZOOKEEPER_CONFIG =
+      "hbase.config.read.zookeeper.config";
+  public static final boolean DEFAULT_HBASE_CONFIG_READ_ZOOKEEPER_CONFIG =
+      false;
 
   /**
    * QOS attributes: these attributes are used to demarcate RPC call processing

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

@@ -36,7 +36,7 @@ import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.util.StringUtils;
 
 /**
- * Utility methods for reading, parsing, and building zookeeper configuration.
+ * Utility methods for reading, and building the ZooKeeper configuration.
  */
 @InterfaceAudience.Public
 @InterfaceStability.Evolving
@@ -49,25 +49,43 @@ public class ZKConfig {
   private static final int VARIABLE_END_LENGTH = VARIABLE_END.length();
 
   /**
-   * Make a Properties object holding ZooKeeper config equivalent to zoo.cfg.
-   * If there is a zoo.cfg in the classpath, simply read it in. Otherwise parse
-   * the corresponding config options from the HBase XML configs and generate
-   * the appropriate ZooKeeper properties.
+   * Make a Properties object holding ZooKeeper config.
+   * 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 zoo.cfg file.
+   * @return Properties holding mappings representing ZooKeeper config file.
    */
   public static Properties makeZKProps(Configuration conf) {
-    // First check if there is a zoo.cfg in the CLASSPATH. If so, simply read
-    // it and grab its configuration properties.
-    ClassLoader cl = HQuorumPeer.class.getClassLoader();
-    final InputStream inputStream =
-      cl.getResourceAsStream(HConstants.ZOOKEEPER_CONFIG_NAME);
-    if (inputStream != null) {
-      try {
-        return parseZooCfg(conf, inputStream);
-      } catch (IOException e) {
-        LOG.warn("Cannot read " + HConstants.ZOOKEEPER_CONFIG_NAME +
-                 ", loading from XML files", e);
+    if (conf.getBoolean(HConstants.HBASE_CONFIG_READ_ZOOKEEPER_CONFIG,
+        false)) {
+      LOG.warn(
+          "Parsing ZooKeeper's " + HConstants.ZOOKEEPER_CONFIG_NAME +
+          " file for ZK properties " +
+          "has been deprecated. Please instead place all ZK related HBase " +
+          "configuration under the hbase-site.xml, using prefixes " +
+          "of the form '" + HConstants.ZK_CFG_PROPERTY_PREFIX + "', and " +
+          "set property '" + HConstants.HBASE_CONFIG_READ_ZOOKEEPER_CONFIG +
+          "' to false");
+      // First check if there is a zoo.cfg in the CLASSPATH. If so, simply read
+      // it and grab its configuration properties.
+      ClassLoader cl = HQuorumPeer.class.getClassLoader();
+      final InputStream inputStream =
+        cl.getResourceAsStream(HConstants.ZOOKEEPER_CONFIG_NAME);
+      if (inputStream != null) {
+        try {
+          return parseZooCfg(conf, inputStream);
+        } catch (IOException e) {
+          LOG.warn("Cannot read " + HConstants.ZOOKEEPER_CONFIG_NAME +
+                   ", loading from XML files", e);
+        }
+      }
+    } else {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(
+            "Skipped reading ZK properties file '" +
+            HConstants.ZOOKEEPER_CONFIG_NAME +
+            "' since '" + HConstants.HBASE_CONFIG_READ_ZOOKEEPER_CONFIG +
+            "' was not set to true");
       }
     }
 
@@ -117,7 +135,10 @@ public class ZKConfig {
    * @param inputStream InputStream to read from.
    * @return Properties parsed from config stream with variables substituted.
    * @throws IOException if anything goes wrong parsing config
+   * @deprecated in 0.96 onwards. HBase will no longer rely on zoo.cfg
+   * availability.
    */
+  @Deprecated
   public static Properties parseZooCfg(Configuration conf,
       InputStream inputStream) throws IOException {
     Properties properties = new Properties();
@@ -220,8 +241,10 @@ public class ZKConfig {
     }
 
     if (servers.isEmpty()) {
-      LOG.fatal("No server.X lines found in conf/zoo.cfg. HBase must have a " +
-                "ZooKeeper cluster configured for its operation.");
+      LOG.fatal("No servers were found in provided ZooKeeper configuration. " +
+          "HBase must have a ZooKeeper cluster configured for its " +
+          "operation. Ensure that you've configured '" +
+          HConstants.ZOOKEEPER_QUORUM + "' properly.");
       return null;
     }
 

hbase-server/src/main/resources/hbase-default.xml

@@ -912,4 +912,14 @@
     <value>600000</value>
     <description>Timeout value for the Catalog Janitor from the master to META.</description>
   </property>
+  <property>
+    <name>hbase.config.read.zookeeper.config</name>
+    <value>false</value>
+    <description>
+        Set to true to allow HBaseConfiguration to read the
+        zoo.cfg file for ZooKeeper properties. Switching this to true
+        is not recommended, since the functionality of reading ZK
+        properties from a zoo.cfg file has been deprecated.
+    </description>
+  </property>
 </configuration>

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

@@ -0,0 +1,56 @@
+/**
+ * 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.hadoop.hbase.zookeeper;
+
+import java.util.Properties;
+
+import junit.framework.Assert;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.SmallTests;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(SmallTests.class)
+public class TestZKConfig {
+  @Test
+  public void testZKConfigLoading() throws Exception {
+    // Test depends on test resource 'zoo.cfg' at src/test/resources/zoo.cfg
+    Configuration conf = HBaseConfiguration.create();
+    // Test that by default we do not pick up any property from the zoo.cfg
+    // since that feature is to be deprecated and removed. So we should read only
+    // from the config instance (i.e. via hbase-default.xml and hbase-site.xml)
+    conf.setInt(HConstants.ZOOKEEPER_CLIENT_PORT, 2181);
+    Properties props = ZKConfig.makeZKProps(conf);
+    Assert.assertEquals(
+        "Property client port should have been default from the HBase config",
+        "2181",
+        props.getProperty("clientPort"));
+    // Test deprecated zoo.cfg read support by explicitly enabling it and
+    // thereby relying on our test resource zoo.cfg to be read.
+    // We may remove this test after a higher release (i.e. post-deprecation).
+    conf.setBoolean(HConstants.HBASE_CONFIG_READ_ZOOKEEPER_CONFIG, true);
+    props = ZKConfig.makeZKProps(conf);
+    Assert.assertEquals(
+        "Property client port should have been from zoo.cfg",
+        "9999",
+        props.getProperty("clientPort"));
+  }
+}

hbase-server/src/test/resources/zoo.cfg

@@ -0,0 +1,43 @@
+#
+# 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.
+#
+# The number of milliseconds of each tick
+tickTime=2000
+# The number of ticks that the initial
+# synchronization phase can take
+initLimit=10
+# The number of ticks that can pass between
+# sending a request and getting an acknowledgement
+syncLimit=5
+# the directory where the snapshot is stored.
+# do not use /tmp for storage, /tmp here is just
+# example sakes.
+dataDir=/tmp/hbase-test-zookeeper-deleteme
+# the port at which the clients will connect
+clientPort=9999
+#
+# Be sure to read the maintenance section of the
+# administrator guide before turning on autopurge.
+#
+# http://zookeeper.apache.org/doc/current/zookeeperAdmin.html#sc_maintenance
+#
+# The number of snapshots to retain in dataDir
+autopurge.snapRetainCount=3
+# Purge task interval in hours
+# Set to "0" to disable auto purge feature
+autopurge.purgeInterval=1
+
+server.1=i-am-a-test-server:7999:8999