Add container migration support

[lenticularis39]

This requires a few changes in client.js, namely specifiying a host where the operation is executed with each Podman command.

---

This patch introduces a graphical interface for CRIU Podman container migration, making use of cockpit-bridge sessions on the source and target hosts to transfer the data.

• Shows the state of the migration process using a label (see picture below)
• Supports transferring the image if not present on the target host
• Requires cockpit-project/cockpit#14641 to work and cockpit-project/cockpit#14849 to handle larger images/checkpoints (currently both open, first one likely close to merging)
• Uses raw channel API a lot, because the higher level cockpit.js API doesn't fit well with channel redirects
• Uses Cockpit session storage data to get the list of available host to migrate to
• I implemented the kebab menu using the PatternFly Dropdown component (as in the adjecent dropdown buttons) - the kebab icon's position looks a little bit off (see picture below)
• There are currently no tests - I'm working on them (a multi-machine setup is required for that, which is a little bit tricky)
• Export phases don't report errors correctly (error message is absent, because it gets saved into the checkpoint/image file) - this will probably be fixed with the tests, since it will be likely needed there

[lenticularis39]

New changes:

• The UI was changed to let the user choose a container on the other host or a new container as the target (I'll also update the screenshots in the main post).
• Migration modal was migrated to PF4.
• Reporting of container export errors was fixed (the error message is now displayed).
• Constants using gettext in ContainerMigrateModal.jsx were moved inside the class.
• #504 was fixed for the migration modal.
• getRemoteHosts was rewritten to use the code in lib/machines.js, which was copied into cockpit-podman from cockpit
• max_read_size in the container/image import implementation was changed to fit the actual size of the file instead of being fixed.

I don't have the tests yet - I managed to provision multiple guests by replicating the multi-machine tests in cockpit, but unlike in cockpit I couldn't get guest-to-guest communication to work (IP addresses are set correctly, but neither ping nor the Cockpit remote host connections works), so I have to do some troubleshooting on this.

Edit: guest communication problem was "fixed" by using listen/connect instead of multicast for networking, therefore I can work on the tests now.

[garrett]

Checkboxes shouldn't have empty labels.

Quick fix suggestion: Make an options group with all the checkboxes instead:

Container	counter (ehhh? what is this?)
Target container name	[___________________________]
Target host	[shadowman@cockpit-podman-2]
Target container	[Create new container ⏷]
Options	☐ Keep temporary checkpoint files ☐ Leave running after migration ☐ Preserve established TCP connections ☐ Do not migrate root file-system changes ☐ Ignore statically-set IP address ☐ Ignore statically-set MAC address

(Ignore alignment; this is a side effect of GitHub tables.)

Notes:

• I don't know what container counter is or why it's here or why it's not editable
• I don't know why the container is an action to create a new container or why it's in a dropdown.
• Shouldn't the target container name default to the same as the original name?
• Are these option strings from some setting somewhere? Or are we writing ones fresh? I changed some with some simple suggestions above, but they'd benefit from more editing.
• I've grouped related items above.

We could also possibly redo the target fields with something like:

Destination	Container name [___________________] Host [shadowman@cockpit-podman-2] Action [Create new container ⏷]

[lenticularis39]

Checkboxes shouldn't have empty labels.

Quick fix suggestion: Make an options group with all the checkboxes instead:

I modeled this after the image run modal, which has two checkboxes without a label, but I agree this would be a better solution, especially considering there are multiple simple checkboxes here. I remember trying to do this when I was writing the migration modal code and having some problems with it - I'll have another look into this.

Notes:

• I don't know what container counter is or why it's here or why it's not editable

It's not a container counter, it's the name of the selected container, which is called counter, similarly to how the image run modal displays the name of the selected image.

• I don't know why the container is an action to create a new container or why it's in a dropdown.

As described above - the user either migrates the container state into an existing (stopped) container on the other host, or creates a new container. The dropdown is there to select the existing container on the other host or to select an option to create a new one (it loads the list of existing containers from the the other host).

• Shouldn't the target container name default to the same as the original name?

That would make sense, but it could lead to confusing behavior in case a container with such name already exists on the other host (the migration code doesn't differentiate between an existing container being selected in the dropdown and its name being entered via the "Create new container" option - in both cases the existing container is used), only if there is no container of the selected name a new one is created).

• Are these option strings from some setting somewhere? Or are we writing ones fresh? I changed some with some simple suggestions above, but they'd benefit from more editing.

They are based on the Podman REST API documentation for the checkpoint and restore operations, with slight changes. They also match the strings in the checkpoint and restore modals where possible. The expressions used in the Podman docs aren't the best, though, they would certainly benefit from editing.

We could also possibly redo the target fields with something like:

Destination Container name [___________________]

Host [shadowman@cockpit-podman-2]

Action [Create new container ⏷]

That looks very nice. The "Container name" field could then alter between a text input and a select, based on the action selected (maybe a radio button could also be used since there are only two options).

[garrett]

the image run modal, which has two checkboxes without a label

Ah, then the run model also needs updating (outside of this PR). Thanks for pointing it out!

It's not a container counter, it's the name of the selected container, which is called counter, similarly to how the image run modal displays the name of the selected image.

Aha! OK, that wasn't clear to me. I thought it was some "counter" value. (Perhaps I just didn't have enough coffee this Monday morning when looking over the PR. :coffee::wink:)

We might want to do source / destination to differentiate between the containers.

In this case, "Source container" and the name. Or, if source is problematic: perhaps "Original container" or "Origin container".

That would make sense, but it could lead to confusing behavior in case a container with such name already exists on the other host

There should be a check to ensure the name isn't a duplicate. When opening the dialog, it could check the default and show an error that the name already exists, and that the user needs to select another. This is the same case if someone were to happen to type in an already-existing name, except that it's checking on what the computer suggests by default.

That looks very nice. The "Container name" field could then alter between a text input and a select, based on the action selected (maybe a radio button could also be used since there are only two options).

Oh, then action should go first, before the other elements it affects.

And, yes, radio buttons are better when there are <=3 options. (Dropdowns should only be used when there are > 3 mutually exclusive options or the number of items are completely unpredictable (but generally still more than 3). That 3 number is fuzzy depending on context; it's generally between 3 to 5 items when one would consider a select/dropdown instead of a radio button list.)

We use a pattern like this in Cockpit already, where the topmost item has two inline radio buttons.

Something like: Action (x) Add (_) Replace

[garrett]

I've made some mockups, where I went back on the destination group, as I realized if we split the add/replace action into its own toplevel element, then that means there will only be a new container name on the destination host or it will replace a container on the host. (At least if I'm understanding this correctly...)

Standard migration, adding a new container on the destination

Standard migration, adding a new container, but the name conflicts

Replace another existing container

---

How does this look?

[garrett]

The combobox for the destination host would be a list of known hosts (perhaps currently used), with an option to add more by typing. It would use the PatternFly typeahead component with the isCreatable option and would override the Create "..." string using the createText property set to... "Select", I guess?

[lenticularis39]

Thank you for doing the mockups, this is definitely a significant design improvement. There are only two things that should be addressed:

• The labels in the Action option do not correspond well to what the operation does. When an existing container is selected, its state is restored from a checkpoint generated from the source container's state rather than it being replaced as a whole ("replace" would imply e.g. that its configuration is discarded and replaced, which is not the case). Something like "Create new container" and "Restore into existing container" would better describe the actions.

• Currently a non-typeahead select is used for the know host list - a combobox is certainly a good idea, because it allows searching, but creating a new entry doesn't make much sense. The list consists of hosts added via the remote host functionality in Cockpit. Because the container migration feature uses a Cockpit session to transfer the checkpoint, only hosts with an active Cockpit session can be used, therefore only those are shown and it doesn't make sense to add new ones (this should be done the usual way via the sidebar, handling authentication and so on).

I'll change the modal and once again update the screenshots once it is done.

[lenticularis39]

UI has been updated.

There are a few minor problems (the helper texts are black instead of red and for some reason the target container text input clears when closing the modal without the update event being called, leading to the validated attribute not being updated), but otherwise it should work (will be tested further in the Selenium tests once they are updated to the new UI).

[garrett]

Something like "Create new container" and "Restore into existing container" would better describe the actions.

OK, sure.

"Restore into existing container" does sound a little weird to me. Can you select an unrelated container to migrate into?

The list consists of hosts added via the remote host functionality in Cockpit.

OH! I wasn't aware! Neat! I guess this is one of the first places in the UI where we're re-using the multi-host configuration, aside from the dashboard (which we've just deprecated). We should probably think how we can improve the flow of adding new hosts.

...But for now:

We need to message how to adjust this list in the UI. I guess adding a new host from the autocomplete might be a little complex, as it'd force a dialog on top of the other dialog and it'd have to be done by calling an API that might not be cross-page capable at the moment?

Perhaps something like this:

🛈 Add destination hosts using the host selector.

(With an actual icon, not UTF-8, of course)

It would be really cool if you could click the link on that and have the host switcher show itself and somehow indicate the "add new host" button with something like a short pulse animation. However, I don't think we can do this at the moment.

But we can, at least, have the text (without the link part). It's better than nothing. Anything more than just text here should be done in a follow-up (if at all).

Also, what happens when:

1. there's only the current host (nothing to migrate to)?
   • would we have an empty state that says something about adding additional hosts as destinations using the host selector?
2. when there's only one remote destination host to migrate to? (perhaps a dropdown, but it's disabled but has the host preselcted?)

[lenticularis39]

"Restore into existing container" does sound a little weird to me. Can you select an unrelated container to migrate into?

You can, but the migration would fail - the target container has to have the same image as the source container. The UI can of course filter the container list for that, but there might be some corner cases where the image is virtually the same, but has a different ID (e.g. it has been imported from a different image format). This was the reason why I didn't want to make the image IDs matching a hard requirement for the container to be possible to select. The actual best solution would depend on whether the migration succeeds in the aforementioned corner cases - I'll test this.

We need to message how to adjust this list in the UI. I guess adding a new host from the autocomplete might be a little complex, as it'd force a dialog on top of the other dialog and it'd have to be done by calling an API that might not be cross-page capable at the moment?

Perhaps something like this:

🛈 Add destination hosts using the host selector.

(With an actual icon, not UTF-8, of course)

It would be really cool if you could click the link on that and have the host switcher show itself and somehow indicate the "add new host" button with something like a short pulse animation. However, I don't think we can do this at the moment.

But we can, at least, have the text (without the link part). It's better than nothing. Anything more than just text here should be done in a follow-up (if at all).

I'll have a look on how the code works and what can be done with that.

Also, what happens when:

1. there's only the current host (nothing to migrate to)?
   
   * would we have an empty state that says something about adding additional hosts as destinations using the host selector?

2. when there's only one remote destination host to migrate to? (perhaps a dropdown, but it's disabled but has the host preselcted?)

