Create Pod Functionality #961

strzinek · 2022-04-14T18:04:53Z

Ability to create new pod group

Podman now allows the creation of pods with an optional port or volume mapping.

jelly

Thanks for the PR! This was something we wanted to add for a while!

@garrett as we have limited set of integrations, do we still want to have a tabbed layout?

I've noticed that pods also supports volumes/mounts so should we add that as well?

jelly · 2022-04-20T14:42:30Z

src/Containers.jsx

+                                  onClick={() => this.setState({ showCreatePodModal: true })}>
+                                  {_("Create pod")}
+                              </DropdownItem>
+                          ]} />


I've digged up our old designs for adding a pod creation to cockpit-podman and I think we don't want it in a new kebab menu but as a button

https://raw.githubusercontent.com/cockpit-project/cockpit-design/master/containers/podman-pod-groups.png

I am planning to implement other functions later which could go to kebab menu, and which are available through the API, e.g. Prune unused pods, Play a kube YAML, ... And I think two buttons together with filters would not look good on narrow layouts too. What do you think?

I don't think a create pod button takes up a lot of space, @garrett what do you think?

src/PodCreateModal.jsx

jelly · 2022-04-20T15:17:35Z

src/Containers.jsx

+                { title: proc, props: { modifier: "nowrap" } },
+                { title: mem, props: { modifier: "nowrap" } },
+                { title: <Badge isRead className={containerStateClass}>{_(containerState)}</Badge> }, // States are defined in util.js
+            ];


I like the idea of hiding the owner column when pods are used, but the current layout seems a bit weird and it's not clear that the pod is a "system" pod.

Could some competent designer have a look at it? It would be better to show also other pod details, such as port mapping and mounts, and present layout is not prepared for this. I'd be glad to implement it. ;-)

Port mappings an mounts are at the pod level, right? I thought I had a design for this somewhere. (But perhaps I'm thinking of something else...?)

I'll look around, but it's end of the day today, so please ping me if I forget and take a while to get back to this.

I wasn't able to find what I was thinking of. Perhaps I have a pattern in mind that was in another mockup.

This means I'll need to mock up what I have in mind for Cockpit-Podman. 😉

I changed the pod title a little bit and added Owner label.

@garrett the pod details design is here #489 but that does not deal with the owner field. Maybe we can improve this in a follow up PR.

garrett · 2022-04-25T16:16:48Z

@garrett as we have limited set of integrations, do we still want to have a tabbed layout?

No, please drop the tabs from this modal dialog. Thanks!

strzinek · 2022-05-02T17:38:51Z

Tabs removed.

I also added volumes section, but found out that mounts wont get created for pod because of bug in Podman: containers/podman#13548 . I am not able to verify it is fixed in 4.x version as I am on fedora 35 for now.

I would also add a new test for creating pod later on.

strzinek · 2022-05-12T20:27:58Z

Now it is possible to choose if infra container is created. Also, infra containers are not hidden in pods listings anymore.

I also prepared two new tests, TestApplication.testCreatePodUser and TestApplication.testCreatePodSystem.

jelly · 2022-05-18T15:56:36Z

src/Containers.jsx

@@ -463,9 +484,6 @@ class Containers extends React.Component {
                );
            }

-            // Remove infra containers
-            filtered = filtered.filter(id => !this.props.containers[id].IsInfra);


Why do you want to show infra containers, they aren't super interesting for users? Do you have a good reason to show it?

Infra container is a placeholder for shared resources of pod containers. The pod port mapping for example is defined on it. Without showing infra container, user of cockpit-podman does not know if pod has such shared resources and what ports pod exposes.

For that we prefer the following design, it takes the info from the infra container. #489

jelly · 2022-05-18T15:57:33Z

src/Containers.jsx

+                                  onClick={() => this.setState({ showCreatePodModal: true })}>
+                                  {_("Create pod")}
+                              </DropdownItem>
+                          ]} />


I don't think a create pod button takes up a lot of space, @garrett what do you think?

src/ImageRunModal.jsx

src/PodCreateModal.jsx

jelly · 2022-05-18T16:39:15Z

src/Containers.jsx

+                { title: proc, props: { modifier: "nowrap" } },
+                { title: mem, props: { modifier: "nowrap" } },
+                { title: <Badge isRead className={containerStateClass}>{_(containerState)}</Badge> }, // States are defined in util.js
+            ];


@garrett the pod details design is here #489 but that does not deal with the owner field. Maybe we can improve this in a follow up PR.

jelly · 2022-05-18T16:50:29Z

I think this is a good PR to look at landing next after the rename PR.

jelly · 2022-08-08T11:29:26Z

The rename PR landed, can you please rebase this PR?

jelly · 2022-08-19T08:36:28Z

I've rebased the PR, resolved the conflicts and dropped some of the things which can be a follow up PR.

marusak · 2022-08-22T06:41:39Z

This breaks pixel tests: https://cockpit-logs.us-east-1.linodeobjects.com/pull-961-20220819-084819-8540cbeb-fedora-35/TestApplication-testRunImageSystem-integration-pixels.png

It causes regression, I did this change a few weeks ago - wrong checkout?

Also needs some new tests

jelly · 2022-08-22T08:59:39Z

This breaks pixel tests: https://cockpit-logs.us-east-1.linodeobjects.com/pull-961-20220819-084819-8540cbeb-fedora-35/TestApplication-testRunImageSystem-integration-pixels.png

It causes regression, I did this change a few weeks ago - wrong checkout?

Also needs some new tests

Hmm the code was moved around so maybe it doesn't pick up some css changes?

jelly · 2022-09-19T09:05:45Z

Failing tests on ubuntu-2204 are due to podman not support user mounts on 3.4.4 and requires 4.0.0 so our gui should hide it containers/podman#10379

jelly · 2022-09-27T10:02:24Z

Issue summary:

Fedora-coreos/debian-testing testCreatePodUser fails for unknown reasons
Changed Pixel tests for https://cockpit-logs.us-east-1.linodeobjects.com/pull-961-20220927-091133-81d25dcc-fedora-36/pixeldiff.html#TestApplication-testRunImageSystem-integration-medium
Ok, issue here is:
- Our own scss is not applied, ImageRunModal.scss has CSS included in Volume.jsx and ImageRunModal.scss now, that makes no sense at all.
Requires a new review

marusak · 2022-09-29T14:22:32Z

Why Owner label is not on the same row?

Like here for example?

Also found it a bit confusing, that when you create pod and give it ports and volumes, you don't see them in the UI unless you add there container and start it.

jelly · 2022-09-29T16:00:25Z

Also found it a bit confusing, that when you create pod and give it ports and volumes, you don't see them in the UI unless you add there container and start it.

Agreed, however.... the Ports can be found in infraContainer but for the mounts we need infraContainerDetails and that is only available when the pod is started. We should ask the podman guys to expose the mounts in the containers information to make this work nicely.

This implements the ability to create a podman pod for the user and admin session. A pod can have a volume and port mapping, note that volumes are only supported from podman 4.0.

marusak

Superb, thank you!

marusak · 2022-10-03T08:49:39Z

Can you please write proper release note with screenshot as well?

Has been fixed

strzinek · 2022-10-03T15:40:51Z

Thanks everyone for pushing things through, I haven't had any time to pursue this myself.

jelly · 2022-10-04T07:23:30Z

Thanks everyone for pushing things through, I haven't had any time to pursue this myself.

No problem, thanks for the initial PR!

jelly requested changes Apr 20, 2022

View reviewed changes

jelly reviewed Apr 20, 2022

View reviewed changes

strzinek requested review from garrett and jelly May 2, 2022 17:40

jelly previously requested changes May 18, 2022

View reviewed changes

jelly force-pushed the create_pod branch 2 times, most recently from 1050367 to 8540cbe Compare August 19, 2022 08:22

jelly force-pushed the create_pod branch 2 times, most recently from a34867f to bfe5b80 Compare September 2, 2022 15:31

jelly changed the title ~~Better pod management~~ Create Pod Functionality Sep 5, 2022

jelly mentioned this pull request Sep 6, 2022

images: add pod infra container cockpit-project/bots#3835

Closed

1 task

jelly added the release-note label Sep 14, 2022

jelly force-pushed the create_pod branch from bfe5b80 to 264c00d Compare September 15, 2022 15:07

jelly force-pushed the create_pod branch from 264c00d to f6d39f6 Compare September 19, 2022 09:20

jelly force-pushed the create_pod branch 3 times, most recently from dba46cf to 81d25dc Compare September 27, 2022 08:55

jelly force-pushed the create_pod branch 3 times, most recently from e244962 to b267776 Compare September 27, 2022 18:17

jelly self-requested a review September 27, 2022 20:56

Support creation of pods

be478c6

This implements the ability to create a podman pod for the user and admin session. A pod can have a volume and port mapping, note that volumes are only supported from podman 4.0.

jelly force-pushed the create_pod branch from b267776 to be478c6 Compare September 29, 2022 16:01

jelly requested a review from marusak September 30, 2022 08:52

marusak approved these changes Oct 3, 2022

View reviewed changes

marusak merged commit 5d98049 into cockpit-project:main Oct 3, 2022

garrett mentioned this pull request Oct 19, 2022

Cockpit 278 cockpit-project/cockpit-project.github.io#568

Merged

jelly removed the release-note label Nov 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Pod Functionality #961

Create Pod Functionality #961

strzinek commented Apr 14, 2022 •

edited by garrett

Loading

jelly left a comment

jelly Apr 20, 2022

strzinek Apr 20, 2022

jelly May 18, 2022

jelly Apr 20, 2022

strzinek Apr 20, 2022

garrett Apr 25, 2022

garrett Apr 25, 2022

strzinek May 2, 2022

jelly May 18, 2022

garrett commented Apr 25, 2022

strzinek commented May 2, 2022

strzinek commented May 12, 2022

jelly May 18, 2022

strzinek May 19, 2022

jelly May 23, 2022 •

edited

Loading

jelly May 18, 2022

jelly May 18, 2022

jelly commented May 18, 2022

jelly commented Aug 8, 2022 •

edited

Loading

jelly commented Aug 19, 2022

marusak commented Aug 22, 2022

jelly commented Aug 22, 2022

jelly commented Sep 19, 2022

jelly commented Sep 27, 2022 •

edited

Loading

marusak commented Sep 29, 2022

jelly commented Sep 29, 2022

marusak left a comment

marusak commented Oct 3, 2022

strzinek commented Oct 3, 2022

jelly commented Oct 4, 2022

Create Pod Functionality #961

Create Pod Functionality #961

Conversation

strzinek commented Apr 14, 2022 • edited by garrett Loading

Ability to create new pod group

jelly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garrett commented Apr 25, 2022

strzinek commented May 2, 2022

strzinek commented May 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelly May 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelly commented May 18, 2022

jelly commented Aug 8, 2022 • edited Loading

jelly commented Aug 19, 2022

marusak commented Aug 22, 2022

jelly commented Aug 22, 2022

jelly commented Sep 19, 2022

jelly commented Sep 27, 2022 • edited Loading

marusak commented Sep 29, 2022

jelly commented Sep 29, 2022

marusak left a comment

Choose a reason for hiding this comment

marusak commented Oct 3, 2022

strzinek commented Oct 3, 2022

jelly commented Oct 4, 2022

strzinek commented Apr 14, 2022 •

edited by garrett

Loading

jelly May 23, 2022 •

edited

Loading

jelly commented Aug 8, 2022 •

edited

Loading

jelly commented Sep 27, 2022 •

edited

Loading