-
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
[NO TESTS NEEDED] Allow podman play kube to read yaml file from stdin #9420
Conversation
@@ -4,12 +4,10 @@ | |||
podman-play-kube - Create pods and containers based on Kubernetes YAML | |||
|
|||
## SYNOPSIS | |||
**podman play kube** [*options*] *file*__.yml__ | |||
**podman play kube** [*options*] **file**__.yml__|**-** |
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.
-**podman play kube** [*options*] **file**__.yml__|**-**
+**podman play kube** [*options*] *file.yml|-*
test/system/700-play.bats
Outdated
load helpers | ||
|
||
@test "podman play with stdin" { | ||
run_podman play kube - <<< test.yaml |
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.
This can't possibly work. I have to assume that you know this, that this is simply a placeholder. (Reason: <<<
means "the next argument is a string variable"; what you probably want is < $PODMAN_TMPDIR/test.yaml
but you need to create that first.)
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.
Fixed
test/system/700-play.bats
Outdated
} | ||
|
||
@test "podman play" { | ||
run_podman play kube test.yaml |
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.
Same, but different reason (test.yaml
does not exist)
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.
fixed
efe1880
to
fcf09c6
Compare
Fixes: containers#8996 Signed-off-by: Daniel J Walsh <[email protected]>
This PR includes tests, but the test checker is screwed up, since it sees a rename but does not know tests were added. |
Short: "Play a pod based on Kubernetes YAML.", | ||
Long: kubeDescription, | ||
RunE: kube, | ||
Args: cobra.ExactArgs(1), | ||
ValidArgsFunction: common.AutocompleteDefaultOneArg, | ||
Example: `podman play kube nginx.yml | ||
cat nginx.yml | podman play kube - |
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.
great example!
@@ -4,12 +4,10 @@ | |||
podman-play-kube - Create pods and containers based on Kubernetes YAML | |||
|
|||
## SYNOPSIS | |||
**podman play kube** [*options*] *file*__.yml__ | |||
**podman play kube** [*options*] *file.yml|-* |
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.
IDK, but I think we want to keep the underscores?
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.
OOOPS, I'll trust @edsantiago's earlier comment.
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, rhatdan 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 |
Fixes: #8996
Signed-off-by: Daniel J Walsh [email protected]