rkt: add command to export a pod to an aci #2889

iaguis · 2016-07-08T13:11:00Z

Adds a new export command to rkt which generates an aci from a
pod; saving any changes made to the pod.

Usage is rkt export POD_UUID my.aci

This is currently restricted to only working with exited pods with a
single app.

iaguis · 2016-07-08T14:40:21Z

There's a problem on no-overlay fly pods. Since we mount /sys, /proc and so on in the host mount namespace, when rkt export goes to the pod directory to generate the ACI, it tries to add files inside those mountpoints.

This is a problem because trying to write files from those dirs in a tar file cause general badness and errors. For example, /proc/1/attr/current can't be read and trying to read /proc/${KERNEL_PROGRAM_PID}/exe results in ENOENT.

There're several things we can do:

Avoid traversing filesystems when exporting pods. This can be a problem because we bind-mount /tmp from the host and it can be the same fs as the pod application.
Avoid traversing mountpoints. The API for this is weird in linux (see https://github.com/systemd/systemd/blob/v230/src/basic/mount-util.c#L79 for example). We can always parse mountinfo though.
Don't allow exporting fly pods. @alban suggests adding an "exportable" annotation to stage1 to distinguish this.

I think the third option sounds reasonable for now. Opinions?

tjdett · 2016-07-08T21:00:40Z

As a future user of the feature, the second option sounds the least surprising. I can't think of any situation where traversing a mount point would be expected. Presumably this is happening after volumes have been unmounted?

iaguis · 2016-07-11T16:16:57Z

Presumably this is happening after volumes have been unmounted?

The volumes are still mounted on the "fly" case after the container exits, but I think not exporting the volumes is reasonable.

euank · 2016-07-11T21:55:22Z

👍 for avoiding traversing mountpoints.

Will this also let us preserve files "masked" by mounts? I assume it will if all the mounts are unmounted first.

e.g. if the original aci has a file /tmp/foo and the user who runs it configures a /tmp bindmount from the host, will the exported aci have a /tmp/foo file? Do we care?

tjdett · 2016-07-12T02:50:10Z

I think capturing files masked by mounts is the definitely the correct behaviour.

Real example: mounting /var/log as a volume. Say you have /var/log/nginx in the rootfs, then following #2315 it should be copied to the new empty volume. This allows shared data volumes to be pre-populated.

In a rkt run → rkt export → rkt run sequence, an app that takes advantage of this behaviour would break when running the exported image unless the masked files are captured.

More abstractly, if you're using rkt export in the ways mentioned in #1197 then you're essentially using rkt run to apply a Image → Image operation against an existing Image. If that operation doesn't touch a specific part of the rootfs, then the expected behaviour would be for it to still be there in the resulting rootfs.

iaguis · 2016-07-12T10:03:03Z

I'd like to emphasize that the problems mentioned in #2889 (comment) only apply to "fly" pods.

On regular pods we already get the behavior mentioned in #2889 (comment): when the container is exited, all the mounts are unmounted.

I'm not sure if making export work with "fly" pods is worth the complexity of implementing it. Since "fly" is just meant to execute single apps with no isolation taking advantage of rkt's image distribution mechanisms, I don't think exporting "fly" pods is a very useful use case.

alban · 2016-07-12T16:20:41Z

I don't think that pods that are both no-overlay and fly should be a priority for rkt export. I'd like to get rkt export merged and handle this special case later.

To detect that rkt is in this special case in order to print an error message ("export unsupported for this kind of pod"), we could either:

use an "exportable" annotation in the stage1 image as suggested below, or
check in /proc/self/mountinfo if there is any mountpoints in the app rootfs directory.

Checking mountpoints is better because we don't need to commit to any new API between stage0 and stage1 (see Stage1 ACI implementor's guide).

iaguis · 2016-07-12T16:33:42Z

I agree with @alban

tjdett · 2016-07-13T00:09:13Z

