From abc4f7bf15b1bb5c8059f19e9ff5929109c7f852 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Wed, 31 May 2023 16:07:43 -0600 Subject: [PATCH 1/2] systests: fixes for coping with extra systemd image We _usually_ have only one image in store, $IMAGE, but it's perfectly fine to also have $SYSTEMD_IMAGE also. Fix a few tests so they can handle that condition. And, cleanup: - remove a no-longer-useful test ("podman load NEWNAME", functionality that was removed 2+ years ago in #8877) - reorder some tests in the image-mount test, to make them safer and easier to understand - use no-such-image, not no-such-container, in image-mount test. Computer don't care, but this human felt confused for a sec. Signed-off-by: Ed Santiago --- test/system/060-mount.bats | 19 +++++++++++-------- test/system/120-load.bats | 10 ---------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index 4498e675fb..dd8aa3e97b 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -50,10 +50,6 @@ load helpers run_podman image mount $IMAGE mount_path="$output" - # Make sure that `mount -a` prints a table - run_podman image mount -a - is "$output" "$IMAGE .*$mount_path" - test -d $mount_path # Image is custom-built and has a file containing the YMD tag. Check it. @@ -66,16 +62,23 @@ load helpers run_podman image mount is "$output" "$IMAGE *$mount_path" "podman image mount with no args" - # Clean up: -f since we mounted it twice + # Clean up, and make sure nothing is mounted any more run_podman image umount -f $IMAGE is "$output" "$iid" "podman image umount: image ID of what was umounted" run_podman image umount $IMAGE is "$output" "" "podman image umount: does not re-umount" - run_podman 125 image umount no-such-container - is "$output" "Error: no-such-container: image not known" \ - "error message from image umount no-such-container" + run_podman 125 image umount no-such-image + is "$output" "Error: no-such-image: image not known" \ + "error message from image umount no-such-image" + + # Tests for mount -a. This may mount more than one image! (E.g. systemd) + run_podman image mount -a + is "$output" "$IMAGE .*$mount_path" + + run_podman image umount -a + assert "$output" =~ "$iid" "Test image is unmounted" run_podman image mount is "$output" "" "podman image mount, no args, after umount" diff --git a/test/system/120-load.bats b/test/system/120-load.bats index 35a6eae421..904b5da49d 100644 --- a/test/system/120-load.bats +++ b/test/system/120-load.bats @@ -63,16 +63,6 @@ verify_iid_and_name() { run_podman images $fqin --format '{{.Repository}}:{{.Tag}}' is "${lines[0]}" "$fqin" "image preserves name across save/load" - # Load with a new tag - local new_name=x1$(random_string 14 | tr A-Z a-z) - local new_tag=t1$(random_string 6 | tr A-Z a-z) - run_podman rmi $fqin - - run_podman load -i $archive - run_podman images --format '{{.Repository}}:{{.Tag}}' --sort tag - is "${lines[0]}" "$IMAGE" "image is preserved" - is "${lines[1]}" "$fqin" "image is reloaded with old fqin" - # Clean up run_podman rmi $fqin } From 8e90244a6e415ce2b0fb72047beaf71aa95001b8 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 31 May 2023 16:12:38 +0200 Subject: [PATCH 2/2] libpod: fix timezone handling The current way of bind mounting the host timezone file has problems. Because /etc/localtime in the image may exist and is a symlink under /usr/share/zoneinfo it will overwrite the targetfile. That confuses timezone parses especially java where this approach does not work at all. So we end up with an link which does not reflect the actual truth. The better way is to just change the symlink in the image like it is done on the host. However because not all images ship tzdata we cannot rely on that either. So now we do both, when tzdata is installed then use the symlink and if not we keep the current way of copying the host timezone file in the container to /etc/localtime. Also note that we need to rebuild the systemd image to include tzdata in order to test this as our images do not contain the tzdata by default. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2149876 Signed-off-by: Paul Holzinger --- libpod/container_internal.go | 43 +++++++++++++++++++++++++++++ libpod/container_internal_common.go | 32 --------------------- libpod/options.go | 12 +++----- test/e2e/create_test.go | 8 ++++-- test/e2e/run_test.go | 15 ++-------- test/system/030-run.bats | 8 ++++++ test/system/build-systemd-image | 5 ++-- test/system/helpers.bash | 2 +- 8 files changed, 68 insertions(+), 57 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index fb78553a44..e23a174a14 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "strconv" @@ -1669,6 +1670,48 @@ func (c *Container) mountStorage() (_ string, deferredErr error) { } } + tz := c.Timezone() + if tz != "" { + timezonePath := filepath.Join("/usr/share/zoneinfo", tz) + if tz == "local" { + timezonePath, err = filepath.EvalSymlinks("/etc/localtime") + if err != nil { + return "", fmt.Errorf("finding local timezone for container %s: %w", c.ID(), err) + } + } + // make sure to remove any existing locatime file in the container to not create invalid links + err = unix.Unlinkat(etcInTheContainerFd, "localtime", 0) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("removing /etc/localtime: %w", err) + } + + hostPath, err := securejoin.SecureJoin(mountPoint, timezonePath) + if err != nil { + return "", fmt.Errorf("resolve zoneinfo path in the container: %w", err) + } + + _, err = os.Stat(hostPath) + if err != nil { + // file does not exists which means tzdata is not installed in the container, just create /etc/locatime which a copy from the host + logrus.Debugf("Timezone %s does not exists in the container, create our own copy from the host", timezonePath) + localtimePath, err := c.copyTimezoneFile(timezonePath) + if err != nil { + return "", fmt.Errorf("setting timezone for container %s: %w", c.ID(), err) + } + if c.state.BindMounts == nil { + c.state.BindMounts = make(map[string]string) + } + c.state.BindMounts["/etc/localtime"] = localtimePath + } else { + // file exists lets just symlink according to localtime(5) + logrus.Debugf("Create locatime symlink for %s", timezonePath) + err = unix.Symlinkat(".."+timezonePath, etcInTheContainerFd, "localtime") + if err != nil { + return "", fmt.Errorf("creating /etc/localtime symlink: %w", err) + } + } + } + // Request a mount of all named volumes for _, v := range c.config.NamedVolumes { vol, err := c.mountNamedVolume(v, mountPoint) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 7d4641fb4f..fadff0b4e5 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -1949,38 +1949,6 @@ func (c *Container) makeBindMounts() error { } } - // Make /etc/localtime - ctrTimezone := c.Timezone() - if ctrTimezone != "" { - // validate the format of the timezone specified if it's not "local" - if ctrTimezone != "local" { - _, err = time.LoadLocation(ctrTimezone) - if err != nil { - return fmt.Errorf("finding timezone for container %s: %w", c.ID(), err) - } - } - if _, ok := c.state.BindMounts["/etc/localtime"]; !ok { - var zonePath string - if ctrTimezone == "local" { - zonePath, err = filepath.EvalSymlinks("/etc/localtime") - if err != nil { - return fmt.Errorf("finding local timezone for container %s: %w", c.ID(), err) - } - } else { - zone := filepath.Join("/usr/share/zoneinfo", ctrTimezone) - zonePath, err = filepath.EvalSymlinks(zone) - if err != nil { - return fmt.Errorf("setting timezone for container %s: %w", c.ID(), err) - } - } - localtimePath, err := c.copyTimezoneFile(zonePath) - if err != nil { - return fmt.Errorf("setting timezone for container %s: %w", c.ID(), err) - } - c.state.BindMounts["/etc/localtime"] = localtimePath - } - } - _, hasRunContainerenv := c.state.BindMounts["/run/.containerenv"] if !hasRunContainerenv { Loop: diff --git a/libpod/options.go b/libpod/options.go index 54cc5b0ae6..d0e71a350f 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "syscall" + "time" "github.com/containers/buildah/pkg/parse" nettypes "github.com/containers/common/libnetwork/types" @@ -1814,15 +1815,10 @@ func WithTimezone(path string) CtrCreateOption { return define.ErrCtrFinalized } if path != "local" { - zone := filepath.Join("/usr/share/zoneinfo", path) - - file, err := os.Stat(zone) + // validate the format of the timezone specified if it's not "local" + _, err := time.LoadLocation(path) if err != nil { - return err - } - // We don't want to mount a timezone directory - if file.IsDir() { - return errors.New("invalid timezone: is a directory") + return fmt.Errorf("finding timezone: %w", err) } } diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index dfe6a06298..0af1e34216 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -461,11 +461,15 @@ var _ = Describe("Podman create", func() { It("podman create --tz", func() { session := podmanTest.Podman([]string{"create", "--tz", "foo", "--name", "bad", ALPINE, "date"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).To(Exit(125)) + Expect(session.ErrorToString()).To( + Equal("Error: running container create option: finding timezone: unknown time zone foo")) session = podmanTest.Podman([]string{"create", "--tz", "America", "--name", "dir", ALPINE, "date"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).To(Exit(125)) + Expect(session.ErrorToString()).To( + Equal("Error: running container create option: finding timezone: is a directory")) session = podmanTest.Podman([]string{"create", "--tz", "Pacific/Honolulu", "--name", "zone", ALPINE, "date"}) session.WaitWithDefaultTimeout() diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 071f46f847..073b419d7a 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1611,6 +1611,7 @@ USER mail`, BB) tzFile := filepath.Join(testDir, "tzfile.txt") file, err := os.Create(tzFile) Expect(err).ToNot(HaveOccurred()) + defer os.Remove(tzFile) _, err = file.WriteString("Hello") Expect(err).ToNot(HaveOccurred()) @@ -1620,18 +1621,8 @@ USER mail`, BB) session := podmanTest.Podman([]string{"run", "--tz", badTZFile, "--rm", ALPINE, "date"}) session.WaitWithDefaultTimeout() Expect(session).To(ExitWithError()) - Expect(session.ErrorToString()).To(ContainSubstring("finding timezone for container")) - - err = os.Remove(tzFile) - Expect(err).ToNot(HaveOccurred()) - - session = podmanTest.Podman([]string{"run", "--tz", "foo", "--rm", ALPINE, "date"}) - session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) - - session = podmanTest.Podman([]string{"run", "--tz", "America", "--rm", ALPINE, "date"}) - session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session.ErrorToString()).To( + Equal("Error: running container create option: finding timezone: time: invalid location name")) session = podmanTest.Podman([]string{"run", "--tz", "Pacific/Honolulu", "--rm", ALPINE, "date", "+'%H %Z'"}) session.WaitWithDefaultTimeout() diff --git a/test/system/030-run.bats b/test/system/030-run.bats index d46111e862..db73955a41 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -472,6 +472,14 @@ json-file | f is "$output" "$expect" "podman run with --tz=local, matches host" } +@test "podman run --tz with zoneinfo" { + # First make sure that zoneinfo is actually in the image otherwise the test is pointless + run_podman run --rm $SYSTEMD_IMAGE ls /usr/share/zoneinfo + + run_podman run --rm --tz Europe/Berlin $SYSTEMD_IMAGE readlink /etc/localtime + assert "$output" == "../usr/share/zoneinfo/Europe/Berlin" "localtime is linked correctly" +} + # run with --runtime should preserve the named runtime @test "podman run : full path to --runtime is preserved" { skip_if_remote "podman-remote does not support --runtime option" diff --git a/test/system/build-systemd-image b/test/system/build-systemd-image index 43bcfc5c62..724340a80f 100755 --- a/test/system/build-systemd-image +++ b/test/system/build-systemd-image @@ -31,10 +31,11 @@ cd $tmpdir echo $YMD >testimage-id cat >Containerfile <