diff --git a/changelog/69803.fixed.md b/changelog/69803.fixed.md new file mode 100644 index 00000000000..7eb4c75ad43 --- /dev/null +++ b/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. diff --git a/salt/modules/timezone.py b/salt/modules/timezone.py index 115ec06eeef..9974b8e18f5 100644 --- a/salt/modules/timezone.py +++ b/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(): diff --git a/tests/pytests/unit/modules/test_timezone.py b/tests/pytests/unit/modules/test_timezone.py index fc9b9039d72..f7eeee227dc 100644 --- a/tests/pytests/unit/modules/test_timezone.py +++ b/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,20 +129,6 @@ 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 @@ -131,7 +136,7 @@ def test_get_zone_os_family_rh_suse(): """ 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")