Thanks for the reminder @iaguis. I missed the distinction that the issue you were referring to only applied to no-overlay fly stage 1 at the beginning, and I think I got a bit distracted by the "exportable" annotation suggestion.

My main concern was that there shouldn't be varying behaviour around what gets exported, and that export should generally be expected to work. Quite happy with testing for conditions which prevent a consistent export and failing on them though. (ie. Failing because things are still mounted, not because it's fly + no-overlay.)

I agree with @alban that getting something merged is much more important at this stage than having it work with no-overlay in all stage 1 implementations.

iaguis · 2016-07-13T13:04:34Z

Updated. PTAL.

tjdett · 2016-07-13T23:50:00Z

I think "unsupported, app has mountpoints" is a bit terse. How about:

unsupported, app has remaining mount points and used no-overlay

As a user, I'd have some idea what to fix then. Am I correct in assuming that the exited pod can still be cleanly exported if somebody manually removes the mount points?

Otherwise, I like it and I'm keen to see it merged. 👍

iaguis · 2016-07-14T10:50:25Z

Changed the message to:

pod has mountpoints. Only pods using overlayfs or with no mountpoints can be exported

iaguis · 2016-07-18T11:25:27Z

PTAL?

alban · 2016-07-18T12:25:46Z

common/overlay/overlay.go

+// MountCfg contains the needed data to construct the overlay mount syscall.
+// The Lower and Upper fields are paths to the filesystems to be merged. The
+// Work field should be an empty directory. Dest is where the mount will be
+// located. Lbl is an syslinux label.


syslinux label or SELinux label?

This adds a new `export` command to rkt which generates an aci from a pod; saving any changes made to the pod. Usage is `rkt export POD_UUID my.aci` This is currently restricted to only working with exited pods with a single app and that have been run with the --no-overlay option. Fixes rkt#1197

alban · 2016-07-18T12:45:58Z

tests/rkt_export_test.go

