diff --git a/cmd/podman/containers/commit.go b/cmd/podman/containers/commit.go index 453b6d6a92..6ea67da212 100644 --- a/cmd/podman/containers/commit.go +++ b/cmd/podman/containers/commit.go @@ -77,7 +77,7 @@ func commitFlags(cmd *cobra.Command) { flags.StringVarP(&commitOptions.Author, authorFlagName, "a", "", "Set the author for the image committed") _ = cmd.RegisterFlagCompletionFunc(authorFlagName, completion.AutocompleteNone) - flags.BoolVarP(&commitOptions.Pause, "pause", "p", false, "Pause container during commit") + flags.BoolVarP(&commitOptions.Pause, "pause", "p", true, "Pause container during commit") flags.BoolVarP(&commitOptions.Quiet, "quiet", "q", false, "Suppress output") flags.BoolVarP(&commitOptions.Squash, "squash", "s", false, "squash newly built layers into a single new layer") flags.BoolVar(&commitOptions.IncludeVolumes, "include-volumes", false, "Include container volumes as image volumes") diff --git a/docs/source/markdown/podman-commit.1.md b/docs/source/markdown/podman-commit.1.md index 7964197553..a02d438aa5 100644 --- a/docs/source/markdown/podman-commit.1.md +++ b/docs/source/markdown/podman-commit.1.md @@ -9,7 +9,7 @@ podman\-commit - Create new image based on the changed container **podman container commit** [*options*] *container* [*image*] ## DESCRIPTION -**podman commit** creates an image based on a changed *container*. The author of the image can be set using the **--author** OPTION. Various image instructions can be configured with the **--change** OPTION and a commit message can be set using the **--message** OPTION. The *container* and its processes aren't paused while the image is committed. If this is not desired, the **--pause** OPTION can be set to *true*. When the commit is complete, Podman prints out the ID of the new image. +**podman commit** creates an image based on a changed *container*. The author of the image can be set using the **--author** OPTION. Various image instructions can be configured with the **--change** OPTION and a commit message can be set using the **--message** OPTION. The *container* and its processes are paused while the image is committed. If this is not desired, the **--pause** OPTION can be set to *false*. When the commit is complete, Podman prints out the ID of the new image. If `image` does not begin with a registry name component, `localhost` is added to the name. If `image` is not provided, the values for the `REPOSITORY` and `TAG` values of the created image is set to ``. @@ -64,8 +64,9 @@ Set commit message for committed image.\ #### **--pause**, **-p** -Pause the container when creating an image.\ -The default is **false**. +Pause the container when creating an image. +Pausing a running container during commit prevents race conditions and potential security issues caused by other processes accessing the container's rootfs while the image is being created.\ +The default is **true**. #### **--quiet**, **-q** @@ -104,9 +105,9 @@ $ podman commit -q --author "firstName lastName" reverent_golick image-committed e3ce4d93051ceea088d1c242624d659be32cf1667ef62f1d16d6b60193e2c7a8 ``` -Pause running container while creating image: +Commit running container without pausing: ``` -$ podman commit -q --pause=true containerID image-committed +$ podman commit -q --pause=false containerID image-committed e3ce4d93051ceea088d1c242624d659be32cf1667ef62f1d16d6b60193e2c7a8 ``` diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index eb879a522a..8638f9865e 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -114,7 +114,8 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) { Tag string `schema:"tag"` // fromSrc string # fromSrc is currently unused }{ - Tag: "latest", + Tag: "latest", + Pause: true, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { @@ -122,9 +123,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) { return } sc := runtime.SystemContext() - options := libpod.ContainerCommitOptions{ - Pause: true, - } + options := libpod.ContainerCommitOptions{} options.CommitOptions = buildah.CommitOptions{ ReportWriter: os.Stderr, SystemContext: sc, diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index c6e677132a..3ab226cacd 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -505,6 +505,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) { Tag string `schema:"tag"` }{ Format: "oci", + Pause: true, } if err := decoder.Decode(&query, r.URL.Query()); err != nil { @@ -513,9 +514,7 @@ func CommitContainer(w http.ResponseWriter, r *http.Request) { } sc := runtime.SystemContext() tag := "latest" - options := libpod.ContainerCommitOptions{ - Pause: true, - } + options := libpod.ContainerCommitOptions{} switch query.Format { case "oci": mimeType = buildah.OCIv1ImageManifest diff --git a/test/system/126-commit.bats b/test/system/126-commit.bats new file mode 100644 index 0000000000..b7f7810604 --- /dev/null +++ b/test/system/126-commit.bats @@ -0,0 +1,81 @@ +#!/usr/bin/env bats -*- bats -*- +# +# tests for podman commit +# + +load helpers + +# Helper: use poll(2) on a cgroup.events file to detect state changes +# without polling. +_cgroup_watch() { + python3 -c " +import select, os, sys +fd = os.open(sys.argv[1], os.O_RDONLY) +os.read(fd, 4096) +p = select.poll() +p.register(fd, select.POLLPRI | select.POLLERR) +print('poll ready', flush=True) +events = p.poll(int(sys.argv[2])) +os.close(fd) +print('event' if events else 'no-event') +" "$@" +} + +# bats test_tags=ci:parallel +@test "podman commit --pause default" { + type -p python3 &>/dev/null || skip "python3 needed for cgroup poll(2)" + + cname="c-$(safename)" + run_podman run -d --name $cname $IMAGE sleep infinity + + run_podman inspect --format '{{.State.Pid}}' $cname + local pid="$output" + + local cgpath + cgpath=$(< /proc/$pid/cgroup) + cgpath=${cgpath#*::} + local events_file="/sys/fs/cgroup${cgpath}/cgroup.events" + if [ ! -f "$events_file" ]; then + skip "cannot find cgroup.events for container" + fi + + local watch_log="${PODMAN_TMPDIR}/cgroup-watch.log" + + # Start a background watcher before commit. + _cgroup_watch "$events_file" 10000 > "$watch_log" & + local watcher_pid=$! + # Wait for the watcher to be ready before triggering commit. + retries=50 + while ! grep -q "poll ready" "$watch_log" && [ $retries -gt 0 ]; do + sleep 0.1 + retries=$((retries - 1)) + done + + # Commit without explicit --pause flag: default is true, so the + # cgroup must receive a freeze event. + local imgname="i-$(safename)" + run_podman commit -q $cname $imgname + wait $watcher_pid || true + assert "$(tail -1 $watch_log)" = "event" \ + "commit should pause (freeze) the container by default" + + # Now test --pause=false: no freeze event expected. + _cgroup_watch "$events_file" 3000 > "$watch_log" & + watcher_pid=$! + retries=50 + while ! grep -q "poll ready" "$watch_log" && [ $retries -gt 0 ]; do + sleep 0.1 + retries=$((retries - 1)) + done + + run_podman commit -q --pause=false $cname "${imgname}-nopause" + wait $watcher_pid || true + assert "$(tail -1 $watch_log)" = "no-event" \ + "commit --pause=false should not freeze the container" + + # Clean up + run_podman rm -f -t0 $cname + run_podman rmi $imgname "${imgname}-nopause" +} + +# vim: filetype=sh