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

Include changes to the container's root file-system in the checkpoint archive #3443

Merged

Conversation

adrianreber
Copy link
Collaborator

One limitation of Podman's container migration feature was that it was problematic to migrate containers which changed their root file-system. The recommendation until now was to mount all directories which the container potentially changes with --tmpfs. Everything in a --tmpfs was already included in the checkpoint archive.

To make it easier for the users to migrate a container, without first mounting all directories with possible changes with --tmpfs, this now adds container root file-system changes to the checkpoint archive.

To extract the changes to the container's root file-system the functionality behind podman diff is used. There were discussion about using podman commit, but that would mean that the restored container is now referencing another storage top layer than it was started with. Using podman diff makes it possible to have the restored container the same as possible as the original container.

If the root file-system changes are very large or unnecessary for the container migration there is now a new option for checkpoint and restore called: --ignore-rootfs. This option tells Podman to not include the root file-system changes in the checkpoint archive during checkpointing, or if they are part of the checkpoint archive to ignore the root file-system changes during restore.

@rh-atomic-bot
Copy link
Collaborator

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@adrianreber adrianreber force-pushed the rootfs-changes-migration branch 4 times, most recently from 2aea592 to bdabbc2 Compare June 27, 2019 13:43
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #3442) made this pull request unmergeable. Please resolve the merge conflicts.

@adrianreber adrianreber force-pushed the rootfs-changes-migration branch 4 times, most recently from a39a548 to e12aa9a Compare June 27, 2019 19:22
@baude
Copy link
Member

baude commented Jun 28, 2019

LGTM

@mheon
Copy link
Member

mheon commented Jun 28, 2019 via email

@baude
Copy link
Member

baude commented Jun 28, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2019
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably #3496) made this pull request unmergeable. Please resolve the merge conflicts.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2019
The newly added function GetDiffTarStream() mirrors the GetDiff()
function. It tries to get the correct layer ID from getLayerID()
and it filters out containerMounts from the tarstream. Thus the
behavior is the same as GetDiff(), but it returns a tarstream.

This also adds the function ApplyDiffTarStream() to apply the tarstream
generated by GetDiffTarStream().

These functions are targeted to support container migration with
root file-system changes.

Signed-off-by: Adrian Reber <[email protected]>
One of the last limitations when migrating a container using Podman's
'podman container checkpoint --export=/path/to/archive.tar.gz' was
that it was necessary to manually handle changes to the container's root
file-system. The recommendation was to mount everything as --tmpfs where
the root file-system was changed.

This extends the checkpoint export functionality to also include all
changes to the root file-system in the checkpoint archive. The
checkpoint archive now includes a tarstream of the result from 'podman
diff'. This tarstream will be applied to the restored container before
restoring the container.

With this any container can now be migrated, even it there are changes
to the root file-system.

There was some discussion before implementing this to base the root
file-system migration on 'podman commit', but it seemed wrong to do
a 'podman commit' before the migration as that would change the parent
layer the restored container is referencing. Probably not really a
problem, but it would have meant that a migrated container will always
reference another storage top layer than it used to reference during
initial creation.

Signed-off-by: Adrian Reber <[email protected]>
The newly added functionality to include the container's root
file-system changes into the checkpoint archive can now be explicitly
disabled. Either during checkpoint or during restore.

If a container changes a lot of files during its runtime it might be
more effective to migrated the root file-system changes in some other
way and to not needlessly increase the size of the checkpoint archive.

If a checkpoint archive does not contain the root file-system changes
information it will automatically be skipped. If the root file-system
changes are part of the checkpoint archive it is also possible to tell
Podman to ignore these changes.

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber adrianreber force-pushed the rootfs-changes-migration branch from e12aa9a to 875cafb Compare July 11, 2019 12:44
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2019
This adds three tests for the --ignore-rootfs option to verify that it
works in all combination.

1. Not used at all
2. Only used during restore
3. Only used during checkpoint

Signed-off-by: Adrian Reber <[email protected]>
@adrianreber adrianreber force-pushed the rootfs-changes-migration branch from 875cafb to c70657a Compare July 11, 2019 13:27
@adrianreber
Copy link
Collaborator Author

Rebased and all tests are green. Can the hold be removed?

@mheon
Copy link
Member

mheon commented Jul 11, 2019

/hold cancel
/approve

Will review today or tomorrow

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2019
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianreber, mheon

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2019
@adrianreber
Copy link
Collaborator Author

@mheon I see that you are really busy with many other PRs, so this is just a friendly ping if you have a chance to look at this.

@rhatdan
Copy link
Member

rhatdan commented Jul 19, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 19, 2019
@openshift-merge-robot openshift-merge-robot merged commit deb087d into containers:master Jul 19, 2019
@rh-atomic-bot rh-atomic-bot mentioned this pull request Jul 19, 2019
7 tasks
@github-actions 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 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants