[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

classic Classic list List threaded Threaded
49 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
GitHub user mikewalch opened a pull request:

    https://github.com/apache/accumulo/pull/228

    ACCUMULO-4596 Improvements to standalone cluster testing

    * Limited use of bash env variables in standalone cluster testing
    * Fixed standalone cluster testing sow that it work with Accumulo 2.0
      script changes
    * Users now only need to configure Accumulo conf directory on client

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mikewalch/accumulo standalone

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/228.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #228
   
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #228: ACCUMULO-4596 Improvements to standalone cluster testin...

ctubbsii
Github user mikewalch commented on the issue:

    https://github.com/apache/accumulo/pull/228
 
    I tested these changes by running `ReadWriteIT` against and a local cluster started by uno.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104527998
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java ---
    @@ -57,7 +57,7 @@
     
       private Instance instance;
       private ClientConfiguration clientConf;
    -  private String accumuloHome, clientAccumuloConfDir, serverAccumuloConfDir, hadoopConfDir;
    --- End diff --
   
    There is a reason that both of these exist. Some tests do things that require knowledge of the `instance.secret` (generally, configuration values which are sensitive). Good deployment practices dictate that there is a separation here -- as such, I don't see how this simplification is helpful.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104528620
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    Ah, this is also a good example of what would be broken by your change.
   
    We should not be requiring that a user be privileged to read the Accumulo services' configuration files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104528417
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -123,11 +114,12 @@ public int exec(Class<?> clz, String[] args) throws IOException {
     
       public Entry<Integer,String> execMapreduceWithStdout(Class<?> clz, String[] args) throws IOException {
         String host = "localhost";
    -    String[] cmd = new String[3 + args.length];
    -    cmd[0] = getToolPath();
    -    cmd[1] = getJarFromClass(clz);
    -    cmd[2] = clz.getName();
    -    for (int i = 0, j = 3; i < args.length; i++, j++) {
    +    String[] cmd = new String[4 + args.length];
    +    cmd[0] = getAccumuloUtilPath();
    +    cmd[1] = "hadoop-jar";
    +    cmd[2] = getJarFromClass(clz);
    +    cmd[3] = clz.getName();
    --- End diff --
   
    This is nice. How do this fail in the case of mis-configuration?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104527430
 
    --- Diff: TESTING.md ---
    @@ -117,8 +117,8 @@ The following properties can be used to configure a standalone cluster:
     - `accumulo.it.cluster.standalone.zookeepers`, Required: ZooKeeper quorum used by the standalone cluster
     - `accumulo.it.cluster.standalone.instance.name`, Required: Accumulo instance name for the cluster
     - `accumulo.it.cluster.standalone.hadoop.conf`, Required: `HADOOP_CONF_DIR`
    -- `accumulo.it.cluster.standalone.home`, Optional: `ACCUMULO_HOME`
    -- `accumulo.it.cluster.standalone.conf`, Optional: `ACCUMULO_CONF_DIR`
    +- `accumulo.it.cluster.standalone.home`, Required: Accumulo home directory on cluster
    --- End diff --
   
    s/Accumulo home directory/Accumulo installation directory/ ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104527380
 
    --- Diff: TESTING.md ---
    @@ -117,8 +117,8 @@ The following properties can be used to configure a standalone cluster:
     - `accumulo.it.cluster.standalone.zookeepers`, Required: ZooKeeper quorum used by the standalone cluster
     - `accumulo.it.cluster.standalone.instance.name`, Required: Accumulo instance name for the cluster
     - `accumulo.it.cluster.standalone.hadoop.conf`, Required: `HADOOP_CONF_DIR`
    --- End diff --
   
    Change this one too if you changed the other two, please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104535648
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java ---
    @@ -57,7 +57,7 @@
     
       private Instance instance;
       private ClientConfiguration clientConf;
    -  private String accumuloHome, clientAccumuloConfDir, serverAccumuloConfDir, hadoopConfDir;
    --- End diff --
   
    OK, I will bring back client and server config directories.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104537251
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    Not sure about your comment.  It looks like it was previously set for user to read Accumulo services' configuration files..
   
    ```java
    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104678440
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -123,11 +114,12 @@ public int exec(Class<?> clz, String[] args) throws IOException {
     
       public Entry<Integer,String> execMapreduceWithStdout(Class<?> clz, String[] args) throws IOException {
         String host = "localhost";
    -    String[] cmd = new String[3 + args.length];
    -    cmd[0] = getToolPath();
    -    cmd[1] = getJarFromClass(clz);
    -    cmd[2] = clz.getName();
    -    for (int i = 0, j = 3; i < args.length; i++, j++) {
    +    String[] cmd = new String[4 + args.length];
    +    cmd[0] = getAccumuloUtilPath();
    +    cmd[1] = "hadoop-jar";
    +    cmd[2] = getJarFromClass(clz);
    +    cmd[3] = clz.getName();
    --- End diff --
   
    It fails just like the previous `tool.sh` command as `accumulo-util hadoop-jar` is the same underlying logic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #228: ACCUMULO-4596 Improvements to standalone cluster testin...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on the issue:

    https://github.com/apache/accumulo/pull/228
 
    @joshelser, let me know if my changes now work for you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104678703
 
    --- Diff: TESTING.md ---
    @@ -117,8 +117,8 @@ The following properties can be used to configure a standalone cluster:
     - `accumulo.it.cluster.standalone.zookeepers`, Required: ZooKeeper quorum used by the standalone cluster
     - `accumulo.it.cluster.standalone.instance.name`, Required: Accumulo instance name for the cluster
     - `accumulo.it.cluster.standalone.hadoop.conf`, Required: `HADOOP_CONF_DIR`
    --- End diff --
   
    Fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104678751
 
    --- Diff: TESTING.md ---
    @@ -117,8 +117,8 @@ The following properties can be used to configure a standalone cluster:
     - `accumulo.it.cluster.standalone.zookeepers`, Required: ZooKeeper quorum used by the standalone cluster
     - `accumulo.it.cluster.standalone.instance.name`, Required: Accumulo instance name for the cluster
     - `accumulo.it.cluster.standalone.hadoop.conf`, Required: `HADOOP_CONF_DIR`
    -- `accumulo.it.cluster.standalone.home`, Optional: `ACCUMULO_HOME`
    -- `accumulo.it.cluster.standalone.conf`, Optional: `ACCUMULO_CONF_DIR`
    +- `accumulo.it.cluster.standalone.home`, Required: Accumulo home directory on cluster
    --- End diff --
   
    Fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104992602
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    Same issue as above with dropping the server conf (so we can probably ignore it). `StopAll` is another utility which requires the correct `instance.secret`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104992750
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -123,11 +114,12 @@ public int exec(Class<?> clz, String[] args) throws IOException {
     
       public Entry<Integer,String> execMapreduceWithStdout(Class<?> clz, String[] args) throws IOException {
         String host = "localhost";
    -    String[] cmd = new String[3 + args.length];
    -    cmd[0] = getToolPath();
    -    cmd[1] = getJarFromClass(clz);
    -    cmd[2] = clz.getName();
    -    for (int i = 0, j = 3; i < args.length; i++, j++) {
    +    String[] cmd = new String[4 + args.length];
    +    cmd[0] = getAccumuloUtilPath();
    +    cmd[1] = "hadoop-jar";
    +    cmd[2] = getJarFromClass(clz);
    +    cmd[3] = clz.getName();
    --- End diff --
   
    Okie doke. Half of this was me wondering how it fails when the configuration is actually wrong (e.g. we can't find the hadoop installation/jars), but that isn't related to this changeset.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo issue #228: ACCUMULO-4596 Improvements to standalone cluster testin...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/228
 
    > let me know if my changes now work for you.
   
    Changes look OK to me now. Thanks for backing out those config changes.
   
    Have you tested (a subset of) the ITs using a standalone cluster to make sure things still work? If you have confirmed that, +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user mikewalch commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104996774
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    So you are not OK with the removal of `ACCUMULO_CONF_DIR + serverAccumuloConfDir`? Why do you need this? I am assuming the Accumulo installation has all server config in the default configuration directory.  We should be getting away from using `ACCUMULO_CONF_DIR` as it setting these env variables is very prone to errors.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104997993
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    > So you are not OK with the removal of ACCUMULO_CONF_DIR + serverAccumuloConfDir
   
    Oh, no, I'm not because you're still breaking things. I didn't realize you didn't revert this change.
   
    > Why do you need this?
   
    See my previous comment: "StopAll is another utility which requires the correct instance.secret.". It's the same reason I -1'ed the removal of the accumuloServerConf property in the first place..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104998228
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -170,9 +162,8 @@ public void adminStopAll() throws IOException {
       public void setGoalState(String goalState) throws IOException {
         requireNonNull(goalState, "Goal state must not be null");
         checkArgument(MasterGoalState.valueOf(goalState) != null, "Unknown goal state: " + goalState);
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, SetGoalState.class.getName(), goalState};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, SetGoalState.class.getName(), goalState};
    --- End diff --
   
    Again, same thing as the above with the `stopAll`. Pretty sure this requires the correct `instance.secret`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] accumulo pull request #228: ACCUMULO-4596 Improvements to standalone cluster...

ctubbsii
In reply to this post by ctubbsii
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/228#discussion_r104998805
 
    --- Diff: minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java ---
    @@ -149,9 +141,8 @@ String getJarFromClass(Class<?> clz) {
     
       @Override
       public void adminStopAll() throws IOException {
    -    File confDir = getConfDir();
    -    String master = getHosts(new File(confDir, "masters")).get(0);
    -    String[] cmd = new String[] {SUDO_CMD, "-u", user, ACCUMULO_CONF_DIR + serverAccumuloConfDir, accumuloPath, Admin.class.getName(), "stopAll"};
    +    String master = getHosts(MASTER_HOSTS_FILE).get(0);
    +    String[] cmd = new String[] {SUDO_CMD, "-u", user, accumuloPath, Admin.class.getName(), "stopAll"};
    --- End diff --
   
    > We should be getting away from using ACCUMULO_CONF_DIR as it setting these env variables is very prone to errors.
   
    That's fine if you want to move away from them, but I don't see a solution presented as to how you don't break existing functionality :). As long as we have to keep the tracer user's password and instance.secret in accumulo-site.xml, we're stuck having a copy of those files which are not globally readable.
   
    Pushing harder on use of the client.conf is one possibility but this is in a half-cocked state, IMO.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
123