Fix timezone.zone_compare when /etc/localtime is missing on Debian (3006.x)

changelog/69803.fixed.md

@@ -0,0 +1 @@
+Fix ``timezone.system`` state on Debian/systemd hosts where ``/etc/localtime`` is missing but ``timedatectl`` reports a valid zone. ``timezone.zone_compare`` now defers to ``timezone.get_zone`` first, so a missing ``/etc/localtime`` no longer raises ``CommandExecutionError`` when the running timezone can be determined. Backport of the fix already shipped in 3007.x/3008.x for #65719.

salt/modules/timezone.py

@@ -50,7 +50,7 @@ def _timedatectl():
     """
     get the output of timedatectl
     """
-    ret = __salt__["cmd.run_all"](["timedatectl"], python_shell=False)
+    ret = __salt__["cmd.run_all"]("timedatectl", python_shell=False)
 
     if ret["retcode"] != 0:
         msg = "timedatectl failed: {}".format(ret["stderr"])
@@ -193,20 +193,28 @@ def get_zone():
         )
 
     else:
-        if __grains__["os"].lower() == "centos":
-            return _get_zone_etc_localtime()
         os_family = __grains__["os_family"]
-        for family in ("RedHat", "Suse"):
-            if family in os_family:
-                return _get_zone_sysconfig()
-        for family in ("Debian", "Gentoo"):
-            if family in os_family:
-                return _get_zone_etc_timezone()
-        if os_family in ("FreeBSD", "OpenBSD", "NetBSD", "NILinuxRT", "Slackware"):
+        if os_family in (
+            "RedHat",
+            "Suse",
+        ):
+            return _get_zone_sysconfig()
+        if os_family in (
+            "Debian",
+            "Gentoo",
+        ):
+            return _get_zone_etc_timezone()
+        if os_family in (
+            "FreeBSD",
+            "OpenBSD",
+            "NetBSD",
+            "NILinuxRT",
+            "Slackware",
+        ):
             return _get_zone_etc_localtime()
-        elif "Solaris" in os_family:
+        if os_family in ("Solaris",):
             return _get_zone_solaris()
-        elif "AIX" in os_family:
+        if os_family in ("AIX",):
             return _get_zone_aix()
     raise CommandExecutionError("Unable to get timezone")
 
@@ -322,9 +330,8 @@ def set_zone(timezone):
 
 def zone_compare(timezone):
     """
-    Compares the given timezone name with the system timezone name.
-    Checks the hash sum between the given timezone, and the one set in
-    /etc/localtime. Returns True if names and hash sums match, and False if not.
+    Compares the given timezone name with the system timezone name determined
+    by timezone.get_zone. Returns True if names match, and False if not.
     Mostly useful for running state checks.
 
     .. versionchanged:: 2016.3.0
@@ -345,28 +352,24 @@ def zone_compare(timezone):
 
         salt '*' timezone.zone_compare 'America/Denver'
     """
-    if "Solaris" in __grains__["os_family"] or "AIX" in __grains__["os_family"]:
-        return timezone == get_zone()
-
-    if "Arch" in __grains__["os_family"] or "FreeBSD" in __grains__["os_family"]:
-        if not os.path.isfile(_get_localtime_path()):
-            return timezone == get_zone()
-
-    tzfile = _get_localtime_path()
-    zonepath = _get_zone_file(timezone)
     try:
