[hostcfg] Fix timezone mismatch after image upgrade (#312)

Why I did it
To fix an issue where newly installed SONiC images default /etc/localtime to UTC even when the previous image
was configured with and used a different timezone, and config save was executed before the image update.
This causes a mismatch between the timezone defined in CONFIG_DB and the one defined in the Linux file /etc/localtime, because when the new image comes up with a new filesystem, the Linux file /etc/localtime is created with the default UTC, while the timezone value in CONFIG_DB (DEVICE_METADATA|localhost|timezone) is preserved from the previous image.

How I did it
In src/sonic-host-services/scripts/hostcfgd::DeviceMetaCfg:load, we already read the timezone from CONFIG_DB to save it for future events (DeviceMetaCfg:timezone_update observes changes in DEVICE_METADATA).
So at this point, when hostcfgd is up, we can simply update the Linux file /etc/localtime with the initial correct timezone value from CONFIG_DB by executing (only if it differs from the current /etc/localtime):
timedatectl set-timezone
(as is already done in timezone_update).
The same pattern exists in hostcfgd::DnsCfg:load.

How to verify it

Set a non-UTC timezone on the current image: config clock timezone Asia/Jerusalem
Run config save
Install a new image
Reboot and check that hostcfgd is up: systemctl status hostcfgd (~1m)
Verify that the timezone is identical in CONFIG_DB and /etc/localtime:
timedatectl
date
cat /etc/localtime
cat /etc/sonic/config_db.json | grep timezone
hget "DEVICE_METADATA|localhost" "timezone"

Author: benle7

scripts/hostcfgd

@@ -50,6 +50,8 @@ MKHOME_DIR_LIB_REG = r'.*pam_mkhomedir'
 ETC_PAMD_SSHD = "/etc/pam.d/sshd"
 ETC_PAMD_LOGIN = "/etc/pam.d/login"
 ETC_LOGIN_DEF = "/etc/login.defs"
+ETC_LOCALTIME = "/etc/localtime"
+ZONEINFO_DIR = "/usr/share/zoneinfo"
 
 # Linux login.def default values (password hardening disable)
 LINUX_DEFAULT_PASS_MAX_DAYS = 99999
@@ -1495,6 +1497,7 @@ class DeviceMetaCfg(object):
 
         # Load appropriate config
         self.timezone = dev_meta.get('localhost', {}).get('timezone')
+        self.apply_timezone_if_needed(self.timezone)
         self.syslog_with_osversion = dev_meta.get('localhost', {}).get('syslog_with_osversion')
 
     def hostname_update(self, data):
@@ -1527,34 +1530,50 @@ class DeviceMetaCfg(object):
                 return
             run_cmd(['sudo', 'monit', 'reload'])
 
-    def timezone_update(self, data):
+    def apply_timezone_if_needed(self, new_tz):
         """
-        Apply timezone handler.
+        Apply timezone if it differs from either:
+        - the cached DB value (self.timezone)
+        - the actual system timezone (/etc/localtime)
         Run the following command in Linux: timedatectl set-timezone <timezone>
+
         Args:
-            data: Read table's key's data.
+            new_tz: New timezone value from DB
         """
-        new_timezone = data.get('timezone')
-        syslog.syslog(syslog.LOG_DEBUG,
-                      f'DeviceMetaCfg: timezone update to {new_timezone}')
+        try:
+            syslog.syslog(syslog.LOG_DEBUG, f'DeviceMetaCfg: timezone update to {new_tz}')
+            if new_tz is None:
+                syslog.syslog(syslog.LOG_DEBUG, 'DeviceMetaCfg: Recieved empty timezone')
+                return
+            
+            system_timezone_realpath = os.path.realpath(ETC_LOCALTIME)
+            new_timezone_realpath = os.path.realpath(f'{ZONEINFO_DIR}/{new_tz}')
+            if new_tz == self.timezone and new_timezone_realpath == system_timezone_realpath:
+                syslog.syslog(syslog.LOG_DEBUG, 'DeviceMetaCfg: No change in timezone')
+                return
 
-        if new_timezone is None:
-            syslog.syslog(syslog.LOG_DEBUG,
-                          f'DeviceMetaCfg: Recieved empty timezone')
-            return
+            run_cmd(['timedatectl', 'set-timezone', new_tz])
+            self.timezone = new_tz
+            syslog.syslog(syslog.LOG_INFO, f'DeviceMetaCfg: Applied timezone {self.timezone}')
 
-        if new_timezone == self.timezone:
-            syslog.syslog(syslog.LOG_DEBUG,
-                          f'DeviceMetaCfg: No change in timezone')
-            return
+            run_cmd(['systemctl', 'restart', 'rsyslog'], True, False)
+            syslog.syslog(syslog.LOG_INFO, 'DeviceMetaCfg: Restarted rsyslog after timezone change')
 
-        # run command will print out log error in case of error
-        run_cmd(['timedatectl', 'set-timezone', new_timezone])
-        self.timezone = new_timezone
+        except OSError as e:
+            syslog.syslog(syslog.LOG_ERR, f'DeviceMetaCfg: Invalid timezone files for {ETC_LOCALTIME} {new_tz}: {e}')
+        except subprocess.CalledProcessError as e:
+            syslog.syslog(syslog.LOG_ERR, f'DeviceMetaCfg: Failed to set-timezone {new_tz} and restart rsyslog: {e}')
+        except Exception as e:
+            syslog.syslog(syslog.LOG_ERR, f'DeviceMetaCfg: Failed to apply timezone {new_tz}: {e}')
 
-        run_cmd(['systemctl', 'restart', 'rsyslog'], True, False)
-        syslog.syslog(syslog.LOG_INFO, 'DeviceMetaCfg: Restart rsyslog after '
-                      'changing timezone')
+    def timezone_update(self, data):
+        """
+        Call apply timezone handler.
+
+        Args:
+            data: Read table's key's data.
+        """
+        self.apply_timezone_if_needed(data.get('timezone'))
 
     def rsyslog_config(self, data):
         """

tests/hostcfgd/hostcfgd_test.py

@@ -853,3 +853,127 @@ def test_process_name_mismatch(self):
             pid = self.mem_stat_cfg.get_memory_statistics_pid()
             self.assertIsNone(pid)
             mock_syslog.assert_any_call(mock.ANY, "MemoryStatisticsCfg: PID 123 does not correspond to memory_statistics_service.py.")
+
+
+class TestDeviceMetaCfgLoad(TestCase):
+    """Test suite for DeviceMetaCfg load method with timezone functionality."""
+
+    def setUp(self):
+        """Set up test environment before each test case."""
+        self.devmeta_cfg = hostcfgd.DeviceMetaCfg()
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_load_initial_timezone_different_from_current(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test initial timezone setting when desired timezone differs from current. """
+        mock_realpath.side_effect = [
+            '/usr/share/zoneinfo/UTC',
+            '/usr/share/zoneinfo/America/New_York'
+        ]
+        dev_meta = {
+            'localhost': {
+                'hostname': 'test-host', 
+                'timezone': 'America/New_York'
+            }
+        }
+        
+        self.devmeta_cfg.load(dev_meta)
+        
+        expected_calls = [
+            call(['timedatectl', 'set-timezone', 'America/New_York']),
+            call(['systemctl', 'restart', 'rsyslog'], True, False)
+        ]
+        mock_run_cmd.assert_has_calls(expected_calls, any_order=False)
+        
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: Applied timezone America/New_York'),
+            call(mock.ANY, 'DeviceMetaCfg: Restarted rsyslog after timezone change')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_apply_timezone_oserror_exception(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test OSError exception handling in apply_timezone_if_needed. """
+        mock_realpath.side_effect = [
+            '/usr/share/zoneinfo/UTC',
+            OSError("Permission denied accessing timezone file")
+        ]
+        self.devmeta_cfg.apply_timezone_if_needed('America/New_York')
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: timezone update to America/New_York'),
+            call(mock.ANY, 'DeviceMetaCfg: Invalid timezone files for /etc/localtime America/New_York: Permission denied accessing timezone file')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+        mock_run_cmd.assert_not_called()
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_apply_timezone_subprocess_exception(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test subprocess.CalledProcessError exception handling in apply_timezone_if_needed. """
+        mock_realpath.side_effect = [
+            '/usr/share/zoneinfo/UTC',
+            '/usr/share/zoneinfo/America/New_York'
+        ]
+        mock_run_cmd.side_effect = [
+            CalledProcessError(returncode=1, cmd=['timedatectl', 'set-timezone', 'America/New_York'], output="Invalid timezone"),
+            None
+        ]
+        self.devmeta_cfg.apply_timezone_if_needed('America/New_York')
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: timezone update to America/New_York'),
+            call(mock.ANY, 'DeviceMetaCfg: Failed to set-timezone America/New_York and restart rsyslog: Command \'[\'timedatectl\', \'set-timezone\', \'America/New_York\']\' returned non-zero exit status 1.')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_apply_timezone_general_exception(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test general Exception handling in apply_timezone_if_needed. """
+        mock_realpath.side_effect = [
+            '/usr/share/zoneinfo/UTC',
+            '/usr/share/zoneinfo/America/New_York'
+        ]
+        mock_run_cmd.side_effect = RuntimeError("Unexpected system error")
+        self.devmeta_cfg.apply_timezone_if_needed('America/New_York')
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: timezone update to America/New_York'),
+            call(mock.ANY, 'DeviceMetaCfg: Failed to apply timezone America/New_York: Unexpected system error')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_apply_timezone_no_change_needed(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test apply_timezone_if_needed when no change is needed. """
+        mock_realpath.side_effect = [
+            '/usr/share/zoneinfo/America/New_York',
+            '/usr/share/zoneinfo/America/New_York'
+        ]
+        self.devmeta_cfg.timezone = 'America/New_York'
+        self.devmeta_cfg.apply_timezone_if_needed('America/New_York')
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: timezone update to America/New_York'),
+            call(mock.ANY, 'DeviceMetaCfg: No change in timezone')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+        mock_run_cmd.assert_not_called()
+
+    @mock.patch('hostcfgd.os.path.realpath')
+    @mock.patch('hostcfgd.run_cmd')
+    @mock.patch('hostcfgd.syslog.syslog')
+    def test_apply_timezone_empty_timezone(self, mock_syslog, mock_run_cmd, mock_realpath):
+        """ Test apply_timezone_if_needed with None/empty timezone. """
+        self.devmeta_cfg.apply_timezone_if_needed(None)  
+        expected_syslog_calls = [
+            call(mock.ANY, 'DeviceMetaCfg: timezone update to None'),
+            call(mock.ANY, 'DeviceMetaCfg: Recieved empty timezone')
+        ]
+        mock_syslog.assert_has_calls(expected_syslog_calls, any_order=False)
+        mock_realpath.assert_not_called()
+        mock_run_cmd.assert_not_called()