-		testContent = "ThisIsATest"
-	)
+const (
+	testAci     = "test.aci"


Why making this global? It seems to be used only 1 time.

Also tries to simplify things in areas; remove some const variable checks, etc.

It wasn't working if the overlay fs was unmounted (e.g. because of a reboot). Also: * Randomize the temporary directory where we mount the overlay fs so an inconsistent state doesn't affect future exports. * Change path.Join to filepath.Join. * Simplify overlay.Mount, we don't need to return the mounted path. * Fix typos and wrap overlay documentation.

alban · 2016-07-18T13:01:07Z

rkt/export.go

+			return 1
+		}
+		if hasMPs {
+			stderr.Printf("pod has mountpoints. Only pods using overlayfs or with no mountpoints can be exported")


What about "pod has remaining mountpoints"?

alban · 2016-07-18T13:02:19Z

Some minor questions. LGTM after that.

If we're using "fly" the mounts we do (`/dev`, `/proc`, `/sys` and `/tmp` and other user-defined mounts) are in the host's mount namespace. Trying to export this kind of pods causes problems (e.g. some files in `/proc` cannot be directly read). Also, the user doesn't expect the contents of mountpoints to be exported. This is not problematic if we use overlay, because in that case we derive the exported fs from the ACI image plus the upper layer. To avoid problems, if the pod doesn't use overlay, we refuse exporting if the app has mountpoints inside.

So we can test with fly.

iaguis · 2016-07-18T13:04:06Z

Updated.

alban · 2016-07-18T13:21:37Z

LGTM if green

alban · 2016-07-18T13:58:11Z

The failure on fedora-23 seems unrelated:

13:47:27 --- FAIL: TestSocketProxyd (7.45s)
13:47:27    rkt_socket_proxyd_test.go:200: read tcp [::1]:56160->[::1]:40181: read: connection reset by peer

It seems to fail sometimes: see #2432

iaguis added kind/enhancement component/store area/usability component/stage0 needs/review kind/design priority/P2 labels Jul 8, 2016

iaguis added this to the v1.11.0 milestone Jul 8, 2016

iaguis mentioned this pull request Jul 8, 2016

rkt: add command to export a pod to an aci #2343

Closed

iaguis force-pushed the iaguis/implement-pod-export branch from 6736dfe to a27c9fa Compare July 13, 2016 13:04

iaguis force-pushed the iaguis/implement-pod-export branch from a27c9fa to 2da24a7 Compare July 14, 2016 10:49

alban reviewed Jul 18, 2016
View reviewed changes

blixtra added 3 commits July 18, 2016 14:44

rkt: enable exporting overlayfs pods

68c7679

rkt: perform user namespace unshift on export

83f9279

alban reviewed Jul 18, 2016
View reviewed changes

functional tests: add export tests for overlay and user ns

f75847f

Also tries to simplify things in areas; remove some const variable checks, etc.

alban reviewed Jul 18, 2016
View reviewed changes

iaguis added 3 commits July 18, 2016 15:02

functional tests: test export with unmounted overlayfs

19e77f1

functional tests: refactor TestExport

461fce0

So we can test with fly.

iaguis force-pushed the iaguis/implement-pod-export branch from 2da24a7 to 461fce0 Compare July 18, 2016 13:04

iaguis merged commit b6e9d5c into rkt:master Jul 18, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rkt: add command to export a pod to an aci #2889

rkt: add command to export a pod to an aci #2889

iaguis commented Jul 8, 2016

iaguis commented Jul 8, 2016

tjdett commented Jul 8, 2016

iaguis commented Jul 11, 2016

euank commented Jul 11, 2016 •

edited

Loading

tjdett commented Jul 12, 2016

iaguis commented Jul 12, 2016

alban commented Jul 12, 2016 •

edited

Loading

iaguis commented Jul 12, 2016

tjdett commented Jul 13, 2016 •

edited

Loading

iaguis commented Jul 13, 2016

tjdett commented Jul 13, 2016

iaguis commented Jul 14, 2016

iaguis commented Jul 18, 2016

alban Jul 18, 2016

iaguis Jul 18, 2016

alban Jul 18, 2016

iaguis Jul 18, 2016

alban Jul 18, 2016

iaguis Jul 18, 2016

alban commented Jul 18, 2016

iaguis commented Jul 18, 2016

alban commented Jul 18, 2016

alban commented Jul 18, 2016 •

edited

Loading

rkt: add command to export a pod to an aci #2889

rkt: add command to export a pod to an aci #2889

Conversation

iaguis commented Jul 8, 2016

iaguis commented Jul 8, 2016

tjdett commented Jul 8, 2016

iaguis commented Jul 11, 2016

euank commented Jul 11, 2016 • edited Loading

tjdett commented Jul 12, 2016

iaguis commented Jul 12, 2016

alban commented Jul 12, 2016 • edited Loading

iaguis commented Jul 12, 2016

tjdett commented Jul 13, 2016 • edited Loading

iaguis commented Jul 13, 2016

tjdett commented Jul 13, 2016

iaguis commented Jul 14, 2016

iaguis commented Jul 18, 2016

alban Jul 18, 2016

Choose a reason for hiding this comment

iaguis Jul 18, 2016

Choose a reason for hiding this comment

alban Jul 18, 2016

Choose a reason for hiding this comment

iaguis Jul 18, 2016

Choose a reason for hiding this comment

alban Jul 18, 2016

Choose a reason for hiding this comment

iaguis Jul 18, 2016

Choose a reason for hiding this comment

alban commented Jul 18, 2016

iaguis commented Jul 18, 2016

alban commented Jul 18, 2016

alban commented Jul 18, 2016 • edited Loading

euank commented Jul 11, 2016 •

edited

Loading

alban commented Jul 12, 2016 •

edited

Loading

tjdett commented Jul 13, 2016 •

edited

Loading

alban commented Jul 18, 2016 •

edited

Loading