-        return filecmp.cmp(tzfile, zonepath, shallow=False)
-    except OSError as exc:
-        problematic_file = exc.filename
-        if problematic_file == zonepath:
-            raise SaltInvocationError(f'Can\'t find a local timezone "{timezone}"')
-        elif problematic_file == tzfile:
-            raise CommandExecutionError(
-                "Failed to read {} to determine current timezone: {}".format(
-                    tzfile, exc.strerror
+        return timezone == get_zone()
+    except CommandExecutionError:
+        tzfile = _get_localtime_path()
+        zonepath = _get_zone_file(timezone)
+        try:
+            return filecmp.cmp(tzfile, zonepath, shallow=False)
+        except OSError as exc:
+            problematic_file = exc.filename
+            if problematic_file == zonepath:
+                raise SaltInvocationError(f'Can\'t find a local timezone "{timezone}"')
+            elif problematic_file == tzfile:
+                raise CommandExecutionError(
+                    "Failed to read {} to determine current timezone: {}".format(
+                        tzfile, exc.strerror
+                    )
                 )
-            )
-        raise
+            raise
 
 
 def _get_localtime_path():

tests/pytests/unit/modules/test_timezone.py

@@ -22,6 +22,7 @@ def configure_loader_modules():
             "__salt__": {
                 "file.sed": MagicMock(),
                 "cmd.run": MagicMock(),
+                "cmd.run_all": MagicMock(),
                 "cmd.retcode": MagicMock(return_value=0),
             },
         }
@@ -102,6 +103,24 @@ def test_missing_localtime():
             pytest.raises(CommandExecutionError, timezone.zone_compare, "foo")
 
 
+def test_zone_compare_missing_localtime_with_timedatectl():
+    """
+    Regression test for #69803.
+
+    On systemd-managed hosts (e.g. Debian 12) ``/etc/localtime`` may be
+    absent while ``timedatectl`` still reports a valid zone. In that case
+    ``zone_compare`` must trust ``get_zone`` rather than failing on the
+    missing ``/etc/localtime`` file.
+    """
+    mock_get_zone = MagicMock(return_value="Europe/Zurich")
+    with patch.object(timezone, "get_zone", mock_get_zone):
+        with patch.dict(timezone.__grains__, {"os_family": "Debian"}):
+            with patch(GET_LOCALTIME_PATH, lambda: "/nonexistent/localtime"):
+                assert timezone.zone_compare("Europe/Zurich") is True
+                assert timezone.zone_compare("America/Denver") is False
+                mock_get_zone.assert_called()
+
+
 def create_tempfile_with_contents(contents, tempfiles=None):
     temp = NamedTemporaryFile(delete=False)
     temp.write(salt.utils.stringutils.to_bytes(contents))
@@ -110,28 +129,14 @@ def create_tempfile_with_contents(contents, tempfiles=None):
     return temp
 
 
-def test_get_zone_centos():
-    """
-    Test CentOS is recognized
-    :return:
-    """
-    with patch("salt.utils.path.which", MagicMock(return_value=False)):
-        with patch.dict(timezone.__grains__, {"os": "centos"}):
-            with patch(
-                "salt.modules.timezone._get_zone_etc_localtime",
-                MagicMock(return_value=TEST_TZ),
-            ):
-                assert timezone.get_zone() == TEST_TZ
-
-
 def test_get_zone_os_family_rh_suse():
     """
     Test RedHat and Suse are recognized
     :return:
     """
     for osfamily in ["RedHat", "Suse"]:
         with patch("salt.utils.path.which", MagicMock(return_value=False)):
