system: add new subcommand "migrate" #2950

giuseppe · 2019-04-15T20:06:59Z

it is useful to migrate existing containers to a new version of
podman. Currently, it is needed to migrate rootless containers that
were created with podman <= 1.2 to a newer version which requires all
containers to be running in the same user namespace.

Closes: #2935

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

openshift-ci-robot · 2019-04-15T20:07:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [giuseppe]

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

mheon · 2019-04-15T20:08:33Z

libpod/runtime.go

We want to be holding the alive lock here, to ensure that other Podman instances don't start while we're still migrating containers

If the BecomeRoot call prevents us from grabbing it here, migrate() itself should take the lock and hold it

once we re-exec, the rootless instance will run with euid==0 and it will hold the lock. This is to prevent a deadlock with the re-execed process

mheon · 2019-04-15T20:09:06Z

libpod/runtime_migrate.go

We should at least log errors here

probably here it is safer to return on error. I'll change it

libpod/runtime_migrate.go

mheon · 2019-04-15T20:10:55Z

libpod/runtime_migrate.go

Can we try and match the full path, in case people were actually specifying a non-default path?

sure, changing it

mheon · 2019-04-15T20:11:15Z

libpod/runtime_migrate.go

Again, probably worth logging failures, at least

Also, maybe we should plumb a context in here through NewRuntime.

docs/podman-system-migrate.1.md

TomSweeneyRedHat · 2019-04-16T16:55:08Z

libpod/runtime_migrate.go

This looks like it can't be rolled back. Do we want to add a "Are your sure you want to do this?" kind of prompt in here somewhere?

I expect this to be used in a non interactive way, e.g. as part of the upgrade

TomSweeneyRedHat

Need to update command.md with an entry for this command.
Any chance to add a test? Seems hard to pull off, but thought I'd ask.

giuseppe · 2019-04-16T20:57:41Z

Need to update command.md with an entry for this command.

do I need to add it? It is a subcommand of system

Any chance to add a test? Seems hard to pull off, but thought I'd ask.

we would need to have an old version of podman preinstalled

TomSweeneyRedHat · 2019-04-16T23:44:24Z

I'll let @mheon make the call on the "should it be on commands.md" or not. I think yes. Currently we have separate listings for "podman container *" and "podman volume *", but we don't do the same for "podman system *".

The test sounds like a headache, I'd not worry about it then. Sounds like you were considering the same roadblock that I was.

rh-atomic-bot · 2019-04-17T17:15:55Z

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

mheon · 2019-04-17T17:55:07Z

@giuseppe @TomSweeneyRedHat If system renumber is in the commands page, we add this; if it's not, we don't. I think we ought to match what the existing similar commands do.

rhatdan · 2019-04-17T18:02:34Z

Is this something we should run in the postinstall? How are we going to tell individual users that we need to run this. Can we do a check in the podman command?

mheon · 2019-04-24T13:45:32Z

libpod/options.go

Isn't this not lock-related so much as pidfile-related? I'd call Migrate a generic way to update container configuration between versions

mheon · 2019-04-24T13:46:32Z

libpod/options.go

I feel like we ought to pass this through NewRuntime() as an argument instead of with an option

rhatdan · 2019-04-24T16:55:42Z

Needs a rebase to get the SELinux fixes.

giuseppe · 2019-04-24T20:51:37Z

tests are happy!

mheon · 2019-04-24T20:52:54Z

I'll hit my comments once this merges in a separate PR, let's get this landed.
LGTM

rh-atomic-bot · 2019-04-24T21:00:59Z

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

mheon · 2019-04-24T21:01:21Z

@giuseppe You picked up merge conflicts.

giuseppe · 2019-04-24T21:09:11Z

@giuseppe You picked up merge conflicts.

rebased again

TomSweeneyRedHat · 2019-04-24T21:12:45Z

LGTM and happy green test buttons.

rh-atomic-bot · 2019-04-25T19:22:38Z

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

rh-atomic-bot · 2019-04-25T22:03:41Z

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

it is useful to migrate existing containers to a new version of podman. Currently, it is needed to migrate rootless containers that were created with podman <= 1.2 to a newer version which requires all containers to be running in the same user namespace. Closes: containers#2935 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2019-04-26T22:22:22Z

tests are green again

mheon · 2019-04-26T22:27:55Z

/lgtm
I'll submit a followup over the weekend or on monday to hit my nits

