Merge pull request #22690 from edsantiago/exitwitherror-final

ExitWithError() - final push to strict mode

test/e2e/checkpoint_test.go

@@ -370,8 +370,16 @@ var _ = Describe("Podman checkpoint", func() {
 		result = podmanTest.Podman([]string{"container", "restore", cid})
 		result.WaitWithDefaultTimeout()
 
-		// FIXME: CRIU failure message not seen by podman (same as above)
-		Expect(result).Should(ExitWithError(125))
+		// default message when using crun
+		expectStderr := "crun: CRIU restoring failed -52. Please check CRIU logfile"
+		if podmanTest.OCIRuntime == "runc" {
+			expectStderr = "runc: criu failed: type NOTIFY errno 0"
+		}
+		if !IsRemote() {
+			// This part is only seen with podman local, never remote
+			expectStderr = "OCI runtime error: " + expectStderr
+		}
+		Expect(result).Should(ExitWithError(125, expectStderr))
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 		Expect(podmanTest.GetContainerStatus()).To(ContainSubstring("Exited"))
 

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_device_test.go

@@ -30,7 +30,7 @@ var _ = Describe("Podman run device", func() {
 	It("podman run bad device test", func() {
 		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/baddevice", ALPINE, "true"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
+		Expect(session).To(ExitWithError(125, "stat /dev/baddevice: no such file or directory"))
 	})
 
 	It("podman run device test", 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/run_staticip_test.go

@@ -82,6 +82,7 @@ var _ = Describe("Podman run with --ip flag", func() {
 		result := podmanTest.Podman([]string{"run", "-d", "--name", "nginx", "--ip", ip, NGINX_IMAGE})
 		result.WaitWithDefaultTimeout()
 		Expect(result).Should(ExitCleanly())
+		cid := result.OutputToString()
 
 		// This test should not use a proxy
 		client := &http.Client{
@@ -112,7 +113,6 @@ var _ = Describe("Podman run with --ip flag", func() {
 		}
 		result = podmanTest.Podman([]string{"run", "--ip", ip, ALPINE, "ip", "addr"})
 		result.WaitWithDefaultTimeout()
-		Expect(result).To(ExitWithError())
-		Expect(result.ErrorToString()).To(ContainSubstring(" address %s ", ip))
+		Expect(result).To(ExitWithError(126, fmt.Sprintf("IPAM error: requested ip address %s is already allocated to container ID %s", ip, cid)))
 	})
 })

test/e2e/runlabel_test.go

@@ -64,17 +64,20 @@ var _ = Describe("podman container runlabel", func() {
 		result.WaitWithDefaultTimeout()
 		Expect(result).Should(ExitCleanly())
 	})
+
 	It("podman container runlabel bogus label should result in non-zero exit code", func() {
 		result := podmanTest.Podman([]string{"container", "runlabel", "RUN", ALPINE})
 		result.WaitWithDefaultTimeout()
-		Expect(result).To(ExitWithError())
+		Expect(result).To(ExitWithError(125, fmt.Sprintf("cannot find the value of label: RUN in image: %s", ALPINE)))
 		// should not panic when label missing the value or don't have the label
 		Expect(result.OutputToString()).To(Not(ContainSubstring("panic")))
 	})
+
 	It("podman container runlabel bogus label in remote image should result in non-zero exit", func() {
-		result := podmanTest.Podman([]string{"container", "runlabel", "RUN", "docker.io/library/ubuntu:latest"})
+		remoteImage := "quay.io/libpod/testimage:00000000"
+		result := podmanTest.Podman([]string{"container", "runlabel", "RUN", remoteImage})
 		result.WaitWithDefaultTimeout()
-		Expect(result).To(ExitWithError())
+		Expect(result).To(ExitWithError(125, fmt.Sprintf("cannot find the value of label: RUN in image: %s", remoteImage)))
 		// should not panic when label missing the value or don't have the label
 		Expect(result.OutputToString()).To(Not(ContainSubstring("panic")))
 	})
@@ -86,7 +89,7 @@ var _ = Describe("podman container runlabel", func() {
 		// runlabel should fail with nonexistent authfile
 		result := podmanTest.Podman([]string{"container", "runlabel", "--authfile", "/tmp/nonexistent", "RUN", image})
 		result.WaitWithDefaultTimeout()
-		Expect(result).To(ExitWithError())
+		Expect(result).To(ExitWithError(125, "credential file is not accessible: faccessat /tmp/nonexistent: no such file or directory"))
 
 		result = podmanTest.Podman([]string{"rmi", image})
 		result.WaitWithDefaultTimeout()

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