Add "podman kube play" cmd #14772

nicrowe00 · 2022-06-29T13:34:08Z

The "podman kube play" command is designed to be a replacement for the
"podman play kube" command.
It performs the same function as "play kube" while also still working with the same flags and options.
The "podman play kube" command is still functional as an alias of "kube play".

Closes #12475
Signed-off-by: Niall Crowe [email protected]

Does this PR introduce a user-facing change?

The command "podman kube play" replaces the "podman play kube" command.
"podman play kube" can still be used in the same manner as before as it is now an alias of
"podman kube play".

nicrowe00 · 2022-06-29T13:34:37Z

@vrothberg PTAL

vrothberg

@rhatdan @baude PTAL

pkg/bindings/kube/kube1.go

pkg/api/handlers/libpod/kube.go

docs/source/markdown/podman-kube-play.1.md

cmd/podman/kube/kube.go

vrothberg · 2022-07-01T08:50:01Z

@mheon, I would like this PR to get in after we branched for 4.2. Do you know when we're going to branch?

docs/source/Commands.rst

giuseppe · 2022-07-05T13:08:11Z

FWIW I find adding a podman kube subcommand confusing. I think adding a podman down kube subcommand will be cleaner and easier to extend in future

vrothberg · 2022-07-05T13:12:12Z

FWIW I find adding a podman kube subcommand confusing. I think adding a podman down kube subcommand will be cleaner and easier to extend in future

I think you are late to the game. @nicrowe00 is working on it for a number of weeks now and mentioned it in the meetings.

What I don't like about scattering kube-things over different commands is that it's impossible to explore functionality. I think commands should be structured by objects, not by actions. For instance, there is {volume,pod,network,image,...} rm but not rm {volume,pod,network,image,...}.

"play" does not mean anything to me personally but I can guess what may happen behind "kube".

baude · 2022-07-05T13:49:34Z

it seems like we have a enough debate, including me, here to put this on pause and hash this out. might I suggest a cabal or a design doc that we can all review?

vrothberg · 2022-07-05T13:54:02Z

How can this change go unnoticed? Niall has mentioned it in every meeting the past 3 weeks.

Let's chat in today's meeting, so we can unblock Niall. Note that there was consensus in #12475 for doing that already and is has been discussed in meetings back then.

baude · 2022-07-05T13:59:31Z

How can this change go unnoticed? Niall has mentioned it in every meeting the past 3 weeks.

Because you brought this up publicly @vrothberg , I am compelled to respond. I have my hands in more than a couple of things right now including planning for the team. I have also been on PTO. It is actually relatively easy to miss something like this as it is NOT my responsibility to know what everyone on the team is doing; there are other people for that.

mheon · 2022-07-05T14:00:18Z

At this point we've had enough people weigh in on both sides that consensus seems unlikely. I'm OK to proceed with this PR given that - we've discussed the point a lot already and could not come to an agreement. The commands are aliased, which seems like the best compromise we can make.

rhatdan · 2022-07-05T14:28:44Z

Lets talk at Cabal.
Since we think this should compete with compose, I believe we should use the same terminology. Up and Down.

vrothberg · 2022-07-05T14:35:09Z

Lets talk at Cabal. Since we think this should compete with compose, I believe we should use the same terminology. Up and Down.

Since you're on PTO. Which Cabal are you referring to?

I am pressing since I would ask Niall to work on something else if we need a longer period of time to iron things out.

rhatdan · 2022-07-05T14:37:21Z

Ill be working on Thursday.

vrothberg · 2022-07-05T14:38:36Z

Ill be working on Thursday.

Thanks! Let's chat during Cabal on Thursday then.

vrothberg · 2022-07-08T08:26:50Z

The remote failures are flakes. Rebasing the PR should fix it. git remote update; git pull --rebase upstream main with upstream being github.com/containers/podman.

vrothberg

Just a couple of comments, mostly related to breaking changes.

@rhatdan @baude @umohnani8 PTAL

cmd/podman/kube/kube.go

cmd/podman/kube/play.go

docs/kube_play_support.md

docs/source/markdown/links/podman-play.1

pkg/bindings/kube/kube.go

pkg/bindings/kube/types.go

pkg/bindings/play/play.go

pkg/bindings/kube/kube.go

vrothberg

Other than the final nit, LGTM

@rhatdan @baude PTAL

pkg/bindings/kube/types.go

docs/kubernetes_support.md

TomSweeneyRedHat · 2022-07-12T20:59:00Z

A lot of unhappy red tests still.

The "podman kube play" command is designed to be a replacement for the "podman play kube" command. It performs the same function as "play kube" while also still working with the same flags and options. The "podman play kube" command is still functional as an alias of "kube play". Closes #12475 Signed-off-by: Niall Crowe <[email protected]> Signed-off-by: Valentin Rothberg <[email protected]>

