diff options
-rw-r--r-- | cmd/podman/main.go | 5 | ||||
-rw-r--r-- | cmd/podman/root.go | 4 | ||||
-rw-r--r-- | contrib/tmpfile/podman.conf | 5 | ||||
-rw-r--r-- | libpod/container_inspect.go | 7 | ||||
-rw-r--r-- | libpod/define/container_inspect.go | 9 | ||||
-rw-r--r-- | libpod/oci_conmon_linux.go | 14 | ||||
-rw-r--r-- | pkg/api/handlers/compat/images.go | 7 | ||||
-rw-r--r-- | test/e2e/logs_test.go | 5 | ||||
-rw-r--r-- | test/system/030-run.bats | 70 | ||||
-rw-r--r-- | test/system/035-logs.bats | 9 | ||||
-rw-r--r-- | test/system/040-ps.bats | 39 | ||||
-rw-r--r-- | test/system/260-sdnotify.bats | 11 | ||||
-rw-r--r-- | test/system/helpers.bash | 41 |
13 files changed, 167 insertions, 59 deletions
diff --git a/cmd/podman/main.go b/cmd/podman/main.go index c3aaf84a8..f076d13f3 100644 --- a/cmd/podman/main.go +++ b/cmd/podman/main.go @@ -31,11 +31,6 @@ func main() { return } - // Hard code TMPDIR functions to use /var/tmp, if user did not override - if _, ok := os.LookupEnv("TMPDIR"); !ok { - os.Setenv("TMPDIR", "/var/tmp") - } - rootCmd = parseCommands() Execute() diff --git a/cmd/podman/root.go b/cmd/podman/root.go index 0830a62a5..1f613a4c5 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -178,6 +178,10 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { return err } } + // Hard code TMPDIR functions to use /var/tmp, if user did not override + if _, ok := os.LookupEnv("TMPDIR"); !ok { + os.Setenv("TMPDIR", "/var/tmp") + } if !registry.IsRemote() { if cmd.Flag("cpu-profile").Changed { diff --git a/contrib/tmpfile/podman.conf b/contrib/tmpfile/podman.conf index d769671e3..e7cad4066 100644 --- a/contrib/tmpfile/podman.conf +++ b/contrib/tmpfile/podman.conf @@ -1,4 +1,5 @@ # /tmp/podman-run-* directory can contain content for Podman containers that have run # for many days. This following line prevents systemd from removing this content. -x /tmp/podman-run-.* -d /run/podman 0700 root root +x /tmp/podman-run-* +D! /run/podman 0700 root root +D! /var/lib/cni/networks diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 2ce3e8e68..51474471b 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -8,6 +8,7 @@ import ( "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/libpod/driver" "github.com/containers/podman/v2/pkg/util" + units "github.com/docker/go-units" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/runtime-tools/validate" @@ -124,8 +125,6 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data) HostnamePath: hostnamePath, HostsPath: hostsPath, StaticDir: config.StaticDir, - LogPath: config.LogPath, - LogTag: config.LogTag, OCIRuntime: config.OCIRuntime, ConmonPidFile: config.ConmonPidFile, Name: config.Name, @@ -354,6 +353,10 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named logConfig := new(define.InspectLogConfig) logConfig.Type = c.config.LogDriver + logConfig.Path = c.config.LogPath + logConfig.Size = units.HumanSize(float64(c.config.LogSize)) + logConfig.Tag = c.config.LogTag + hostConfig.LogConfig = logConfig restartPolicy := new(define.InspectRestartPolicy) diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go index 775965477..c15bcedf2 100644 --- a/libpod/define/container_inspect.go +++ b/libpod/define/container_inspect.go @@ -82,10 +82,15 @@ type InspectRestartPolicy struct { } // InspectLogConfig holds information about a container's configured log driver -// and is presently unused. It is retained for Docker compatibility. type InspectLogConfig struct { Type string `json:"Type"` Config map[string]string `json:"Config"` //idk type, TODO + // Path specifies a path to the log file + Path string `json:"Path"` + // Tag specifies a custom log tag for the container + Tag string `json:"Tag"` + // Size specifies a maximum size of the container log + Size string `json:"Size"` } // InspectBlkioWeightDevice holds information about the relative weight @@ -620,8 +625,6 @@ type InspectContainerData struct { StaticDir string `json:"StaticDir"` OCIConfigPath string `json:"OCIConfigPath,omitempty"` OCIRuntime string `json:"OCIRuntime,omitempty"` - LogPath string `json:"LogPath"` - LogTag string `json:"LogTag"` ConmonPidFile string `json:"ConmonPidFile"` Name string `json:"Name"` RestartCount int32 `json:"RestartCount"` diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 307b9bc54..7e0a1d457 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1433,6 +1433,14 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec } if mustCreateCgroup { + // Usually rootless users are not allowed to configure cgroupfs. + // There are cases though, where it is allowed, e.g. if the cgroup + // is manually configured and chowned). Avoid detecting all + // such cases and simply use a lower log level. + logLevel := logrus.WarnLevel + if rootless.IsRootless() { + logLevel = logrus.InfoLevel + } // TODO: This should be a switch - we are not guaranteed that // there are only 2 valid cgroup managers cgroupParent := ctr.CgroupParent() @@ -1447,17 +1455,17 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec logrus.Infof("Running conmon under slice %s and unitName %s", realCgroupParent, unitName) if err := utils.RunUnderSystemdScope(cmd.Process.Pid, realCgroupParent, unitName); err != nil { - logrus.Warnf("Failed to add conmon to systemd sandbox cgroup: %v", err) + logrus.StandardLogger().Logf(logLevel, "Failed to add conmon to systemd sandbox cgroup: %v", err) } } else { cgroupPath := filepath.Join(ctr.config.CgroupParent, "conmon") control, err := cgroups.New(cgroupPath, &spec.LinuxResources{}) if err != nil { - logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) + logrus.StandardLogger().Logf(logLevel, "Failed to add conmon to cgroupfs sandbox cgroup: %v", err) } else if err := control.AddPid(cmd.Process.Pid); err != nil { // we need to remove this defer and delete the cgroup once conmon exits // maybe need a conmon monitor? - logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) + logrus.StandardLogger().Logf(logLevel, "Failed to add conmon to cgroupfs sandbox cgroup: %v", err) } } } diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index a51dd8ed3..dc72500e4 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -22,6 +22,7 @@ import ( "github.com/gorilla/schema" "github.com/opencontainers/go-digest" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // mergeNameAndTagOrDigest creates an image reference as string from the @@ -386,6 +387,12 @@ func LoadImages(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "failed to create tempfile")) return } + defer func() { + err := os.Remove(f.Name()) + if err != nil { + logrus.Errorf("Failed to remove temporary file: %v.", err) + } + }() if err := SaveFromBody(f, r); err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "failed to write temporary file")) return diff --git a/test/e2e/logs_test.go b/test/e2e/logs_test.go index aae6d4f02..b370aeec1 100644 --- a/test/e2e/logs_test.go +++ b/test/e2e/logs_test.go @@ -332,6 +332,11 @@ var _ = Describe("Podman logs", func() { wait.WaitWithDefaultTimeout() Expect(wait).To(Exit(0)) + inspect := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.HostConfig.LogConfig.Size}}", cid}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).To(Exit(0)) + Expect(inspect.OutputToString()).To(Equal("10kB")) + results := podmanTest.Podman([]string{"logs", cid}) results.WaitWithDefaultTimeout() Expect(results).To(Exit(0)) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 3ee141f5f..29dc95dc3 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -401,7 +401,7 @@ json-file | f is "$output" "$driver" "podman inspect: driver" # If LogPath is non-null, check that it exists and has a valid log - run_podman inspect --format '{{.LogPath}}' myctr + run_podman inspect --format '{{.HostConfig.LogConfig.Path}}' myctr if [[ $do_check != '-' ]]; then is "$output" "/.*" "LogPath (driver=$driver)" if ! test -e "$output"; then @@ -415,13 +415,18 @@ json-file | f fi if [[ $driver != 'none' ]]; then - run_podman logs myctr - is "$output" "$msg" "check that podman logs works as expected" + if [[ $driver = 'journald' ]] && journald_unavailable; then + # Cannot perform check + : + else + run_podman logs myctr + is "$output" "$msg" "podman logs, with driver '$driver'" + fi else run_podman 125 logs myctr if ! is_remote; then is "$output" ".*this container is using the 'none' log driver, cannot read logs.*" \ - "podman logs does not work with none log driver" + "podman logs, with driver 'none', should fail with error" fi fi run_podman rm myctr @@ -437,14 +442,7 @@ json-file | f skip_if_remote "We cannot read journalctl over remote." # We can't use journald on RHEL as rootless, either: rhbz#1895105 - if is_rootless; then - run journalctl -n 1 - if [[ $status -ne 0 ]]; then - if [[ $output =~ permission ]]; then - skip "Cannot use rootless journald on this system" - fi - fi - fi + skip_if_journald_unavailable msg=$(random_string 20) pidfile="${PODMAN_TMPDIR}/$(random_string 20)" @@ -550,27 +548,33 @@ json-file | f } @test "Verify /run/.containerenv exist" { - run_podman run --rm $IMAGE ls -1 /run/.containerenv - is "$output" "/run/.containerenv" - - run_podman run --privileged --rm $IMAGE sh -c '. /run/.containerenv; echo $engine' - is "$output" ".*podman.*" "failed to identify engine" - - run_podman run --privileged --name "testcontainerenv" --rm $IMAGE sh -c '. /run/.containerenv; echo $name' - is "$output" ".*testcontainerenv.*" - - run_podman run --privileged --rm $IMAGE sh -c '. /run/.containerenv; echo $image' - is "$output" ".*$IMAGE.*" "failed to idenitfy image" - - run_podman run --privileged --rm $IMAGE sh -c '. /run/.containerenv; echo $rootless' - # FIXME: on some CI systems, 'run --privileged' emits a spurious - # warning line about dup devices. Ignore it. - remove_same_dev_warning - if is_rootless; then - is "$output" "1" - else - is "$output" "0" - fi + # Nonprivileged container: file exists, but must be empty + run_podman run --rm $IMAGE stat -c '%s' /run/.containerenv + is "$output" "0" "file size of /run/.containerenv, nonprivileged" + + # Prep work: get ID of image; make a cont. name; determine if we're rootless + run_podman inspect --format '{{.ID}}' $IMAGE + local iid="$output" + + random_cname=c$(random_string 15 | tr A-Z a-z) + local rootless=0 + if is_rootless; then + rootless=1 + fi + + run_podman run --privileged --rm --name $random_cname $IMAGE \ + sh -c '. /run/.containerenv; echo $engine; echo $name; echo $image; echo $id; echo $imageid; echo $rootless' + + # FIXME: on some CI systems, 'run --privileged' emits a spurious + # warning line about dup devices. Ignore it. + remove_same_dev_warning + + is "${lines[0]}" "podman-.*" 'containerenv : $engine' + is "${lines[1]}" "$random_cname" 'containerenv : $name' + is "${lines[2]}" "$IMAGE" 'containerenv : $image' + is "${lines[3]}" "[0-9a-f]\{64\}" 'containerenv : $id' + is "${lines[4]}" "$iid" 'containerenv : $imageid' + is "${lines[5]}" "$rootless" 'containerenv : $rootless' } @test "podman run with --net=host and --port prints warning" { diff --git a/test/system/035-logs.bats b/test/system/035-logs.bats index a081a7ce1..bac153b8e 100644 --- a/test/system/035-logs.bats +++ b/test/system/035-logs.bats @@ -55,14 +55,7 @@ ${cid[0]} d" "Sequential output from logs" @test "podman logs over journald" { # We can't use journald on RHEL as rootless: rhbz#1895105 - if is_rootless; then - run journalctl -n 1 - if [[ $status -ne 0 ]]; then - if [[ $output =~ permission ]]; then - skip "Cannot use rootless journald on this system" - fi - fi - fi + skip_if_journald_unavailable msg=$(random_string 20) diff --git a/test/system/040-ps.bats b/test/system/040-ps.bats index 1ed2779b2..0447122b1 100644 --- a/test/system/040-ps.bats +++ b/test/system/040-ps.bats @@ -82,4 +82,43 @@ load helpers run_podman rm -a } +@test "podman ps -a --storage" { + skip_if_remote "ps --storage does not work over remote" + + # Setup: ensure that we have no hidden storage containers + run_podman ps --storage -a + is "${#lines[@]}" "1" "setup check: no storage containers at start of test" + + # Force a buildah timeout; this leaves a buildah container behind + PODMAN_TIMEOUT=5 run_podman 124 build -t thiswillneverexist - <<EOF +FROM $IMAGE +RUN sleep 30 +EOF + + run_podman ps -a + is "${#lines[@]}" "1" "podman ps -a does not see buildah container" + + run_podman ps --storage -a + is "${#lines[@]}" "2" "podman ps -a --storage sees buildah container" + is "${lines[1]}" \ + "[0-9a-f]\{12\} \+$IMAGE *buildah .* seconds ago .* storage .* ${PODMAN_TEST_IMAGE_NAME}-working-container" \ + "podman ps --storage" + + cid="${lines[1]:0:12}" + + # 'rm -a' should be a NOP + run_podman rm -a + run_podman ps --storage -a + is "${#lines[@]}" "2" "podman ps -a --storage sees buildah container" + + # This is what deletes the container + # FIXME: why doesn't "podman rm --storage $cid" do anything? + run_podman rm -f "$cid" + + run_podman ps --storage -a + is "${#lines[@]}" "1" "storage container has been removed" +} + + + # vim: filetype=sh diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index c99ba4fa6..a5fa0f4e6 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -100,8 +100,17 @@ function _assert_mainpid_is_conmon() { run_podman logs sdnotify_conmon_c is "$output" "READY" "\$NOTIFY_SOCKET in container" + # The 'echo's help us debug failed runs run cat $_SOCAT_LOG - is "${lines[-1]}" "READY=1" "final output from sdnotify" + echo "socat log:" + echo "$output" + + # ARGH! 'READY=1' should always be the last output line. But sometimes, + # for reasons unknown, we get an extra MAINPID=xxx after READY=1 (#8718). + # Who knows if this is a systemd bug, or conmon, or what. I don't + # even know where to begin asking. So, to eliminate the test flakes, + # we look for READY=1 _anywhere_ in the output, not just the last line. + is "$output" ".*READY=1.*" "sdnotify sent READY=1" _assert_mainpid_is_conmon "${lines[0]}" diff --git a/test/system/helpers.bash b/test/system/helpers.bash index 6a7c6cc42..a4b89ec99 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -168,8 +168,11 @@ function run_podman() { if [ "$status" -eq 124 ]; then if expr "$output" : ".*timeout: sending" >/dev/null; then - echo "*** TIMED OUT ***" - false + # It's possible for a subtest to _want_ a timeout + if [[ "$expected_rc" != "124" ]]; then + echo "*** TIMED OUT ***" + false + fi fi fi @@ -259,6 +262,31 @@ function is_cgroupsv2() { test "$cgroup_type" = "cgroup2fs" } +# rhbz#1895105: rootless journald is unavailable except to users in +# certain magic groups; which our testuser account does not belong to +# (intentional: that is the RHEL default, so that's the setup we test). +function journald_unavailable() { + if ! is_rootless; then + # root must always have access to journal + return 1 + fi + + run journalctl -n 1 + if [[ $status -eq 0 ]]; then + return 1 + fi + + if [[ $output =~ permission ]]; then + return 0 + fi + + # This should never happen; if it does, it's likely that a subsequent + # test will fail. This output may help track that down. + echo "WEIRD: 'journalctl -n 1' failed with a non-permission error:" + echo "$output" + return 1 +} + ########################### # _add_label_if_missing # make sure skip messages include rootless/remote ########################### @@ -315,6 +343,15 @@ function skip_if_cgroupsv1() { fi } +################################## +# skip_if_journald_unavailable # rhbz#1895105: rootless journald permissions +################################## +function skip_if_journald_unavailable { + if journald_unavailable; then + skip "Cannot use rootless journald on this system" + fi +} + ######### # die # Abort with helpful message ######### |