Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/228#discussion_r105457380
--- Diff:
minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java
---
@@ -149,9 +147,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, serverEnv,
accumuloPath, Admin.class.getName(), "stopAll"};
--- End diff --
I wasn't assuming anything. I was observing my own ignorance about the
reasons why this was being done. In any case, you answered the question I
should have asked, instead ("Why is this being done?").
Your response indicates that the reason this is being done is so that this
test framework can switch users to control an external running Accumulo
instance, running as a specific user, assuming `sudo` is configured in a very
particular way to allow running the necessary executables without a tty or
authentication.
I now understand why it uses `sudo`, but its inclusion suggests that this
code exists not to test the upstream Accumulo project, but to test specific
downstream products. That makes me a bit uncomfortable because I don't like the
idea of the upstream project being burdened with the maintenance of tests for
downstream vendors. However, I can also see it the other way around: that the
upstream project has an interest in providing some test code to ensure any
downstream packaging meets some minimum standards of quality so that Accumulo.
So, I'm not necessarily going to argue we should remove it.
I would, however, argue that we "genericize" it a bit, so we don't have to
directly support the use of privilege escalation features of an operating
system for our tests. We can support that indirectly by allowing the user to
supply the launch command. The direct use of `sudo` is mostly what makes me
uncomfortable. I've seen a lot of open source projects code, and this is the
first time I've ever seen that used in any project's test suite that wasn't
`sudo` itself.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
