Skip to content
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

Show user owned containers and images #173

Merged
merged 3 commits into from
Oct 18, 2019

Conversation

marusak
Copy link
Member

@marusak marusak commented Aug 8, 2019

This implements showing of user owned containers. Can be tried with this podman build.
Fixes #84
Fixes #83

Blocked on:

podmanuserstuff

TODOs:

  • Tests
  • Verify tests on F31
  • Add Fixes and closes lines

Follow-ups:

  • Switching (filtering) between user, root and all containers
  • Show selection of root/user in image download only when privileged.
  • Sort images and containers - first user(root?) and then root(user?). Now the order is rather random (depending which service replies quicker) and when some changes happen while cockpit is opened new or updated containers/images would end up on the bottom of the list (so mixing it up).

@marusak
Copy link
Member Author

marusak commented Aug 13, 2019

The user service landed. We can continue with this (while we wait for release).
The problem is that we cannot show statistics (CPU and Memory of running containers). This needs CGroupsV2 (which are planed for Fedora 31, I have kinda doubts about rhel-8). Are we going to wait or do we want to work around this somehow? For example we can just show there some placeholder (to show that it is running but we don't have this information).
What do you think @martinpitt @garrett ?

@martinpitt
Copy link
Member

@marusak: I wouldn't block on that. We need to be able to run on Fedora 30, RHEL 8.1, or even custom kernels without cgroupsv2. So some placeholder, or simply not showing the graphs at all, seems prudent.

@garrett
Copy link
Member

garrett commented Aug 14, 2019

I agree with @martinpitt. The cpu/memory graphs shouldn't be mixed into this PR. Also, don't show placeholders.

@marusak
Copy link
Member Author

marusak commented Aug 20, 2019

Now I am reading comment from @martinpitt and I realized I misread it the first time. So I guess I should finish it up.

The cpu/memory graphs shouldn't be mixed into this PR.

What do you mean by that? I am afraid I do not follow.

Also, don't show placeholders.

I just thought that we should show some information and not just leave those fields blank, as blank are for containers that are not running.

@garrett
Copy link
Member

garrett commented Sep 3, 2019

The conversation is basically as follows:

The problem is that we cannot show statistics (CPU and Memory of running containers). This needs CGroupsV2 (which are planed for Fedora 31, I have kinda doubts about rhel-8). Are we going to wait or do we want to work around this somehow?

@martinpitt: I wouldn't block on that.

@garrett: I agree. The cpu/memory graphs shouldn't be mixed into this PR. Also, don't show placeholders.

Basically: Don't worry about the graphs. (Don't add in a placeholder, either.) Just ignore it for now, please.

@marusak
Copy link
Member Author

marusak commented Sep 3, 2019

Just ignore it for now, please.

Just to make sure, I meant these stats:
62718373-005a5180-ba06-11e9-8ebf-8df7f4876d27

Running image would have there some values, but we cannot get those for user containers, only for root containers. It may seem a bit empty and confusing for user.

@garrett
Copy link
Member

garrett commented Sep 3, 2019

@marusak Oh, I thought you meant this (from the Docker page):

Screenshot_2019-09-03 Docker Containers - Rain

Whoops!

I guess if there are only user containers and no system containers, don't show those rows?

If it makes the code too spaghetti-ish; we could have something like has-system-containers has-user-containers as classes on the list, then do something like the following CSS:

/* Don't show CPU and Memory columns when there are only user containers */
.has-user-containers:not(.has-system-containers) .container-cpu,
.has-user-containers:not(.has-system-containers) .container-memory {
  display: none;
}

@garrett
Copy link
Member

garrett commented Sep 3, 2019

I get a bunch of blank named containers that I don't see when using podman from the command line...

Screenshot_2019-09-03 Podman Containers - Rain

At the command line, I have this:

$ podman images

