diff --git a/cli/dcos-spark/main.go b/cli/dcos-spark/main.go index 8e7c0b79..99814d35 100644 --- a/cli/dcos-spark/main.go +++ b/cli/dcos-spark/main.go @@ -171,7 +171,7 @@ func handleCommands(app *kingpin.Application) { run := app.Command("run", "Submit a job to the Spark Mesos Dispatcher").Action(cmd.runSubmit) run.Flag("submit-args", fmt.Sprintf("Arguments matching what would be sent to 'spark-submit': %s", - sparkSubmitHelp())).Required().PlaceHolder("ARGS").StringVar(&cmd.submitArgs) + sparkSubmitHelp())).Required().PlaceHolder("\"ARGS\"").StringVar(&cmd.submitArgs) // TODO this should be moved to submit args run.Flag("docker-image", "Docker image to run the job within"). Default(""). diff --git a/cli/dcos-spark/submit_builder.go b/cli/dcos-spark/submit_builder.go index a701e57d..b81f8471 100644 --- a/cli/dcos-spark/submit_builder.go +++ b/cli/dcos-spark/submit_builder.go @@ -6,14 +6,16 @@ import ( "encoding/json" "errors" "fmt" - "github.com/mesosphere/dcos-commons/cli/client" - "github.com/mesosphere/dcos-commons/cli/config" - "gopkg.in/alecthomas/kingpin.v3-unstable" "log" "net/url" "os" "regexp" "strings" + + "github.com/mattn/go-shellwords" + "github.com/mesosphere/dcos-commons/cli/client" + "github.com/mesosphere/dcos-commons/cli/config" + "gopkg.in/alecthomas/kingpin.v3-unstable" ) var keyWhitespaceValPattern = regexp.MustCompile("(.+)\\s+(.+)") @@ -146,8 +148,10 @@ Args: StringVar(&args.mainClass) // note: spark-submit can autodetect, but only for file://local.jar submit.Flag("properties-file", "Path to file containing whitespace-separated Spark property defaults."). PlaceHolder("PATH").ExistingFileVar(&args.propertiesFile) - submit.Flag("conf", "Custom Spark configuration properties."). - PlaceHolder("PROP=VALUE").StringMapVar(&args.properties) + submit.Flag("conf", "Custom Spark configuration properties. "+ + "If submitting properties with multiple values, "+ + "wrap in single quotes e.g. --conf prop='val1 val2'"). + PlaceHolder("prop=value").StringMapVar(&args.properties) submit.Flag("kerberos-principal", "Principal to be used to login to KDC."). PlaceHolder("user@REALM").Default("").StringVar(&args.kerberosPrincipal) submit.Flag("keytab-secret-path", "Path to Keytab in secret store to be used in the Spark drivers"). @@ -280,75 +284,84 @@ func parseApplicationFile(args *sparkArgs) error { return nil } -func cleanUpSubmitArgs(argsStr string, boolVals []*sparkVal) ([]string, []string) { - - // collapse two or more spaces to one. - argsCompacted := collapseSpacesPattern.ReplaceAllString(argsStr, " ") +// we use Kingpin to parse CLI commands and options +// spark-submit by convention uses '--arg val' while kingpin only supports --arg=val +// transformSubmitArgs turns the former into the latter +func transformSubmitArgs(argsStr string, boolVals []*sparkVal) ([]string, []string) { // clean up any instances of shell-style escaped newlines: "arg1\\narg2" => "arg1 arg2" - argsCleaned := strings.TrimSpace(backslashNewlinePattern.ReplaceAllLiteralString(argsCompacted, " ")) - // HACK: spark-submit uses '--arg val' by convention, while kingpin only supports '--arg=val'. - // translate the former into the latter for kingpin to parse. - args := strings.Split(argsCleaned, " ") - argsEquals := make([]string, 0) - appFlags := make([]string, 0) - i := 0 -ARGLOOP: - for i < len(args) { - arg := args[i] - if !strings.HasPrefix(arg, "-") { - // looks like we've exited the flags entirely, and are now at the jar and/or args. - // any arguments without a dash at the front should've been joined to preceding keys. - // flush the rest and exit. - for i < len(args) { - arg = args[i] - // if we have a --flag going to the application we need to take the arg (flag) and the value ONLY - // if it's not of the format --flag=val which scopt allows - if strings.HasPrefix(arg, "-") { - appFlags = append(appFlags, arg) - if strings.Contains(arg, "=") || (i+1) >= len(args) { - i += 1 - } else { - // if there's a value with this flag, add it - if !strings.HasPrefix(args[i+1], "-") { - appFlags = append(appFlags, args[i+1]) - i += 1 - } - i += 1 - } - } else { - argsEquals = append(argsEquals, arg) - i += 1 - } + argsStr = strings.TrimSpace(backslashNewlinePattern.ReplaceAllLiteralString(argsStr, " ")) + // collapse two or more spaces to one + argsStr = collapseSpacesPattern.ReplaceAllString(argsStr, " ") + // parse argsStr into []string args maintaining shell escaped sequences + args, err := shellwords.Parse(argsStr) + if err != nil { + log.Fatalf("Could not parse string args correctly. Error: %v", err) + } + sparkArgs, appArgs := make([]string, 0), make([]string, 0) +LOOP: + for i := 0; i < len(args); { + current := strings.TrimSpace(args[i]) + switch { + // The main assumption with --submit-args is that all spark-submit flags come before the spark jar URL + // if current is a spark jar/app, we've processed all flags + case isSparkApp(current): + sparkArgs = append(sparkArgs, args[i]) + appArgs = append(appArgs, args[i+1:]...) + break LOOP + case strings.HasPrefix(current, "--"): + if isBoolFlag(boolVals, current) { + sparkArgs = append(sparkArgs, current) + i++ + continue LOOP } - break - } - // join this arg to the next arg if...: - // 1. we're not at the last arg in the array - // 2. we start with "--" - // 3. we don't already contain "=" (already joined) - // 4. we aren't a boolean value (no val to join) - if i < len(args)-1 && strings.HasPrefix(arg, "--") && !strings.Contains(arg, "=") { - // check for boolean: - for _, boolVal := range boolVals { - if boolVal.flagName == arg[2:] { - argsEquals = append(argsEquals, arg) - i += 1 - continue ARGLOOP - } + if strings.Contains(current, "=") { + // already in the form arg=val, no merge required + sparkArgs = append(sparkArgs, current) + i++ + continue LOOP } - // merge this --key against the following val to get --key=val - argsEquals = append(argsEquals, arg+"="+args[i+1]) + // otherwise, merge current with next into form arg=val; eg --driver-memory=512m + next := args[i+1] + sparkArgs = append(sparkArgs, current+"="+next) i += 2 - } else { - // already joined or at the end, pass through: - argsEquals = append(argsEquals, arg) - i += 1 + default: + // if not a flag or jar, current is a continuation of the last arg and should not have been split + // eg extraJavaOptions="-Dparam1 -Dparam2" was parsed as [extraJavaOptions, -Dparam1, -Dparam2] + combined := sparkArgs[len(sparkArgs)-1] + " " + current + sparkArgs = append(sparkArgs[:len(sparkArgs)-1], combined) + i++ } } - client.PrintVerbose("Translated spark-submit arguments: '%s'", argsEquals) - client.PrintVerbose("Translated application arguments: '%s'", appFlags) + if config.Verbose { + client.PrintVerbose("Translated spark-submit arguments: '%s'", strings.Join(sparkArgs, ", ")) + client.PrintVerbose("Translated application arguments: '%s'", strings.Join(appArgs, ", ")) + } + return sparkArgs, appArgs +} - return argsEquals, appFlags +var acceptedSparkAppExtensions = []string{ + ".jar", + ".py", + ".R", +} + +func isSparkApp(str string) bool { + for _, ext := range acceptedSparkAppExtensions { + if strings.HasSuffix(str, ext) { + return true + } + } + return false +} + +// check if string is a boolean flag (eg --supervise) +func isBoolFlag(boolVals []*sparkVal, str string) bool { + for _, boolVal := range boolVals { + if boolVal.flagName == str[2:] { + return true + } + } + return false } func getValsFromPropertiesFile(path string) map[string]string { @@ -416,7 +429,7 @@ func buildSubmitJson(cmd *SparkCommand, marathonConfig map[string]interface{}) ( // then map flags submit, args := sparkSubmitArgSetup() // setup // convert and get application flags, add them to the args passed to the spark app - submitArgs, appFlags := cleanUpSubmitArgs(cmd.submitArgs, args.boolVals) + submitArgs, appFlags := transformSubmitArgs(cmd.submitArgs, args.boolVals) args.appArgs = append(args.appArgs, appFlags...) _, err := submit.Parse(submitArgs) diff --git a/cli/dcos-spark/submit_builder_test.go b/cli/dcos-spark/submit_builder_test.go index 44c24136..dd8fcfcd 100644 --- a/cli/dcos-spark/submit_builder_test.go +++ b/cli/dcos-spark/submit_builder_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "os" "testing" @@ -37,10 +38,10 @@ func TestCliTestSuite(t *testing.T) { } // test spaces -func (suite *CliTestSuite) TestCleanUpSubmitArgs() { +func (suite *CliTestSuite) TestTransformSubmitArgs() { _, args := sparkSubmitArgSetup() - inputArgs := "--conf spark.app.name=kerberosStreaming --conf spark.cores.max=8" - submitArgs, _ := cleanUpSubmitArgs(inputArgs, args.boolVals) + inputArgs := "--conf spark.app.name=kerberosStreaming --conf spark.cores.max=8 main.jar 100" + submitArgs, _ := transformSubmitArgs(inputArgs, args.boolVals) if "--conf=spark.app.name=kerberosStreaming" != submitArgs[0] { suite.T().Errorf("Failed to reduce spaces while cleaning submit args.") } @@ -50,12 +51,54 @@ func (suite *CliTestSuite) TestCleanUpSubmitArgs() { } } +func (suite *CliTestSuite) TestTransformSubmitArgsWithMulitpleValues() { + _, args := sparkSubmitArgSetup() + inputArgs := "--conf spark.driver.extraJavaOptions='-XX:+PrintGC -Dparam1=val1 -Dparam2=val2' main.py 100" + expected := "--conf=spark.driver.extraJavaOptions=-XX:+PrintGC -Dparam1=val1 -Dparam2=val2" + actual, _ := transformSubmitArgs(inputArgs, args.boolVals) + assert.Equal(suite.T(), expected, actual[0]) +} + +func (suite *CliTestSuite) TestTransformSubmitArgsWithSpecialCharacters() { + _, args := sparkSubmitArgSetup() + inputArgs := "--conf spark.driver.extraJavaOptions='-Dparam1=\"val 1?\" -Dparam2=val\\ 2! -Dmulti.dot.param3=\"val\\ 3\" -Dpath=$PATH' main.py 100" + expected := "--conf=spark.driver.extraJavaOptions=-Dparam1=\"val 1?\" -Dparam2=val\\ 2! -Dmulti.dot.param3=\"val\\ 3\" -Dpath=$PATH" + actual, _ := transformSubmitArgs(inputArgs, args.boolVals) + assert.Equal(suite.T(), expected, actual[0]) +} + +func (suite *CliTestSuite) TestTransformSubmitArgsConfsAlreadyHasEquals() { + _, args := sparkSubmitArgSetup() + inputArgs := "--conf=spark.driver.extraJavaOptions='-Dparam1=val1' main.py 100" + expected := "--conf=spark.driver.extraJavaOptions=-Dparam1=val1" + actual, _ := transformSubmitArgs(inputArgs, args.boolVals) + assert.Equal(suite.T(), expected, actual[0]) +} + +func (suite *CliTestSuite) TestTransformSubmitArgsMultilines() { + _, args := sparkSubmitArgSetup() + inputArgs := `--conf spark.driver.extraJavaOptions='-XX:+PrintGC -XX:+PrintGCTimeStamps' \ + --supervise --driver-memory 1g \ + main.py 100` + expected := []string{"--conf=spark.driver.extraJavaOptions=-XX:+PrintGC -XX:+PrintGCTimeStamps", "--supervise", "--driver-memory=1g", "main.py"} + actual, _ := transformSubmitArgs(inputArgs, args.boolVals) + assert.Equal(suite.T(), expected, actual) +} + +func (suite *CliTestSuite) TestIsSparkApp() { + assert.True(suite.T(), isSparkApp("mainApp.jar")) + assert.True(suite.T(), isSparkApp("pythonFile.py")) + assert.True(suite.T(), isSparkApp("file.R")) + assert.False(suite.T(), isSparkApp("app.c")) + assert.False(suite.T(), isSparkApp("randomFlag")) +} + // test scopts pattern for app args when have full submit args func (suite *CliTestSuite) TestScoptAppArgs() { _, args := sparkSubmitArgSetup() inputArgs := `--driver-cores 1 --conf spark.cores.max=1 --driver-memory 512M --class org.apache.spark.examples.SparkPi http://spark-example.jar --input1 value1 --input2 value2` - submitArgs, appFlags := cleanUpSubmitArgs(inputArgs, args.boolVals) + submitArgs, appFlags := transformSubmitArgs(inputArgs, args.boolVals) if "--input1" != appFlags[0] { suite.T().Errorf("Failed to parse app args.") @@ -91,6 +134,7 @@ func (suite *CliTestSuite) TestPayloadSimple() { "--driver-cores %s "+ "--conf spark.cores.max=%s "+ "--driver-memory %s "+ + "--conf spark.driver.extraJavaOptions='-XX:+PrintGC -Dparam1=val1 -Dparam2=val2' "+ "--class %s "+ "%s --input1 value1 --input2 value2", driverCores, maxCores, driverMemory, mainClass, appJar) @@ -124,6 +168,7 @@ func (suite *CliTestSuite) TestPayloadSimple() { "spark.submit.deployMode": "cluster", "spark.mesos.driver.labels": fmt.Sprintf("DCOS_SPACE:%s", marathonAppId), "spark.driver.memory": driverMemory, + "spark.driver.extraJavaOptions": "-XX:+PrintGC -Dparam1=val1 -Dparam2=val2", "spark.jars": appJar, } diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/.travis.yml b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/.travis.yml new file mode 100644 index 00000000..16d1430a --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/.travis.yml @@ -0,0 +1,8 @@ +language: go +go: + - tip +before_install: + - go get github.com/mattn/goveralls + - go get golang.org/x/tools/cmd/cover +script: + - $HOME/gopath/bin/goveralls -repotoken 2FMhp57u8LcstKL9B190fLTcEnBtAAiEL diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/LICENSE b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/LICENSE new file mode 100644 index 00000000..740fa931 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2017 Yasuhiro Matsumoto + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/README.md b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/README.md new file mode 100644 index 00000000..b1d235c7 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/README.md @@ -0,0 +1,47 @@ +# go-shellwords + +[![Coverage Status](https://coveralls.io/repos/mattn/go-shellwords/badge.png?branch=master)](https://coveralls.io/r/mattn/go-shellwords?branch=master) +[![Build Status](https://travis-ci.org/mattn/go-shellwords.svg?branch=master)](https://travis-ci.org/mattn/go-shellwords) + +Parse line as shell words. + +## Usage + +```go +args, err := shellwords.Parse("./foo --bar=baz") +// args should be ["./foo", "--bar=baz"] +``` + +```go +os.Setenv("FOO", "bar") +p := shellwords.NewParser() +p.ParseEnv = true +args, err := p.Parse("./foo $FOO") +// args should be ["./foo", "bar"] +``` + +```go +p := shellwords.NewParser() +p.ParseBacktick = true +args, err := p.Parse("./foo `echo $SHELL`") +// args should be ["./foo", "/bin/bash"] +``` + +```go +shellwords.ParseBacktick = true +p := shellwords.NewParser() +args, err := p.Parse("./foo `echo $SHELL`") +// args should be ["./foo", "/bin/bash"] +``` + +# Thanks + +This is based on cpan module [Parse::CommandLine](https://metacpan.org/pod/Parse::CommandLine). + +# License + +under the MIT License: http://mattn.mit-license.org/2017 + +# Author + +Yasuhiro Matsumoto (a.k.a mattn) diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/_example/pipe.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/_example/pipe.go new file mode 100644 index 00000000..b08ae008 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/_example/pipe.go @@ -0,0 +1,44 @@ +// +build ignore + +package main + +import ( + "fmt" + "log" + + "github.com/mattn/go-shellwords" +) + +func isSpace(r byte) bool { + switch r { + case ' ', '\t', '\r', '\n': + return true + } + return false +} + +func main() { + line := ` + /usr/bin/ls -la | sort 2>&1 | tee files.log + ` + parser := shellwords.NewParser() + + for { + args, err := parser.Parse(line) + if err != nil { + log.Fatal(err) + } + fmt.Println(args) + if parser.Position < 0 { + break + } + i := parser.Position + for ; i < len(line); i++ { + if isSpace(line[i]) { + break + } + } + fmt.Println(string([]rune(line)[parser.Position:i])) + line = string([]rune(line)[i+1:]) + } +} diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords.go new file mode 100644 index 00000000..bcd1e1f0 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords.go @@ -0,0 +1,178 @@ +package shellwords + +import ( + "errors" + "os" + "regexp" +) + +var ( + ParseEnv bool = false + ParseBacktick bool = false +) + +var envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) + +func isSpace(r rune) bool { + switch r { + case ' ', '\t', '\r', '\n': + return true + } + return false +} + +func replaceEnv(s string) string { + return envRe.ReplaceAllStringFunc(s, func(s string) string { + s = s[1:] + if s[0] == '{' { + s = s[1 : len(s)-1] + } + return os.Getenv(s) + }) +} + +type Parser struct { + ParseEnv bool + ParseBacktick bool + Position int +} + +func NewParser() *Parser { + return &Parser{ParseEnv, ParseBacktick, 0} +} + +func (p *Parser) Parse(line string) ([]string, error) { + args := []string{} + buf := "" + var escaped, doubleQuoted, singleQuoted, backQuote, dollarQuote bool + backtick := "" + + pos := -1 + got := false + +loop: + for i, r := range line { + if escaped { + buf += string(r) + escaped = false + continue + } + + if r == '\\' { + if singleQuoted { + buf += string(r) + } else { + escaped = true + } + continue + } + + if isSpace(r) { + if singleQuoted || doubleQuoted || backQuote || dollarQuote { + buf += string(r) + backtick += string(r) + } else if got { + if p.ParseEnv { + buf = replaceEnv(buf) + } + args = append(args, buf) + buf = "" + got = false + } + continue + } + + switch r { + case '`': + if !singleQuoted && !doubleQuoted && !dollarQuote { + if p.ParseBacktick { + if backQuote { + out, err := shellRun(backtick) + if err != nil { + return nil, err + } + buf = out + } + backtick = "" + backQuote = !backQuote + continue + } + backtick = "" + backQuote = !backQuote + } + case ')': + if !singleQuoted && !doubleQuoted && !backQuote { + if p.ParseBacktick { + if dollarQuote { + out, err := shellRun(backtick) + if err != nil { + return nil, err + } + buf = out + } + backtick = "" + dollarQuote = !dollarQuote + continue + } + backtick = "" + dollarQuote = !dollarQuote + } + case '(': + if !singleQuoted && !doubleQuoted && !backQuote { + if !dollarQuote && len(buf) > 0 && buf == "$" { + dollarQuote = true + buf += "(" + continue + } else { + return nil, errors.New("invalid command line string") + } + } + case '"': + if !singleQuoted && !dollarQuote { + doubleQuoted = !doubleQuoted + continue + } + case '\'': + if !doubleQuoted && !dollarQuote { + singleQuoted = !singleQuoted + continue + } + case ';', '&', '|', '<', '>': + if !(escaped || singleQuoted || doubleQuoted || backQuote) { + if r == '>' && len(buf) > 0 { + if c := buf[0]; '0' <= c && c <= '9' { + i -= 1 + got = false + } + } + pos = i + break loop + } + } + + got = true + buf += string(r) + if backQuote || dollarQuote { + backtick += string(r) + } + } + + if got { + if p.ParseEnv { + buf = replaceEnv(buf) + } + args = append(args, buf) + } + + if escaped || singleQuoted || doubleQuoted || backQuote || dollarQuote { + return nil, errors.New("invalid command line string") + } + + p.Position = pos + + return args, nil +} + +func Parse(line string) ([]string, error) { + return NewParser().Parse(line) +} diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords_test.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords_test.go new file mode 100644 index 00000000..1a1b0f66 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/shellwords_test.go @@ -0,0 +1,248 @@ +package shellwords + +import ( + "os" + "reflect" + "testing" +) + +var testcases = []struct { + line string + expected []string +}{ + {`var --bar=baz`, []string{`var`, `--bar=baz`}}, + {`var --bar="baz"`, []string{`var`, `--bar=baz`}}, + {`var "--bar=baz"`, []string{`var`, `--bar=baz`}}, + {`var "--bar='baz'"`, []string{`var`, `--bar='baz'`}}, + {"var --bar=`baz`", []string{`var`, "--bar=`baz`"}}, + {`var "--bar=\"baz'"`, []string{`var`, `--bar="baz'`}}, + {`var "--bar=\'baz\'"`, []string{`var`, `--bar='baz'`}}, + {`var --bar='\'`, []string{`var`, `--bar=\`}}, + {`var "--bar baz"`, []string{`var`, `--bar baz`}}, + {`var --"bar baz"`, []string{`var`, `--bar baz`}}, + {`var --"bar baz"`, []string{`var`, `--bar baz`}}, +} + +func TestSimple(t *testing.T) { + for _, testcase := range testcases { + args, err := Parse(testcase.line) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(args, testcase.expected) { + t.Fatalf("Expected %#v, but %#v:", testcase.expected, args) + } + } +} + +func TestError(t *testing.T) { + _, err := Parse("foo '") + if err == nil { + t.Fatal("Should be an error") + } + _, err = Parse(`foo "`) + if err == nil { + t.Fatal("Should be an error") + } + + _, err = Parse("foo `") + if err == nil { + t.Fatal("Should be an error") + } +} + +func TestLastSpace(t *testing.T) { + args, err := Parse("foo bar\\ ") + if err != nil { + t.Fatal(err) + } + if len(args) != 2 { + t.Fatal("Should have two elements") + } + if args[0] != "foo" { + t.Fatal("1st element should be `foo`") + } + if args[1] != "bar " { + t.Fatal("1st element should be `bar `") + } +} + +func TestBacktick(t *testing.T) { + goversion, err := shellRun("go version") + if err != nil { + t.Fatal(err) + } + + parser := NewParser() + parser.ParseBacktick = true + args, err := parser.Parse("echo `go version`") + if err != nil { + t.Fatal(err) + } + expected := []string{"echo", goversion} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + args, err = parser.Parse(`echo $(echo foo)`) + if err != nil { + t.Fatal(err) + } + expected = []string{"echo", "foo"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + parser.ParseBacktick = false + args, err = parser.Parse(`echo $(echo "foo")`) + expected = []string{"echo", `$(echo "foo")`} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + args, err = parser.Parse("echo $(`echo1)") + if err != nil { + t.Fatal(err) + } + expected = []string{"echo", "$(`echo1)"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } +} + +func TestBacktickError(t *testing.T) { + parser := NewParser() + parser.ParseBacktick = true + _, err := parser.Parse("echo `go Version`") + if err == nil { + t.Fatal("Should be an error") + } + expected := "exit status 2:go: unknown subcommand \"Version\"\nRun 'go help' for usage.\n" + if expected != err.Error() { + t.Fatalf("Expected %q, but %q", expected, err.Error()) + } + _, err = parser.Parse(`echo $(echo1)`) + if err == nil { + t.Fatal("Should be an error") + } + _, err = parser.Parse(`echo $(echo1`) + if err == nil { + t.Fatal("Should be an error") + } + _, err = parser.Parse(`echo $ (echo1`) + if err == nil { + t.Fatal("Should be an error") + } + _, err = parser.Parse(`echo (echo1`) + if err == nil { + t.Fatal("Should be an error") + } + _, err = parser.Parse(`echo )echo1`) + if err == nil { + t.Fatal("Should be an error") + } +} + +func TestEnv(t *testing.T) { + os.Setenv("FOO", "bar") + + parser := NewParser() + parser.ParseEnv = true + args, err := parser.Parse("echo $FOO") + if err != nil { + t.Fatal(err) + } + expected := []string{"echo", "bar"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } +} + +func TestNoEnv(t *testing.T) { + parser := NewParser() + parser.ParseEnv = true + args, err := parser.Parse("echo $BAR") + if err != nil { + t.Fatal(err) + } + expected := []string{"echo", ""} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } +} + +func TestDupEnv(t *testing.T) { + os.Setenv("FOO", "bar") + os.Setenv("FOO_BAR", "baz") + + parser := NewParser() + parser.ParseEnv = true + args, err := parser.Parse("echo $$FOO$") + if err != nil { + t.Fatal(err) + } + expected := []string{"echo", "$bar$"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + args, err = parser.Parse("echo $${FOO_BAR}$") + if err != nil { + t.Fatal(err) + } + expected = []string{"echo", "$baz$"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } +} + +func TestHaveMore(t *testing.T) { + parser := NewParser() + parser.ParseEnv = true + + line := "echo foo; seq 1 10" + args, err := parser.Parse(line) + if err != nil { + t.Fatalf(err.Error()) + } + expected := []string{"echo", "foo"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + if parser.Position == 0 { + t.Fatalf("Commands should be remaining") + } + + line = string([]rune(line)[parser.Position+1:]) + args, err = parser.Parse(line) + if err != nil { + t.Fatalf(err.Error()) + } + expected = []string{"seq", "1", "10"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + if parser.Position > 0 { + t.Fatalf("Commands should not be remaining") + } +} + +func TestHaveRedirect(t *testing.T) { + parser := NewParser() + parser.ParseEnv = true + + line := "ls -la 2>foo" + args, err := parser.Parse(line) + if err != nil { + t.Fatalf(err.Error()) + } + expected := []string{"ls", "-la"} + if !reflect.DeepEqual(args, expected) { + t.Fatalf("Expected %#v, but %#v:", expected, args) + } + + if parser.Position == 0 { + t.Fatalf("Commands should be remaining") + } +} diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_go15.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_go15.go new file mode 100644 index 00000000..180f00f0 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_go15.go @@ -0,0 +1,24 @@ +// +build !go1.6 + +package shellwords + +import ( + "os" + "os/exec" + "runtime" + "strings" +) + +func shellRun(line string) (string, error) { + var b []byte + var err error + if runtime.GOOS == "windows" { + b, err = exec.Command(os.Getenv("COMSPEC"), "/c", line).Output() + } else { + b, err = exec.Command(os.Getenv("SHELL"), "-c", line).Output() + } + if err != nil { + return "", err + } + return strings.TrimSpace(string(b)), nil +} diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_posix.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_posix.go new file mode 100644 index 00000000..eaf1011d --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_posix.go @@ -0,0 +1,22 @@ +// +build !windows,go1.6 + +package shellwords + +import ( + "errors" + "os" + "os/exec" + "strings" +) + +func shellRun(line string) (string, error) { + shell := os.Getenv("SHELL") + b, err := exec.Command(shell, "-c", line).Output() + if err != nil { + if eerr, ok := err.(*exec.ExitError); ok { + b = eerr.Stderr + } + return "", errors.New(err.Error() + ":" + string(b)) + } + return strings.TrimSpace(string(b)), nil +} diff --git a/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_windows.go b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_windows.go new file mode 100644 index 00000000..e46f89a1 --- /dev/null +++ b/cli/dcos-spark/vendor/github.com/mattn/go-shellwords/util_windows.go @@ -0,0 +1,22 @@ +// +build windows,go1.6 + +package shellwords + +import ( + "errors" + "os" + "os/exec" + "strings" +) + +func shellRun(line string) (string, error) { + shell := os.Getenv("COMSPEC") + b, err := exec.Command(shell, "/c", line).Output() + if err != nil { + if eerr, ok := err.(*exec.ExitError); ok { + b = eerr.Stderr + } + return "", errors.New(err.Error() + ":" + string(b)) + } + return strings.TrimSpace(string(b)), nil +} diff --git a/manifest.json b/manifest.json index 03ffb2d1..6e919da9 100644 --- a/manifest.json +++ b/manifest.json @@ -2,16 +2,16 @@ "spark_version": "2.2.1", "default_spark_dist": { "hadoop_version": "2.6", - "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-2-bin-2.6.tgz" + "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-3-SNAPSHOT-bin-2.6.tgz" }, "spark_dist": [ { "hadoop_version": "2.6", - "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-2-bin-2.6.tgz" + "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-3-SNAPSHOT-bin-2.6.tgz" }, { "hadoop_version": "2.7", - "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-2-bin-2.7.tgz" + "uri": "https://downloads.mesosphere.com/spark/assets/spark-2.2.1-3-SNAPSHOT-bin-2.7.tgz" } ] } diff --git a/tests/jobs/scala/src/main/scala/MultiConfs.scala b/tests/jobs/scala/src/main/scala/MultiConfs.scala new file mode 100644 index 00000000..ddebd1e1 --- /dev/null +++ b/tests/jobs/scala/src/main/scala/MultiConfs.scala @@ -0,0 +1,27 @@ +import org.apache.spark.SparkConf + +/** + * Application that outputs the Spark configurations. + * Specifically we want to test whether long strings of multiple arg=values are passed through correctly. + * E.g. + * dcos spark run --submit-args="\ + --conf=spark.driver.extraJavaOptions='arg1=val1 arg2=val2 -Dparam3=\"valA valB\"' \ + --class MultiConfs \ + arg1 val1" + */ +object MultiConfs { + def main(args: Array[String]): Unit = { + val AppName = "MultiConfs App" + println(s"Running $AppName\n") + + // Verify property is set in Spark conf + val conf = new SparkConf().setAppName(AppName) + println("Printing all conf values...") + conf.getAll.foreach(println) + + // Verify property is set in system + val props = System.getProperties() + println("Printing all System.properties...") + props.list(System.out) + } +} diff --git a/tests/test_spark.py b/tests/test_spark.py index 89fecee1..dd1e5622 100644 --- a/tests/test_spark.py +++ b/tests/test_spark.py @@ -62,8 +62,9 @@ def test_task_not_lost(): @pytest.mark.xfail(sdk_utils.is_strict_mode(), reason="Currently fails in strict mode") -@pytest.mark.sanity -@pytest.mark.smoke +@pytest.mark.skip(reason="Currently fails due to CI misconfiguration") #TODO: Fix CI/update mesos-integration-tests +# @pytest.mark.sanity +# @pytest.mark.smoke def test_jar(service_name=utils.SPARK_SERVICE_NAME): master_url = ("https" if sdk_utils.is_strict_mode() else "http") + "://leader.mesos:5050" spark_job_runner_args = '{} dcos \\"*\\" spark:only 2 --auth-token={}'.format( @@ -105,6 +106,17 @@ def test_sparkPi(service_name=utils.SPARK_SERVICE_NAME): args=["--class org.apache.spark.examples.SparkPi"]) +@pytest.mark.sanity +def test_multi_arg_confs(service_name=utils.SPARK_SERVICE_NAME): + utils.run_tests( + app_url=utils.dcos_test_jar_url(), + app_args="", + expected_output="spark.driver.extraJavaOptions,-XX:+PrintGCDetails -XX:+PrintGCTimeStamps -Dparam3=\"valA valB\"", + service_name=service_name, + args=["--conf spark.driver.extraJavaOptions='-XX:+PrintGCDetails -XX:+PrintGCTimeStamps -Dparam3=\\\"valA valB\\\"'", + "--class MultiConfs"]) + + @pytest.mark.sanity @pytest.mark.smoke def test_python():