-            with patch.dict(timezone.__grains__, {"os_family": [osfamily]}):
+            with patch.dict(timezone.__grains__, {"os_family": osfamily}):
                 with patch(
                     "salt.modules.timezone._get_zone_sysconfig",
                     MagicMock(return_value=TEST_TZ),
@@ -146,7 +151,7 @@ def test_get_zone_os_family_debian_gentoo():
     """
     for osfamily in ["Debian", "Gentoo"]:
         with patch("salt.utils.path.which", MagicMock(return_value=False)):
-            with patch.dict(timezone.__grains__, {"os_family": [osfamily]}):
+            with patch.dict(timezone.__grains__, {"os_family": osfamily}):
                 with patch(
                     "salt.modules.timezone._get_zone_etc_timezone",
                     MagicMock(return_value=TEST_TZ),
@@ -175,7 +180,7 @@ def test_get_zone_os_family_slowlaris():
     :return:
     """
     with patch("salt.utils.path.which", MagicMock(return_value=False)):
-        with patch.dict(timezone.__grains__, {"os_family": ["Solaris"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Solaris"}):
             with patch(
                 "salt.modules.timezone._get_zone_solaris",
                 MagicMock(return_value=TEST_TZ),
@@ -189,7 +194,7 @@ def test_get_zone_os_family_aix():
     :return:
     """
     with patch("salt.utils.path.which", MagicMock(return_value=False)):
-        with patch.dict(timezone.__grains__, {"os_family": ["AIX"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "AIX"}):
             with patch(
                 "salt.modules.timezone._get_zone_aix",
                 MagicMock(return_value=TEST_TZ),
@@ -203,7 +208,7 @@ def test_set_zone_os_family_nilinuxrt(patch_os):
     Test zone set on NILinuxRT
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["NILinuxRT"]}), patch.dict(
+    with patch.dict(timezone.__grains__, {"os_family": "NILinuxRT"}), patch.dict(
         timezone.__grains__, {"lsb_distrib_id": "nilrt"}
     ):
         assert timezone.set_zone(TEST_TZ)
@@ -226,7 +231,7 @@ def test_set_zone_redhat(patch_os):
     Test zone set on RH series
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["RedHat"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "RedHat"}):
         assert timezone.set_zone(TEST_TZ)
         name, args, kwargs = timezone.__salt__["file.sed"].mock_calls[0]
         assert args == ("/etc/sysconfig/clock", "^ZONE=.*", 'ZONE="UTC"')
@@ -238,7 +243,7 @@ def test_set_zone_suse(patch_os):
     Test zone set on SUSE series
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["Suse"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Suse"}):
         assert timezone.set_zone(TEST_TZ)
         name, args, kwargs = timezone.__salt__["file.sed"].mock_calls[0]
         assert args == ("/etc/sysconfig/clock", "^TIMEZONE=.*", 'TIMEZONE="UTC"')
@@ -250,7 +255,7 @@ def test_set_zone_gentoo(patch_os):
     Test zone set on Gentoo series
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["Gentoo"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Gentoo"}):
         with patch("salt.utils.files.fopen", mock_open()) as m_open:
             assert timezone.set_zone(TEST_TZ)
             fh_ = m_open.filehandles["/etc/timezone"][0]
@@ -264,7 +269,7 @@ def test_set_zone_debian(patch_os):
     Test zone set on Debian series
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["Debian"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Debian"}):
         with patch("salt.utils.files.fopen", mock_open()) as m_open:
             assert timezone.set_zone(TEST_TZ)
             fh_ = m_open.filehandles["/etc/timezone"][0]
@@ -313,7 +318,7 @@ def test_get_hwclock_redhat(patch_os):
     Test get hwclock on RedHat
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["RedHat"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "RedHat"}):
         timezone.get_hwclock()
         name, args, kwarg = timezone.__salt__["cmd.run"].mock_calls[0]
         assert args == (["tail", "-n", "1", "/etc/adjtime"],)
@@ -327,7 +332,7 @@ def _test_get_hwclock_debian(
     Test get hwclock on Debian
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["Debian"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Debian"}):
         timezone.get_hwclock()
         name, args, kwarg = timezone.__salt__["cmd.run"].mock_calls[0]
         assert args == (["tail", "-n", "1", "/etc/adjtime"],)
@@ -341,7 +346,7 @@ def test_get_hwclock_solaris(patch_os):
     :return:
     """
     # Incomplete
-    with patch.dict(timezone.__grains__, {"os_family": ["Solaris"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Solaris"}):
         assert timezone.get_hwclock() == "UTC"
         with patch("salt.utils.files.fopen", mock_open()):
             assert timezone.get_hwclock() == "localtime"
@@ -357,7 +362,7 @@ def test_get_hwclock_aix(patch_os):
     hwclock = "localtime"
     if not os.path.isfile("/etc/environment"):
         hwclock = "UTC"
-    with patch.dict(timezone.__grains__, {"os_family": ["AIX"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "AIX"}):
         assert timezone.get_hwclock() == hwclock
 
 
@@ -367,7 +372,7 @@ def test_get_hwclock_slackware_with_adjtime(patch_os):
     Test get hwclock on Slackware with /etc/adjtime present
     :return:
     """
-    with patch.dict(timezone.__grains__, {"os_family": ["Slackware"]}):
+    with patch.dict(timezone.__grains__, {"os_family": "Slackware"}):
         timezone.get_hwclock()
         name, args, kwarg = timezone.__salt__["cmd.run"].mock_calls[0]
         assert args == (["tail", "-n", "1", "/etc/adjtime"],)
@@ -384,7 +389,7 @@ def test_get_hwclock_slackware_without_adjtime():
         with patch("os.path.exists", MagicMock(return_value=False)):
             with patch("os.unlink", MagicMock()):
                 with patch("os.symlink", MagicMock()):
-                    with patch.dict(timezone.__grains__, {"os_family": ["Slackware"]}):
+                    with patch.dict(timezone.__grains__, {"os_family": "Slackware"}):
                         with patch(
                             "salt.utils.files.fopen", mock_open(read_data="UTC")
                         ):
@@ -436,7 +441,7 @@ def test_set_hwclock_solaris(patch_os):
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
         with patch.dict(
-            timezone.__grains__, {"os_family": ["Solaris"], "cpuarch": "x86"}
+            timezone.__grains__, {"os_family": "Solaris", "cpuarch": "x86"}
         ):
             with pytest.raises(SaltInvocationError):
                 assert timezone.set_hwclock("forty two")
@@ -455,7 +460,7 @@ def test_set_hwclock_arch(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["Arch"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Arch"}):
             assert timezone.set_hwclock("UTC")
             name, args, kwargs = timezone.__salt__["cmd.retcode"].mock_calls[0]
             assert args == (["timezonectl", "set-local-rtc", "false"],)
@@ -471,7 +476,7 @@ def test_set_hwclock_redhat(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["RedHat"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "RedHat"}):
             assert timezone.set_hwclock("UTC")
             name, args, kwargs = timezone.__salt__["file.sed"].mock_calls[0]
             assert args == ("/etc/sysconfig/clock", "^ZONE=.*", 'ZONE="TEST_TIMEZONE"')
@@ -486,7 +491,7 @@ def test_set_hwclock_suse(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["Suse"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Suse"}):
             assert timezone.set_hwclock("UTC")
             name, args, kwargs = timezone.__salt__["file.sed"].mock_calls[0]
             assert args == (
@@ -505,7 +510,7 @@ def test_set_hwclock_debian(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["Debian"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Debian"}):
             assert timezone.set_hwclock("UTC")
             name, args, kwargs = timezone.__salt__["file.sed"].mock_calls[0]
             assert args == ("/etc/default/rcS", "^UTC=.*", "UTC=yes")
@@ -524,7 +529,7 @@ def test_set_hwclock_gentoo(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["Gentoo"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Gentoo"}):
             with pytest.raises(SaltInvocationError):
                 timezone.set_hwclock("forty two")
 
@@ -546,7 +551,7 @@ def test_set_hwclock_slackware(patch_os):
     with patch(
         "salt.modules.timezone.get_zone", MagicMock(return_value="TEST_TIMEZONE")
     ):
-        with patch.dict(timezone.__grains__, {"os_family": ["Slackware"]}):
+        with patch.dict(timezone.__grains__, {"os_family": "Slackware"}):
             with pytest.raises(SaltInvocationError):
                 timezone.set_hwclock("forty two")