[MESOS-3142] As a Developer I want a better way to run shell commands

• Type: Story
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.23.0
• Fix Version/s: 0.24.0
• Component/s: stout
• Labels: mesosphere tech-debt
• Sprint: Mesosphere Sprint 16
• Story Points: 2
• Assignee: Marco Massenzio
• Reporter: Benjamin Hindman
• Shepherd: Benjamin Hindman
• Created: 24/Jul/15 06:50
• Updated: 26/Nov/18 12:53
• Resolved: 26/Nov/18 12:53

Description

When reviewing the code in r/36425 benjaminhindman noticed that there is a better abstraction that is possible to introduce for os::shell() that will simplify the caller's life.

Instead of having to handle all possible outcomes, we propose to refactor os::shell() as follows:

/**
 * Returns the output from running the specified command with the shell.
 */
Try<std::string> shell(const string& command)
{
  // Actually handle the WIFEXITED, WIFSIGNALED here!
}

where the returned string is stdout and, should the program be signaled, or exit with a non-zero exit code, we will simply return a Failure with an error message that will encapsulate both the returned/signaled state, and, possibly stderr.

And some test driven development:

EXPECT_ERROR(os::shell("false"));
EXPECT_SOME(os::shell("true"));

EXPECT_SOME_EQ("hello world", os::shell("echo hello world"));

Alternatively, the caller can ask to have stderr conflated with stdout:

Try<string> outAndErr = os::shell("myCmd --foo 2>&1");

However, stderr will be ignored by default:

// We don't read standard error by default.
EXPECT_SOME_EQ("", os::shell("echo hello world 1>&2"));

// We don't even read stderr if something fails (to return in Try::error).
Try<string> output = os::shell("echo hello world 1>&2 && false");
EXPECT_ERROR(output);
EXPECT_FALSE(strings::contains(output.error(), "hello world"));

An analysis of existing usage shows that in almost all cases, the caller only cares if not error; in fact, the actual exit code is read only once, and even then, in a test case.

We believe this will simplify the API to the caller, and will significantly reduce the length and complexity at the calling sites (<6 LOC against the current 20+).

Issue Links

• blocks: MESOS-3154 Enable Mesos Agent Node to use arbitrary script / module to figure out IP, HOSTNAME