test/apiv2/80-kube.at

vrothberg

LGTM

@rhatdan @baude PTAL
Let's wait for a head nod from @edsantiago as well.

test/system/700-play.bats

rhatdan · 2022-07-13T19:05:18Z

/lgtm
Once we have a podman kube down, do we remove podman kube play --down?
/approve

openshift-ci · 2022-07-13T19:05:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nicrowe00, 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:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

test/apiv2/80-kube.at

openshift-ci bot added the release-note label Jun 29, 2022

vrothberg reviewed Jun 29, 2022

View reviewed changes

pkg/bindings/kube/kube1.go Outdated Show resolved Hide resolved

pkg/api/handlers/libpod/kube.go Show resolved Hide resolved

docs/source/markdown/podman-kube-play.1.md Show resolved Hide resolved

nicrowe00 changed the title ~~Add "podman kube play" cmd~~ WIP: Add "podman kube play" cmd Jun 29, 2022

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2022

rhatdan reviewed Jun 29, 2022

View reviewed changes

cmd/podman/kube/kube.go Outdated Show resolved Hide resolved

vrothberg reviewed Jul 1, 2022

View reviewed changes

docs/source/Commands.rst Show resolved Hide resolved

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 1, 2022

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2022

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 8, 2022

vrothberg reviewed Jul 8, 2022

View reviewed changes

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 8, 2022

nicrowe00 changed the title ~~WIP: Add "podman kube play" cmd~~ Add "podman kube play" cmd Jul 12, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 12, 2022

vrothberg reviewed Jul 12, 2022

View reviewed changes

pkg/bindings/kube/types.go Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Jul 12, 2022

View reviewed changes

docs/kubernetes_support.md Show resolved Hide resolved

vrothberg reviewed Jul 13, 2022

View reviewed changes

test/apiv2/80-kube.at Show resolved Hide resolved

vrothberg reviewed Jul 13, 2022

View reviewed changes

edsantiago reviewed Jul 13, 2022

View reviewed changes

test/system/700-play.bats Show resolved Hide resolved

openshift-ci bot assigned rhatdan Jul 13, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 13, 2022

openshift-ci bot merged commit 5c3eec5 into containers:main Jul 13, 2022

edsantiago reviewed Jul 13, 2022

View reviewed changes

test/apiv2/80-kube.at Show resolved Hide resolved

edsantiago mentioned this pull request Jul 13, 2022

deadlock(?) in podman rm(?) #14929

Closed

nicrowe00 mentioned this pull request Aug 23, 2022

podman kube play/down --read from URL #15426

Merged

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "podman kube play" cmd #14772

Add "podman kube play" cmd #14772

nicrowe00 commented Jun 29, 2022

nicrowe00 commented Jun 29, 2022

vrothberg left a comment

vrothberg commented Jul 1, 2022

giuseppe commented Jul 5, 2022

vrothberg commented Jul 5, 2022 •

edited

Loading

baude commented Jul 5, 2022

vrothberg commented Jul 5, 2022

baude commented Jul 5, 2022

mheon commented Jul 5, 2022

rhatdan commented Jul 5, 2022

vrothberg commented Jul 5, 2022

rhatdan commented Jul 5, 2022

vrothberg commented Jul 5, 2022

vrothberg commented Jul 8, 2022

vrothberg left a comment

vrothberg left a comment

TomSweeneyRedHat commented Jul 12, 2022

vrothberg left a comment

rhatdan commented Jul 13, 2022

openshift-ci bot commented Jul 13, 2022

Add "podman kube play" cmd #14772

Add "podman kube play" cmd #14772

Conversation

nicrowe00 commented Jun 29, 2022

Does this PR introduce a user-facing change?

nicrowe00 commented Jun 29, 2022

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg commented Jul 1, 2022

giuseppe commented Jul 5, 2022

vrothberg commented Jul 5, 2022 • edited Loading

baude commented Jul 5, 2022

vrothberg commented Jul 5, 2022

baude commented Jul 5, 2022

mheon commented Jul 5, 2022

rhatdan commented Jul 5, 2022

vrothberg commented Jul 5, 2022

rhatdan commented Jul 5, 2022

vrothberg commented Jul 5, 2022

vrothberg commented Jul 8, 2022

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg left a comment

Choose a reason for hiding this comment

TomSweeneyRedHat commented Jul 12, 2022

vrothberg left a comment

Choose a reason for hiding this comment

rhatdan commented Jul 13, 2022

openshift-ci bot commented Jul 13, 2022

vrothberg commented Jul 5, 2022 •

edited

Loading