-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
podman: run --replace prints only the new container id #20189
podman: run --replace prints only the new container id #20189
Conversation
7de16bc
to
720afad
Compare
720afad
to
de0882a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/hold
test/e2e/run_test.go
Outdated
session.WaitWithDefaultTimeout() | ||
Expect(session).Should(ExitCleanly()) | ||
|
||
// make sure Podman prints only one ID | ||
Expect(session.OutputToStringArray()).To(HaveLen(1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: In case you have to repush, I'd like to check that it's really the CID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll second this. Very easy fix:
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 036cd4569..423c2e44c 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -1622,3 +1622,3 @@ VOLUME %s`, ALPINE, volPath, volPath)
// make sure Podman prints only one ID
- Expect(session.OutputToStringArray()).To(HaveLen(1))
+ Expect(session.OutputToString()).To(HaveLen(64))
}
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
de0882a
to
0fc5e74
Compare
test/e2e/run_test.go
Outdated
session.WaitWithDefaultTimeout() | ||
Expect(session).Should(ExitCleanly()) | ||
|
||
// make sure Podman prints only one ID | ||
Expect(session.OutputToStringArray()).To(HaveLen(64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I think this is going to fail - it should be OutputToString()
, not ...Array()
UPDATE: yep, tested. It fails.
LGTM |
print only the new container ID when using --replace instead of the terminated container ID if it was stopped. Closes: containers#20185 Signed-off-by: Giuseppe Scrivano <[email protected]>
0fc5e74
to
f21c1d2
Compare
/lgtm |
/hold cancel |
Does this PR introduce a user-facing change?