Add annotation that cockpit created given container

[marusak]

This helps us to recognize such containers and we can later better
distinguish whether we can safely recreate it or not.

[garrett]

Should we add a version? It might be useful in the future. (Not sure what reason, but it still could possibly?)

Of course, we should probably check based on feature versus a version whenever possible, just like we do for browsers.

[martinpitt]

To be honest I'm not a fan of this. This contradicts our ideals, at least the "directly interacts" and the "only the project name" ones, and it also is a dangerous path. The goal suggests that it is possible to edit existing containers -- then users can do that for "created_by: cockpit" containers and break re-creation in cockpit. Editing is bad practice in general (containers should be more or less immutable, unless it's toolbox) -- what are some examples that we want to edit? E.g. add/remove volumes?

Deciding whether or not it's safe to edit a container should be done via property inspection: if there is any property that c-podman does not know about, it should be disallowed. Some concrete examples would be nice for a better discussion, do you have some?

[garrett]

It's for re-creating a container. We wouldn't block it, but if it was created by Cockpit or doesn't have specific command line flags, then it's safely recreateable within Cockpit. Otherwise, it might work or not (and we'd still let someone re-create, but with a warning).

Recreating a container is super important for the following reasons:

• Setting up a container locally, but realizing that you missed a port or volume or need a different command. Instead of having to start from scratch, you can recreate and modify the settings.
• Updating a container to the latest version of the image. Without being able to recreate the container, you'll have to start over from scratch and manually re-enter everything (including ports, volumes, etc.)

FWIW: This idea has been in the mockups since the start too (see the menu at the side): https://user-images.githubusercontent.com/10246/124738736-8694b080-df19-11eb-91e3-dae599569683.png

[martinpitt]

We wouldn't block it, but if it was created by Cockpit or doesn't have specific command line flags, then it's safely recreateable within Cockpit.

That's what I meant -- I challenge both assumptions:

• podman itself provides no real "edit" API for running containers. The closest one is podman {generate,play} kube, but it's brittle and is missing the runtime status of the container, i.e. the file system overlays. So with the existing API, any attempt of "editing" would mean data loss. If proper "edit" functionality is wanted, we need first-class podman API support for that.

• It also cannot capture extra options like --device or --network, and I somewhat doubt that it implements all of --shm-size and similar correctly. Thus "editing" would mean configuration drift. That is mitigable by carefully parsing the "inspect" json and checking for unknown properties, but it's a lot of work.

• You can create a container with cockpit, then put it under systemd maintenance (podman generate systemd) or otherwise edit it, and then it's not safe to edit in cockpit any more. This might be mitigated by only allowing this "editing" in the currently running session (without writing annotations to disk).

So IMHO a better way to call this would be "cloning" or "templating", with re-using the known/parseable properties from a given container. That makes fewer promises, and clarifies that this is a new container with fresh state. Alternatively, creating a container could also offer an input like "store these values as template: [ name ]" (kept in localStorage), and then creating a container could offer these?

[martinpitt]

See above -- this needs a lot more thought and design first.

[garrett]

podman itself provides no real "edit" API for running containers.

Yes it does: --replace. I use it on my server, with a shell script, where I pass it the exact same configuration every time (with the same exact settings Cockpit Podman supports) along with a --pull=always, as I want to always upgrade to the latest version of the container.

Here's my Nextcloud container script:

#!/bin/sh

podman run -d --init \
           -p 8000:80 \
           -v /srv/nextcloud:/var/www/html:Z \
           -v /mnt/:/mnt:Z \
           -v /srv/nextcloud-php.ini:/usr/local/etc/php/conf.d/custom.ini:Z \
	   --name NextCloud \
           --pull=always --replace \
	   --restart=unless-stopped \
	   nextcloud:latest

Cockpit-Podman supports all of this already, except for --pull=always --replace, which is what we're talking about here.

This is specifically not "cloning", as the name persists across the updated containers, thanks to --replace.

---

It also cannot capture extra options

Which is why tagging that Cockpit made the container was suggested, hence this discussion.

We have since talked about looking at the command line arguments. If they don't exist (which is what happens when Cockpit makes the container right now) or if they exist and we see arguments that Cockpit supports (and possibly ones that we know do not matter) without any other arguments, then we don't have to show a warning.

But if unsupported features do exist, then we show a warning that it will build the new container with settings that Cockpit Podman supports.

This is the same thing that's needed for "cloning" a container with a different name. The only difference here is the --replace, which replaces the container with one that has the same name.

Cloning also has other problems, such as volumes and ports that are already in use from the original container. Replacing a container doesn't have this issue.

---

put it under systemd maintenance

True. At least the --restart option gets rid of most of the reason why people would use custom systemd units.

In my script, before the autostart stuff, I also had:

podman generate systemd -n NextCloud --restart-policy=always > /etc/systemd/system/podman-nextcloud.service
systemctl daemon-reload

This regenerates the unit and reloads systemd, effectively doing the same thing as --replace, but for a systemd unit. But that's a bit out of scope of Cockpit-Podman. 😉

For what it's worth, the resulting systemd unit looks like this:

# container-NextCloud.service
# autogenerated by Podman 4.1.1
# Thu Aug 11 08:33:05 UTC 2022

[Unit]
Description=Podman container-NextCloud.service
Documentation=man:podman-generate-systemd(1)
Wants=network-online.target
After=network-online.target
RequiresMountsFor=/run/containers/storage

[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
Restart=always
TimeoutStopSec=70
ExecStart=/usr/bin/podman start NextCloud
ExecStop=/usr/bin/podman stop -t 10 NextCloud
ExecStopPost=/usr/bin/podman stop -t 10 NextCloud
PIDFile=/run/containers/storage/overlay-containers/1583c9e76e6762348e04239da564150177da01fe80d335a7ab4b5b6fd40f4828/userdata/conmon.pid
Type=forking

[Install]
WantedBy=default.target

It's possible to see that a container has a systemd unit file too:

systemctl cat '*' 2> /dev/null | grep "ExecStart=.*podman start NextCloud"

All the podman containers that have a systemd service on my system:

systemctl cat '*' 2> /dev/null | grep "ExecStart=.*podman start "
ExecStart=/usr/bin/podman start lms
ExecStart=/usr/bin/podman start NextCloud
ExecStart=/usr/bin/podman start Caddy

(I need to switch to using the new --restart feature, which wouldn't be hard really, as I already even have it in the script.)

---

store these values as template: [ name ]" (kept in localStorage)

LocalStorage isn't really the right place for having a template like this.

But even still, a template system would need some way to edit the templates, manage them, delete them, export/import, etc. I don't think that makes sense for Cockpit-Podman.

Again: The goal is to have a way to re-use the configuration of an existing container, either to:

1. update it, for one of the following reasons:
   a. correct a mistake, such as forgetting to add a port or volume
   b. replace it with a newer version without having to redo everything from scratch
2. copy to a new container with different volumes, ports, etc. (as they would clash with the first one)

A templating system does nothing for most people, as most people actually care about fixing mistakes or updating a container, not making duplicates with a different name, ports, and volumes.

[garrett]

Note that we don't need to add any annotation that it was created by Cockpit to get any of these things done.

[martinpitt]

podman itself provides no real "edit" API for running containers

Yes it does: --replace.

I'm aware of --replace. But that is not at all "editing" of an existing container. --replace does nothing about reading the configuration of the existing container and applying it to the new one (you need to explicitly give all of that again), and all the internal state of the container will be lost. In fact you could create an entirely different container (image, options, volumes, etc.) -- --replace merely means that it will kill an existing container with the same name first, no more no less. Calling this "editing" is just not appropriate.

What the aim here is to create a new container with the same settings: image label/tag (i.e. not necessarily the same image), volumes, ports, etc; possibly also the name. This is more like "templating" then. But you need some place to store that information. That can be podman inspect, podman generate kube, or something cockpit specific if we really must. All of these are hard to implement and error prone; the inspect route still seems like the most promising to me (as generate kube is still very limited).

most people actually care about fixing mistakes or updating a container

Yes, I think we agree what we mean as a result, and just disagree about naming it 😁 "Re-using configuration of an existing container". This is much much weaker than "editing an existing container", and IMHO more like templating, but I'm sure we can find a better word or just describe it in a longer way in the UI.

[jelly]

As #1270 is open now, can we close this PR and continue with the cloning approach as alternative for tagging?

[martinpitt]

Yes, agreed. This is the path to the dark side.