From b431f06e64ec9eb35e34e1facb6c66654ff5e3fc Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 30 Apr 2025 18:53:07 +0200 Subject: [PATCH 1/4] pkg/machine: do not add broken localtime symlink The timezone might be empty so the zoneinfo link would then be invalid. Signed-off-by: Paul Holzinger --- pkg/machine/ignition/ignition.go | 44 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/pkg/machine/ignition/ignition.go b/pkg/machine/ignition/ignition.go index d6206ff658..cf18e2aa98 100644 --- a/pkg/machine/ignition/ignition.go +++ b/pkg/machine/ignition/ignition.go @@ -142,10 +142,8 @@ func (ign *DynamicIgnition) GenerateIgnitionConfig() error { // Add or set the time zone for the machine if len(ign.TimeZone) > 0 { - var ( - err error - tz string - ) + var err error + tz := ign.TimeZone // local means the same as the host // look up where it is pointing to on the host if ign.TimeZone == "local" { @@ -153,25 +151,29 @@ func (ign *DynamicIgnition) GenerateIgnitionConfig() error { if err != nil { return fmt.Errorf("error getting local timezone: %q", err) } + } + // getLocalTimeZone() can return empty string, do not add broken symlink in that case + // coreos will default to UTC + if tz == "" { + logrus.Info("Unable to determine local timezone, machine will default to UTC") } else { - tz = ign.TimeZone + tzLink := Link{ + Node: Node{ + Group: GetNodeGrp("root"), + Path: "/etc/localtime", + Overwrite: BoolToPtr(false), + User: GetNodeUsr("root"), + }, + LinkEmbedded1: LinkEmbedded1{ + Hard: BoolToPtr(false), + // We always want this value in unix form (/path/to/something) because this is being + // set in the machine OS (always Linux). However, filepath.join on windows will use a "\\" + // separator; therefore we use ToSlash to convert the path to unix style + Target: filepath.ToSlash(filepath.Join("/usr/share/zoneinfo", tz)), + }, + } + ignStorage.Links = append(ignStorage.Links, tzLink) } - tzLink := Link{ - Node: Node{ - Group: GetNodeGrp("root"), - Path: "/etc/localtime", - Overwrite: BoolToPtr(false), - User: GetNodeUsr("root"), - }, - LinkEmbedded1: LinkEmbedded1{ - Hard: BoolToPtr(false), - // We always want this value in unix form (/path/to/something) because this is being - // set in the machine OS (always Linux). However, filepath.join on windows will use a "\\" - // separator; therefore we use ToSlash to convert the path to unix style - Target: filepath.ToSlash(filepath.Join("/usr/share/zoneinfo", tz)), - }, - } - ignStorage.Links = append(ignStorage.Links, tzLink) } // This service gets environment variables that are provided From 193d7b82024162953a9bbdcc86fdfb5de2fef677 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 30 Apr 2025 19:28:48 +0200 Subject: [PATCH 2/4] pkg/machine: properly setup zoneinfo symlink If you run timedatectl inside it will not show the correct timezone, it seems systemd really wants a relative link which is also documented by coreos[1]. Also we can just use path.Join() directly and don't have to convert the path again on windows. [1] https://docs.fedoraproject.org/en-US/fedora-coreos/time-zone/#_setting_the_time_zone_via_ignition Signed-off-by: Paul Holzinger --- pkg/machine/e2e/init_test.go | 6 ++++++ pkg/machine/ignition/ignition.go | 9 +++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index 12b9f4a2c0..0e6c427db6 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -280,6 +280,12 @@ var _ = Describe("podman machine init", func() { Expect(err).ToNot(HaveOccurred()) Expect(timezoneSession).To(Exit(0)) Expect(timezoneSession.outputToString()).To(ContainSubstring("HST")) + + sshTimezone = sshMachine{} + timezoneSession, err = mb.setName(name).setCmd(sshTimezone.withSSHCommand([]string{"timedatectl show --property Timezone"})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(timezoneSession).To(Exit(0)) + Expect(timezoneSession.outputToString()).To(ContainSubstring("Pacific/Honolulu")) }) It("machine init with volume", func() { diff --git a/pkg/machine/ignition/ignition.go b/pkg/machine/ignition/ignition.go index cf18e2aa98..d8a0eabee3 100644 --- a/pkg/machine/ignition/ignition.go +++ b/pkg/machine/ignition/ignition.go @@ -166,10 +166,11 @@ func (ign *DynamicIgnition) GenerateIgnitionConfig() error { }, LinkEmbedded1: LinkEmbedded1{ Hard: BoolToPtr(false), - // We always want this value in unix form (/path/to/something) because this is being - // set in the machine OS (always Linux). However, filepath.join on windows will use a "\\" - // separator; therefore we use ToSlash to convert the path to unix style - Target: filepath.ToSlash(filepath.Join("/usr/share/zoneinfo", tz)), + // We always want this value in unix form (../usr/share/zoneinfo) because this is being + // set in the machine OS (always Linux) and systemd needs the relative symlink. However, + // filepath.join on windows will use a "\\" separator so use path.Join() which always + // uses the slash. + Target: path.Join("../usr/share/zoneinfo", tz), }, } ignStorage.Links = append(ignStorage.Links, tzLink) From a90fad3fc889a4e31a4bc26f8f6924e9f7d574a3 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 30 Apr 2025 19:06:47 +0200 Subject: [PATCH 3/4] pkg/machine: rework getLocalTimeZone on linux Get the timezone off the localtime symlink like systemd does it. It is more efficient then fork/exec another command for it that may or may not exits and the /etc/timezone files doesn't exist on most distros so that is not a great fallback. Signed-off-by: Paul Holzinger --- pkg/machine/ignition/ignition_linux.go | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/machine/ignition/ignition_linux.go b/pkg/machine/ignition/ignition_linux.go index 8ca303cbdb..c1fcab116f 100644 --- a/pkg/machine/ignition/ignition_linux.go +++ b/pkg/machine/ignition/ignition_linux.go @@ -1,29 +1,29 @@ package ignition import ( + "errors" "os" - "os/exec" + "path/filepath" "strings" - - "github.com/sirupsen/logrus" ) func getLocalTimeZone() (string, error) { - trimTzFunc := func(s string) string { - return strings.TrimPrefix(strings.TrimSuffix(s, "\n"), "Timezone=") + path, err := filepath.EvalSymlinks("/etc/localtime") + if err != nil { + // of the path does not exist, ignore it as the code default to UTC then + if errors.Is(err, os.ErrNotExist) { + return "", nil + } + return "", err } - // perform a variety of ways to see if we can determine the tz - output, err := exec.Command("timedatectl", "show", "--property=Timezone").Output() - if err == nil { - return trimTzFunc(string(output)), nil + // Allow using TZDIR per: + // https://sourceware.org/git/?p=glibc.git;a=blob;f=time/tzfile.c;h=8a923d0cccc927a106dc3e3c641be310893bab4e;hb=HEAD#l149 + zoneinfo := os.Getenv("TZDIR") + if zoneinfo == "" { + // default zoneinfo location + zoneinfo = "/usr/share/zoneinfo" } - logrus.Debugf("Timedatectl show --property=Timezone failed: %s", err) - output, err = os.ReadFile("/etc/timezone") - if err == nil { - return trimTzFunc(string(output)), nil - } - logrus.Debugf("unable to read /etc/timezone, falling back to empty timezone: %s", err) - // if we cannot determine the tz, return empty string - return "", nil + // Trim of the TZDIR part to extract the actual timezone name + return strings.TrimPrefix(path, filepath.Clean(zoneinfo)+"/"), nil } From ac6080bea9debebb9d3b4a40d2130864a9809daf Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 2 May 2025 11:55:27 +0200 Subject: [PATCH 4/4] pkg/machinie: use TZ env for reading local timezone The TZ var can be commonly used to overwrite the timezone so we should honour that one as well. Signed-off-by: Paul Holzinger --- pkg/machine/ignition/ignition.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/machine/ignition/ignition.go b/pkg/machine/ignition/ignition.go index d8a0eabee3..3617c2e789 100644 --- a/pkg/machine/ignition/ignition.go +++ b/pkg/machine/ignition/ignition.go @@ -147,9 +147,13 @@ func (ign *DynamicIgnition) GenerateIgnitionConfig() error { // local means the same as the host // look up where it is pointing to on the host if ign.TimeZone == "local" { - tz, err = getLocalTimeZone() - if err != nil { - return fmt.Errorf("error getting local timezone: %q", err) + if env, ok := os.LookupEnv("TZ"); ok { + tz = env + } else { + tz, err = getLocalTimeZone() + if err != nil { + return fmt.Errorf("error getting local timezone: %q", err) + } } } // getLocalTimeZone() can return empty string, do not add broken symlink in that case