ExitWithError() - enforce required exit status & stderr

Final followup to containers#22270. That PR added a temporary convention
allowing a new form of ExitWithError(), one with an exit code
and stderr substring. In order to allow bite-size progress,
the old no-args form was still allowed. This PR removes
support for no-args ExitWithError().

This PR also adds one piece of new functionality: passing ""
(empty string) as the stderr arg means "expect exit code
but fail if there's anything at all in stderr".

Signed-off-by: Ed Santiago <[email protected]>

test/e2e/play_kube_test.go

@@ -2396,7 +2396,7 @@ var _ = Describe("Podman kube play", func() {
 
 		hc := podmanTest.Podman([]string{"healthcheck", "run", ctrName})
 		hc.WaitWithDefaultTimeout()
-		Expect(hc).Should(ExitWithError(1))
+		Expect(hc).Should(ExitWithError(1, ""))
 
 		exec := podmanTest.Podman([]string{"exec", ctrName, "sh", "-c", "echo 'startup probe success' > /testfile"})
 		exec.WaitWithDefaultTimeout()
@@ -5737,7 +5737,7 @@ spec:
 
 		curlTest := podmanTest.Podman([]string{"run", "--network", "host", NGINX_IMAGE, "curl", "-s", "localhost:19000"})
 		curlTest.WaitWithDefaultTimeout()
-		Expect(curlTest).Should(ExitWithError(7))
+		Expect(curlTest).Should(ExitWithError(7, ""))
 	})
 
 	It("without Ports, publish in command line - curl should succeed", func() {

test/e2e/run_memory_test.go

@@ -82,7 +82,7 @@ var _ = Describe("Podman run memory", func() {
 		// create a container that gets oomkilled
 		session := podmanTest.Podman([]string{"run", "--name", ctrName, "--read-only", "--memory-swap=20m", "--memory=20m", "--oom-score-adj=1000", ALPINE, "sort", "/dev/urandom"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitWithError(137))
+		Expect(session).Should(ExitWithError(137, ""))
 
 		inspect := podmanTest.Podman(([]string{"inspect", "--format", "{{.State.OOMKilled}} {{.State.ExitCode}}", ctrName}))
 		inspect.WaitWithDefaultTimeout()

test/e2e/run_seccomp_test.go

@@ -37,7 +37,7 @@ var _ = Describe("Podman run", func() {
 			// TODO: worse than that. With runc, we get two alternating failures:
 			//   126 + cannot start a container that has stopped
 			//   127 + failed to connect to container's attach socket ... ENOENT
-			Expect(session).To(ExitWithError())
+			Expect(session.ExitCode()).To(BeNumerically(">=", 126), "Exit status using runc")
 		} else {
 			expect := "OCI runtime error: crun: read from the init process"
 			if IsRemote() {

test/e2e/start_test.go

@@ -39,7 +39,7 @@ var _ = Describe("Podman start", func() {
 		Expect(session).Should(ExitWithError(125, "not found in $PATH"))
 		session = podmanTest.Podman([]string{"container", "exists", "test"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError(1))
+		Expect(session).To(ExitWithError(1, ""))
 	})
 
 	It("podman start --rm --attach removed on failure", func() {
@@ -52,7 +52,7 @@ var _ = Describe("Podman start", func() {
 		Expect(session).Should(ExitWithError(125, "not found in $PATH"))
 		session = podmanTest.Podman([]string{"container", "exists", cid})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError(1))
+		Expect(session).To(ExitWithError(1, ""))
 	})
 
 	It("podman container start single container by id", func() {
@@ -97,7 +97,7 @@ var _ = Describe("Podman start", func() {
 		session = podmanTest.Podman([]string{"start", "--attach", cid})
 		session.WaitWithDefaultTimeout()
 		// It should forward the signal
-		Expect(session).Should(ExitWithError(1))
+		Expect(session).Should(ExitWithError(1, ""))
 	})
 
 	It("podman start multiple containers", func() {

test/utils/matchers.go

@@ -23,29 +23,10 @@ type ExitMatcher struct {
 	msg              string
 }
 
-// ExitWithError matches when assertion is > argument.  Default 0
+// ExitWithError checks both exit code and stderr, fails if either does not match
 // Modeled after the gomega Exit() matcher and also operates on sessions.
-func ExitWithError(expectations ...interface{}) *ExitMatcher {
-	exitCode := 0
-	expectStderr := ""
-	// FIXME: once all ExitWithError()s have been migrated to new form,
-	//        change interface to (int, ...string)
-	if len(expectations) > 0 {
-		var ok bool
-		exitCode, ok = expectations[0].(int)
-		if !ok {
-			panic("ExitWithError(): first arg, if present, must be an int")
-		}
-
-		if len(expectations) > 1 {
-			expectStderr, ok = expectations[1].(string)
-			if !ok {
-				panic("ExitWithError(): second arg, if present, must be a string")
-			}
-		}
-	}
-
-	return &ExitMatcher{ExpectedExitCode: exitCode, ExpectedStderr: expectStderr}
+func ExitWithError(expectExitCode int, expectStderr string) *ExitMatcher {
+	return &ExitMatcher{ExpectedExitCode: expectExitCode, ExpectedStderr: expectStderr}
 }
 
 // Match follows gexec.Matcher interface.
@@ -61,16 +42,6 @@ func (matcher *ExitMatcher) Match(actual interface{}) (success bool, err error)
 		return false, nil
 	}
 
-	// FIXME: temporary until all ExitWithError()s are migrated
-	//        to new mandatory-int form.
-	if matcher.ExpectedExitCode == 0 {
-		if matcher.ExitCode == 0 {
-			matcher.msg = "Expected process to exit nonzero. It did not."
-			return false, nil
-		}
-		return true, nil
-	}
-
 	// Check exit code first. If it's not what we want, there's no point
 	// in checking error substrings
 	if matcher.ExitCode != matcher.ExpectedExitCode {
@@ -83,6 +54,11 @@ func (matcher *ExitMatcher) Match(actual interface{}) (success bool, err error)
 			matcher.msg = fmt.Sprintf("Command exited %d as expected, but did not emit '%s'", matcher.ExitCode, matcher.ExpectedStderr)
 			return false, nil
 		}
+	} else {
+		if session.ErrorToString() != "" {
+			matcher.msg = "Command exited with expected exit status, but emitted unwanted stderr"
+			return false, nil
+		}
 	}
 
 	return true, nil