Return-Path: X-Original-To: apmail-accumulo-commits-archive@www.apache.org Delivered-To: apmail-accumulo-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 884DA182D6 for ; Wed, 24 Jun 2015 16:40:20 +0000 (UTC) Received: (qmail 87379 invoked by uid 500); 24 Jun 2015 16:40:20 -0000 Delivered-To: apmail-accumulo-commits-archive@accumulo.apache.org Received: (qmail 87275 invoked by uid 500); 24 Jun 2015 16:40:20 -0000 Mailing-List: contact commits-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list commits@accumulo.apache.org Received: (qmail 87228 invoked by uid 99); 24 Jun 2015 16:40:20 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 24 Jun 2015 16:40:20 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 3B097E3607; Wed, 24 Jun 2015 16:40:20 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: elserj@apache.org To: commits@accumulo.apache.org Date: Wed, 24 Jun 2015 16:40:22 -0000 Message-Id: <53a515b86a0443b796f531515a26e3f6@git.apache.org> In-Reply-To: <0cfa8728bf324abebddbd8b92b5deb72@git.apache.org> References: <0cfa8728bf324abebddbd8b92b5deb72@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [3/6] accumulo git commit: ACCUMULO-3779 Restore proper use zookeepers and add clientconf warning. ACCUMULO-3779 Restore proper use zookeepers and add clientconf warning. Warn when we don't find a client configuration file to use in any of the normal locations and ensure that use of ZK goes from shell option, to client conf to accumulo site. Conflicts: core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/af058765 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/af058765 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/af058765 Branch: refs/heads/master Commit: af058765d6286198719b0f2d9b6fcffbdb5a6add Parents: e785da1 Author: Josh Elser Authored: Thu May 7 16:01:45 2015 -0400 Committer: Josh Elser Committed: Wed Jun 24 12:33:55 2015 -0400 ---------------------------------------------------------------------- .../core/client/ClientConfiguration.java | 8 ++++ .../apache/accumulo/core/util/shell/Shell.java | 40 +++++++++++++------- .../core/util/shell/ShellOptionsJC.java | 12 ++++++ .../core/util/shell/ShellConfigTest.java | 37 ++++++++++++++++++ .../core/util/shell/ShellSetInstanceTest.java | 15 +++++--- 5 files changed, 93 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/af058765/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java index a71b7e1..2a7a48c 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java @@ -31,6 +31,8 @@ import org.apache.commons.configuration.CompositeConfiguration; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Contains a list of property keys recognized by the Accumulo client and convenience methods for setting them. @@ -38,6 +40,8 @@ import org.apache.commons.configuration.PropertiesConfiguration; * @since 1.6.0 */ public class ClientConfiguration extends CompositeConfiguration { + private static final Logger log = LoggerFactory.getLogger(ClientConfiguration.class); + public static final String USER_ACCUMULO_DIR_NAME = ".accumulo"; public static final String USER_CONF_FILENAME = "config"; public static final String GLOBAL_CONF_FILENAME = "client.conf"; @@ -146,6 +150,10 @@ public class ClientConfiguration extends CompositeConfiguration { configs.add(new PropertiesConfiguration(conf)); } } + // We couldn't find the client configuration anywhere + if (configs.isEmpty()) { + log.warn("Found no client.conf in default paths. Using default client configuration values."); + } return new ClientConfiguration(configs); } catch (ConfigurationException e) { throw new IllegalStateException("Error loading client configuration", e); http://git-wip-us.apache.org/repos/asf/accumulo/blob/af058765/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java index 1f14b6f..8cfb689 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java @@ -431,27 +431,41 @@ public class Shell extends ShellOptions { } } + /** + * Get the ZooKeepers. Use the value passed in (if there was one), then fall back to the ClientConf, finally trying the accumulo-site.xml. + * + * @param keepers + * ZooKeepers passed to the shell + * @param clientConfig + * ClientConfiguration instance + * @return The ZooKeepers to connect to + */ + static String getZooKeepers(String keepers, ClientConfiguration clientConfig, AccumuloConfiguration conf) { + if (null != keepers) { + return keepers; + } + + if (clientConfig.containsKey(ClientProperty.INSTANCE_ZK_HOST.getKey())) { + return clientConfig.get(ClientProperty.INSTANCE_ZK_HOST); + } + + return conf.get(Property.INSTANCE_ZK_HOST); + } + /* * Takes instanceName and keepers as separate arguments, rather than just packaged into the clientConfig, so that we can fail over to accumulo-site.xml or * HDFS config if they're unspecified. */ - private static Instance getZooInstance(String instanceName, String keepers, ClientConfiguration clientConfig) { + private static Instance getZooInstance(String instanceName, String keepersOption, ClientConfiguration clientConfig) { UUID instanceId = null; if (instanceName == null) { instanceName = clientConfig.get(ClientProperty.INSTANCE_NAME); } - if (keepers == null) { - keepers = clientConfig.get(ClientProperty.INSTANCE_ZK_HOST); - } - if (instanceName == null || keepers == null) { - AccumuloConfiguration conf = SiteConfiguration.getInstance(ServerConfigurationUtil.convertClientConfig(DefaultConfiguration.getInstance(), clientConfig)); - if (instanceName == null) { - Path instanceDir = new Path(VolumeConfiguration.getVolumeUris(conf)[0], "instance_id"); - instanceId = UUID.fromString(ZooUtil.getInstanceIDFromHdfs(instanceDir, conf)); - } - if (keepers == null) { - keepers = conf.get(Property.INSTANCE_ZK_HOST); - } + AccumuloConfiguration conf = SiteConfiguration.getInstance(ServerConfigurationUtil.convertClientConfig(DefaultConfiguration.getInstance(), clientConfig)); + String keepers = getZooKeepers(keepersOption, clientConfig, conf); + if (instanceName == null) { + Path instanceDir = new Path(VolumeConfiguration.getVolumeUris(conf)[0], "instance_id"); + instanceId = UUID.fromString(ZooUtil.getInstanceIDFromHdfs(instanceDir, conf)); } if (instanceId != null) { return new ZooKeeperInstance(clientConfig.withInstance(instanceId).withZkHosts(keepers)); http://git-wip-us.apache.org/repos/asf/accumulo/blob/af058765/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java index dfa24c5..ecb2695 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java @@ -26,7 +26,12 @@ import java.util.TreeMap; import org.apache.accumulo.core.client.ClientConfiguration; import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty; +import org.apache.accumulo.core.client.impl.ServerConfigurationUtil; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.DefaultConfiguration; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.conf.SiteConfiguration; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; import org.apache.log4j.Logger; @@ -275,6 +280,13 @@ public class ShellOptionsJC { if (useSsl()) { clientConfig.setProperty(ClientProperty.INSTANCE_RPC_SSL_ENABLED, "true"); } + + // Automatically try to add in the proper ZK from accumulo-site for backwards compat. + if (!clientConfig.containsKey(ClientProperty.INSTANCE_ZK_HOST.getKey())) { + AccumuloConfiguration siteConf = SiteConfiguration.getInstance(ServerConfigurationUtil.convertClientConfig(DefaultConfiguration.getInstance(), clientConfig)); + clientConfig.withZkHosts(siteConf.get(Property.INSTANCE_ZK_HOST)); + } + return clientConfig; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/af058765/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java index 47deba6..6292622 100644 --- a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java @@ -16,6 +16,7 @@ */ package org.apache.accumulo.core.util.shell; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -23,11 +24,18 @@ import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.PrintStream; import java.io.PrintWriter; +import java.nio.file.Files; +import java.util.HashMap; +import java.util.Map; import jline.console.ConsoleReader; +import org.apache.accumulo.core.client.ClientConfiguration; import org.apache.accumulo.core.client.security.tokens.PasswordToken; import org.apache.accumulo.core.util.shell.ShellTest.TestOutputStream; +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.ConfigurationCopy; +import org.apache.accumulo.core.conf.Property; import org.apache.log4j.Level; import org.junit.After; import org.junit.Before; @@ -87,4 +95,33 @@ public class ShellConfigTest { assertFalse(shell.config("--fake", "-tc", PasswordToken.class.getCanonicalName(), "-l", "password=foo", "-p", "bar")); assertTrue(output.get().contains(ParameterException.class.getCanonicalName())); } + + @Test + public void testZooKeeperHostFallBackToSite() throws Exception { + ClientConfiguration clientConfig = new ClientConfiguration(); + Map data = new HashMap(); + data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); + AccumuloConfiguration conf = new ConfigurationCopy(data); + assertEquals("site_hostname", Shell.getZooKeepers(null, clientConfig, conf)); + } + + @Test + public void testZooKeeperHostFromClientConfig() throws Exception { + ClientConfiguration clientConfig = new ClientConfiguration(); + clientConfig.withZkHosts("cc_hostname"); + Map data = new HashMap(); + data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); + AccumuloConfiguration conf = new ConfigurationCopy(data); + assertEquals("cc_hostname", Shell.getZooKeepers(null, clientConfig, conf)); + } + + @Test + public void testZooKeeperHostFromOption() throws Exception { + ClientConfiguration clientConfig = new ClientConfiguration(); + clientConfig.withZkHosts("cc_hostname"); + Map data = new HashMap(); + data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); + AccumuloConfiguration conf = new ConfigurationCopy(data); + assertEquals("opt_hostname", Shell.getZooKeepers("opt_hostname", clientConfig, conf)); + } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/af058765/core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java index 0959c35..0453beb 100644 --- a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java +++ b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java @@ -30,6 +30,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.UUID; @@ -167,18 +168,14 @@ public class ShellSetInstanceTest { expect(clientConf.get(ClientProperty.INSTANCE_NAME)).andReturn(null); } - if (!onlyHosts) { - expect(clientConf.get(ClientProperty.INSTANCE_ZK_HOST)).andReturn(null); - } - mockStatic(ConfigSanityCheck.class); ConfigSanityCheck.validate(EasyMock. anyObject()); expectLastCall().atLeastOnce(); replay(ConfigSanityCheck.class); if (!onlyHosts) { - expect(clientConf.containsKey(Property.INSTANCE_ZK_HOST.getKey())).andReturn(true).atLeastOnce(); - expect(clientConf.getString(Property.INSTANCE_ZK_HOST.getKey())).andReturn("host1,host2").atLeastOnce(); + expect(clientConf.containsKey(ClientProperty.INSTANCE_ZK_HOST.getKey())).andReturn(true).atLeastOnce(); + expect(clientConf.get(ClientProperty.INSTANCE_ZK_HOST)).andReturn("host1,host2").atLeastOnce(); expect(clientConf.withZkHosts("host1,host2")).andReturn(clientConf); } if (!onlyInstance) { @@ -224,9 +221,13 @@ public class ShellSetInstanceTest { expect(opts.isFake()).andReturn(false); expect(opts.getClientConfiguration()).andReturn(clientConf); expect(opts.isHdfsZooInstance()).andReturn(false); + expect(clientConf.getKeys()).andReturn(Arrays.asList(ClientProperty.INSTANCE_NAME.getKey(), ClientProperty.INSTANCE_ZK_HOST.getKey()).iterator()); + expect(clientConf.getString(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey())).andReturn(null); if (dashZ) { expect(clientConf.withInstance("foo")).andReturn(clientConf); + expect(clientConf.getString(ClientProperty.INSTANCE_NAME.getKey())).andReturn("foo"); expect(clientConf.withZkHosts("host1,host2")).andReturn(clientConf); + expect(clientConf.getString(ClientProperty.INSTANCE_ZK_HOST.getKey())).andReturn("host1,host2"); List zl = new java.util.ArrayList(); zl.add("foo"); zl.add("host1,host2"); @@ -234,7 +235,9 @@ public class ShellSetInstanceTest { expectLastCall().anyTimes(); } else { expect(clientConf.withInstance("bar")).andReturn(clientConf); + expect(clientConf.getString(ClientProperty.INSTANCE_NAME.getKey())).andReturn("bar"); expect(clientConf.withZkHosts("host3,host4")).andReturn(clientConf); + expect(clientConf.getString(ClientProperty.INSTANCE_ZK_HOST.getKey())).andReturn("host3,host4"); expect(opts.getZooKeeperInstance()).andReturn(Collections. emptyList()); expect(opts.getZooKeeperInstanceName()).andReturn("bar"); expect(opts.getZooKeeperHosts()).andReturn("host3,host4");