REPOSITORY                                      TAG      IMAGE ID       CREATED        SIZE
localhost/brscan                                latest   0be09b995085   4 weeks ago    1.38 GB
docker.io/library/python                        3.6      1bf411c4dc77   5 weeks ago    936 MB
localhost/fedora-toolbox-garrett                30       90f2a3231b9e   6 weeks ago    420 MB
docker.io/library/fedora                        latest   2b74bf3d2430   6 weeks ago    255 MB
registry.fedoraproject.org/f30/fedora-toolbox   30       cb04d2792cf8   7 weeks ago    419 MB
registry.fedoraproject.org/f31/fedora-toolbox   31       940fabe824b3   4 months ago   493 MB

I have been using toolbox, so perhaps it's related...

$ toolbox list

Images created by toolbox
IMAGE ID      IMAGE NAME                                        CREATED
90f2a3231b9e  localhost/fedora-toolbox-garrett:30               6 weeks ago
cb04d2792cf8  registry.fedoraproject.org/f30/fedora-toolbox:30  7 weeks ago
940fabe824b3  registry.fedoraproject.org/f31/fedora-toolbox:31  4 months ago

Containers created by toolbox
CONTAINER ID  CONTAINER NAME     CREATED      STATUS                    IMAGE NAME
146f51127d55  31-2               5 hours ago  Created                   registry.fedoraproject.org/f31/fedora-toolbox:31
a00508c0e94e  31                 5 weeks ago  Created                   registry.fedoraproject.org/f31/fedora-toolbox:31
146f51127d55  31-2               5 hours ago  Created                   registry.fedoraproject.org/f31/fedora-toolbox:31
a7407e40fc9d  cockpit-website    6 weeks ago  Exited (143) 2 weeks ago  registry.fedoraproject.org/f30/fedora-toolbox:30
dcc67f3b05b3  cockpit            6 weeks ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
a7407e40fc9d  cockpit-website    6 weeks ago  Exited (143) 2 weeks ago  registry.fedoraproject.org/f30/fedora-toolbox:30
82617a4d650d  darktable          5 weeks ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
09ad2732ad17  displaycal         4 weeks ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
f89f41388c2b  fedora-toolbox-30  4 weeks ago  Up 3 hours ago            registry.fedoraproject.org/f30/fedora-toolbox:30
fea1a33c8e5d  google-chrome      6 weeks ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
34904bd9e74d  rpmbuild           4 hours ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
28b87b2ede78  test               5 hours ago  Created                   registry.fedoraproject.org/f30/fedora-toolbox:30
e2c79d4c50fe  test2              5 hours ago  Created                   registry.fedoraproject.org/f31/fedora-toolbox:31
e2c79d4c50fe  test2              5 hours ago  Created                   registry.fedoraproject.org/f31/fedora-toolbox:31

@marusak marusak added the no-test label Sep 3, 2019
@marusak marusak force-pushed the non-root-stuff branch 3 times, most recently from 1ad32e1 to 791cdfa Compare September 12, 2019 09:44
@marusak marusak force-pushed the non-root-stuff branch 2 times, most recently from 71e665a to 44dc0ad Compare September 12, 2019 17:54
@marusak
Copy link
Member Author

marusak commented Sep 12, 2019

This is ready for review. Anyone interested? :)
I know it is a lot of changes, but more than half of those changes are in tests.

@martinpitt
Copy link
Member

The tests failed due to the internal mirror thing, that should be fixed now. Retrying.

@martinpitt
Copy link
Member