Nothing special happens. In the first case the target host select is empty (except for the placeholder selection):

In the second case the dropdown simply shows the placeholder and the one remote host:

I'm not sure if it's better to keep the placeholder option or to remove it and set the placeholder text (as it is with the target container:)

Perhaps the placeholder option could be removed, along with preselecting the one remote host in the case where there is only one (and also doing the same with the target container).

[lenticularis39]

• The action checkbox was changed to inline.
• Validated error labels are now red.
• Problem with target container name text field input was fixed.
• Both the target host and target container selects pre-select the only available option when there's only one.
• Target host select placeholder was changed from a placeholder item to placeholder text to match the target container select.
• A bug in client.js migration code that caused a read channel done message not to be detected was fixed.
• The branch was rebased to the current master.
• The dialog was migrated to PatternFly React forms to match the other dialogs.
• Information label about the possibility of adding more remote hosts via the machines interface was added.
• Spinner moved to the migrate button to match other modals.

About the unrelated containers in the select question - the problematic case I encountered before doesn't seem to happen anymore, so I couldn't test it. However I don't like the idea of the user not being able to select the desired container in case an unexpected image hash mismatch happens, so I kept the select as is.

Also jquery was added to package.json, because it it used by machines.js (a slightly modfified source file from Cockpit).

[lenticularis39]

Closing, since it depends on the closed cockpit-project/cockpit#14641, which I won't be working on anymore.