We merged containers#2950 with some nits still remaining, as Giuseppe was going on PTO. This addresses those small requested changes. Signed-off-by: Matthew Heon <matthew.heon@pm.me>

Small fixes for #2950

openshift-ci-robot requested review from rhatdan and umohnani8 April 15, 2019 20:07

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L labels Apr 15, 2019

giuseppe changed the title ~~system: add new subcommand "migrate"~~ [WIP] system: add new subcommand "migrate" Apr 15, 2019

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 15, 2019

giuseppe mentioned this pull request Apr 15, 2019

podman list fails in rawhide #2935

Closed

mheon reviewed Apr 15, 2019

View reviewed changes

libpod/runtime_migrate.go Outdated Show resolved Hide resolved

mheon reviewed Apr 15, 2019

View reviewed changes

giuseppe force-pushed the podman-system-migrate branch 3 times, most recently from a4a45fc to 431994c Compare April 16, 2019 07:39

giuseppe changed the title ~~[WIP] system: add new subcommand "migrate"~~ system: add new subcommand "migrate" Apr 16, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2019

TomSweeneyRedHat reviewed Apr 16, 2019

View reviewed changes

docs/podman-system-migrate.1.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Apr 16, 2019

View reviewed changes

docs/podman-system-migrate.1.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Apr 16, 2019

View reviewed changes

docs/podman-system-migrate.1.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Apr 16, 2019

View reviewed changes

giuseppe force-pushed the podman-system-migrate branch from 1344dc3 to 301b7f8 Compare April 16, 2019 17:14

mheon added the Release Notes 1.3.0 label Apr 17, 2019

mheon reviewed Apr 24, 2019

View reviewed changes

libpod/options.go Outdated

Copy link

Member

mheon Apr 24, 2019

Choose a reason for hiding this comment

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

I feel like we ought to pass this through NewRuntime() as an argument instead of with an option

giuseppe force-pushed the podman-system-migrate branch from e781039 to 1156494 Compare April 24, 2019 18:46

giuseppe force-pushed the podman-system-migrate branch from 1156494 to fd60dd7 Compare April 24, 2019 21:09

giuseppe force-pushed the podman-system-migrate branch from fd60dd7 to e2c2e6e Compare April 25, 2019 20:17

giuseppe force-pushed the podman-system-migrate branch from e2c2e6e to 182e761 Compare April 26, 2019 06:58

giuseppe added 2 commits April 26, 2019 22:23

runtime: pass down the context

f49e0c1

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the podman-system-migrate branch from 182e761 to f49e0c1 Compare April 26, 2019 20:24

openshift-ci-robot assigned mheon Apr 26, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2019

openshift-merge-robot merged commit fe3acdd into containers:master Apr 26, 2019

This was referenced Apr 26, 2019

We should allow multiple containers storage drivers. #1847

Closed

WIP: test for https://github.com/containers/storage/pull/317 #2976

Closed

mheon mentioned this pull request May 1, 2019

Small fixes for #2950 #3051

Merged

openshift-merge-robot added a commit that referenced this pull request May 2, 2019

Merge pull request #3051 from mheon/podman_migrate_fixes

139eeb3

Small fixes for #2950

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 bot locked as resolved and limited conversation to collaborators Sep 26, 2023

system: add new subcommand "migrate" #2950

system: add new subcommand "migrate" #2950

Uh oh!

Conversation

giuseppe commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Apr 16, 2019

Uh oh!

TomSweeneyRedHat commented Apr 16, 2019

Uh oh!

rh-atomic-bot commented Apr 17, 2019

Uh oh!

mheon commented Apr 17, 2019

Uh oh!

rhatdan commented Apr 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhatdan commented Apr 24, 2019

Uh oh!

giuseppe commented Apr 24, 2019

Uh oh!

mheon commented Apr 24, 2019

Uh oh!

rh-atomic-bot commented Apr 24, 2019

Uh oh!

mheon commented Apr 24, 2019

Uh oh!

giuseppe commented Apr 24, 2019

Uh oh!

TomSweeneyRedHat commented Apr 24, 2019

Uh oh!

rh-atomic-bot commented Apr 25, 2019

Uh oh!

rh-atomic-bot commented Apr 25, 2019

Uh oh!

giuseppe commented Apr 26, 2019

Uh oh!

mheon commented Apr 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

giuseppe commented Apr 15, 2019 •

edited

Loading