Nice work! Initial testing:

  • I do see my system and user container images.
  • I also see my user container image (I don't have any system one). The nitpick here is that the "State" column is too narrow, as the command gets too much width. That might be unrelated to this PR, though:

podman-user

  • Trying to get a console of my toolbox doesn't work: It immediately says "disconnected". This could be because I already had a toolbox enter (aka podman exec shell), and possibly there can only be one of these? I tried to exit my podman shell.
  • After that, there is no "Reconnect" button, so I tried to reload the page. But that somehow upset the podman varlink process, it spun at 100% CPU and the container list was stuck at "Loading" forever. (This is not cockpit-podman's fault, but we should report it)
  • After some killing and reloading, I finally get the container view again. But console of my toolbox container still doesn't work. Possibly that's because that container has no tty enabled? When starting a new one, there's a "With terminal" checkbox, so whatever that controls might not be on for toolbox. Can we detect that and not show the Console tab in that case? (This may also not be related to this PR though, and affect system containers as well)
  • I started a new container from cockpit/unit-tests, with terminal. For that the Console works.
  • I force-deleted the container from the previous step. It went away from podman ps, but not the c-podman "Containers" list. Seems we are missing an event there? Reloading the page once again makes the podman varlink process freak out, and the page stuck at "Loading..".

So, ignoring all the podman varlink process bugs, and the unrelated Console unavailability, I think the only issue that's really related to this PR is the State column width.

@martinpitt
Copy link
Member

It may be worth re-testing this with a current podman (the F30/F31release is a month old). Not sure how well that can be tested from a git build.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely through yet (sorry, need to run), but this is the first review batch. Thanks!

src/util.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
src/Containers.jsx Outdated Show resolved Hide resolved
src/ImageSearchModal.jsx Outdated Show resolved Hide resolved
src/ImageSearchModal.jsx Outdated Show resolved Hide resolved
src/ImageSearchModal.jsx Outdated Show resolved Hide resolved
src/ImageUsedBy.jsx Outdated Show resolved Hide resolved
@marusak
Copy link
Member Author

marusak commented Sep 16, 2019

The nitpick here is that the "State" column is too narrow, as the command gets too much width.

Good point. This is most likely not related, it is now just more obvious. I'll try to style it.

Trying to get a console of my toolbox doesn't work: It immediately says "disconnected"

Can you podman attach <container>? That is exactly what it is doing.

Possibly that's because that container has no tty enabled?

This works for me. I think the problem might be the executable of your toolbox container.

It went away from podman ps, but not the c-podman "Containers" list. Seems we are missing an event there?

I think that podman was just acting up. Normally it works.

Reloading the page once again makes the podman varlink process freak out

What version of podman do you use? I think I haven't seen such problems so often.

Thank you Martin for the review and comments, I'll look into those:)

@martinpitt
Copy link
Member

Trying to get a console of my toolbox doesn't work: It immediately says "disconnected"

Can you podman attach <container>? That is exactly what it is doing.

podman attach devel just hangs without showing any error. I tried to type ls but it doesn't do anything. So this is broken at the podman level then.

What version of podman do you use? I think I haven't seen such problems so often.

podman-1.5.1-3.fc30.x86_64

@marusak
Copy link
Member Author

marusak commented Oct 15, 2019

Needs cockpit-project/bots#138

@martinpitt
Copy link
Member

The fedora-30 failure looks like it's going to need a special case for not having cgroupsv2 yet?

marusak added a commit to marusak/cockpit-podman that referenced this pull request Oct 15, 2019
marusak added a commit to marusak/cockpit-podman that referenced this pull request Oct 15, 2019
marusak added a commit to marusak/cockpit-podman that referenced this pull request Oct 15, 2019
marusak added a commit to marusak/cockpit-podman that referenced this pull request Oct 16, 2019
@marusak
Copy link
Member Author

marusak commented Oct 16, 2019

Fedora-31 very often does not shut down in image-customize. This needs to be looked at. https://logs-https-cockpit.apps.ci.centos.org/logs/pull-173-20191016-054343-ee67d486-cockpit-project-cockpit-podman--fedora-31/log.html (outside of this PR)

martinpitt
martinpitt previously approved these changes Oct 16, 2019
@martinpitt
Copy link
Member

martinpitt commented Oct 17, 2019

Debugging the shutdown hang a bit. I can't reproduce any hang with the standard unmodified image:

  • A simple bots/vm-run fedora-31 (in a clean tree, no overlay) doesn't hang, it reboots cleanly.
  • time bots/image-customize -v -r true fedora-31 is 17s on my system, and works in a loop.
  • bots/image-customize -v -i cockpit-ws -s test/vm.install fedora-31 doesn't hang (everything except installing the built cockpit-podman, which should be fairly unintrusive)
  • The full TEST_OS=fedora-31 make vm also works fine.

However, it does hang with the vm.install change from this PR. The smallest reproducer is

bots/image-customize -v -r 'sudo -i -u admin podman ps' fedora-31

while running podman help or another command in the admin session doesn't hang:

bots/image-customize -v -r 'sudo -i -u admin podman --help' fedora-31
bots/image-customize -v -r 'sudo -i -u admin whoami' fedora-31

So the problem isn't with the sudo -i -u admin part, i. e. it's not the user session itself that hangs. When you follow along with virsh console fedora-31-127.0.0.2-2201, you see that it doesn't actually starts to shut down, there's just some kernel messages (just audit, nothing useful) and the entire thing freezes -- no ssh any more and no VT login.

Doing this more interactively with bots/vm-run fedora-31, VT-logging in directly as admin, and running podman ps works, though -- no lockup, one can reboot without problems. Same with logging in as root and doing it through sudo. Same with ssh c 'sudo -i -u admin podman ps'.

But it's also not the command itself that hangs:

bots/vm-reset; bots/image-customize -v -r 'echo BOOT; sleep 5; echo CMD; sudo -i -u admin podman ps; sleep 5; echo DONE; sleep 5; echo REBOOT' fedora-31

During all phases I can login with ssh and the machine works, and it gets insta-broken at REBOOT.

With a slightly longer sleep, I can console-attach and log in after DONE, run reboot, and get the same.

There are still some admin processes running after that command, mostly systemd --user, dbus, and podman itself. But killing them doesn't help:

bots/vm-reset; bots/image-customize -v -r 'sudo -i -u admin podman ps; pkill -e -u admin; echo DONE; pgrep -au admin' fedora-31

So let's enable debug mode:

bots/vm-reset; bots/image-customize -v -r 'grub2-editenv - set "$(grub2-editenv list | grep ^kernelopts) debug"' fedora-31
bots/image-customize -v -r 'sudo -i -u admin podman ps' fedora-31

And now of course it doesn't hang! (grumble Heisenbug)

I tried the same with a standard F31 cloud image:

curl -L -O https://download.fedoraproject.org/pub/fedora/linux/releases/test/31_Beta/Cloud/x86_64/images/Fedora-Cloud-Base-31_Beta-1.1.x86_64.raw.xz

curl -O https://raw.githubusercontent.com/cockpit-project/bots/master/machine/cloud-init.iso
curl -O https://raw.githubusercontent.com/cockpit-project/bots/master/machine/identity.pub
curl -O https://raw.githubusercontent.com/cockpit-project/bots/master/machine/identity
chmod 600 identity

qemu-system-x86_64 -enable-kvm -display sdl -m 1024 Fedora-Cloud-Base-31_Beta-1.1.x86_64.qcow2  -snapshot -cdrom cloud-init.iso -net nic,model=virtio -net user,hostfwd=tcp::2222-:22

ssh -p 2222 -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o CheckHostIP=no -i identity root@localhost dnf install -y podman
ssh -p 2222 -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o CheckHostIP=no -i identity root@localhost 'sudo -i -u admin podman ps; reboot'

and that reproduces it as well.

I filed this as https://bugzilla.redhat.com/show_bug.cgi?id=1762663

@martinpitt
Copy link
Member

@marusak : I filed the kernel bug (see above); ideas for workaround:

  • enable debug, do vm.install, disable debug again (three invocations of image-customize)
  • append "sync; poweroff -f" to the command and ignore the failure

Makefile Outdated Show resolved Hide resolved
Makefile Outdated
@@ -123,7 +123,10 @@ rpm: dist-gzip $(RPM_NAME).spec
# build a VM with locally built rpm installed
$(VM_IMAGE): rpm bots
rm -f $(VM_IMAGE) $(VM_IMAGE).qcow2
# HACK: https://bugzilla.redhat.com/show_bug.cgi?id=1762663
if [ $(TEST_OS) == "fedora-31" ]; then bots/image-customize -v -r 'grub2-editenv - set "$$(grub2-editenv list | grep ^kernelopts) debug"' $(TEST_OS); fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bashism, please use a single =

@marusak marusak merged commit 1531a6e into cockpit-project:master Oct 18, 2019
marusak added a commit that referenced this pull request Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display owner show and manage rootless containers
4 participants