From 5531d5cee744f90ff24231a9ef467282b254adc2 Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Thu, 15 Jul 2021 18:09:30 +0200 Subject: [PATCH] Force immediate NTP time sync with chronyd at IPA startup In order to make sure we have the correct time early, e.g. by the time we create a TLS certificate, this patch proposes to force an immediate NTP update when using chronyd. While the previous approach uses the passed NTP server as well, the update may happen only after chronyd has performed measurements (which may be too late). Story: #2009058 Task: #42843 Change-Id: I6edafe8edeb8549f324959e7a1ec175c3049a515 --- .../tests/unit/extensions/test_standby.py | 8 ++--- ironic_python_agent/tests/unit/test_utils.py | 31 +++---------------- ironic_python_agent/utils.py | 20 +++--------- ...ix_chronyd_time_sync-626a14b66ca37677.yaml | 6 ++++ 4 files changed, 19 insertions(+), 46 deletions(-) create mode 100644 releasenotes/notes/fix_chronyd_time_sync-626a14b66ca37677.yaml diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index 97dffbe8e..559a15bb9 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -1385,15 +1385,15 @@ class TestStandbyExtension(base.IronicAgentTest): self.agent_extension._sync_clock() - calls = [mock.call('chronyd', check_exit_code=[0, 1]), - mock.call('chronyc', 'add', 'server', '192.168.1.1'), - mock.call('chronyc', 'makestep'), + calls = [mock.call('chronyc', 'shutdown', check_exit_code=[0, 1]), + mock.call("chronyd -q 'server 192.168.1.1 iburst'", + shell=True), mock.call('hwclock', '-v', '--systohc')] execute_mock.assert_has_calls(calls) execute_mock.reset_mock() execute_mock.side_effect = [ - ('', ''), ('', ''), ('', ''), + ('', ''), ('', ''), processutils.ProcessExecutionError('boop') ] diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index eb6520f01..bfad67f08 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -856,27 +856,8 @@ class TestClockSyncUtils(ironic_agent_base.IronicAgentTest): mock_time_method.return_value = 'chronyd' utils.sync_clock() mock_execute.assert_has_calls([ - mock.call('chronyd', check_exit_code=[0, 1]), - mock.call('chronyc', 'add', 'server', '192.168.1.1'), - mock.call('chronyc', 'makestep'), - ]) - - @mock.patch.object(utils, 'determine_time_method', autospec=True) - def test_sync_clock_chrony_already_present(self, mock_time_method, - mock_execute): - self.config(ntp_server='192.168.1.1') - mock_time_method.return_value = 'chronyd' - mock_execute.side_effect = [ - ('', ''), - processutils.ProcessExecutionError( - stderr='Source already present'), - ('', ''), - ] - utils.sync_clock() - mock_execute.assert_has_calls([ - mock.call('chronyd', check_exit_code=[0, 1]), - mock.call('chronyc', 'add', 'server', '192.168.1.1'), - mock.call('chronyc', 'makestep'), + mock.call('chronyc', 'shutdown', check_exit_code=[0, 1]), + mock.call("chronyd -q 'server 192.168.1.1 iburst'", shell=True), ]) @mock.patch.object(utils, 'determine_time_method', autospec=True) @@ -889,12 +870,8 @@ class TestClockSyncUtils(ironic_agent_base.IronicAgentTest): processutils.ProcessExecutionError(stderr='time verboten'), ] self.assertRaisesRegex(errors.CommandExecutionError, - 'Error occured adding ntp', - utils.sync_clock) - mock_execute.assert_has_calls([ - mock.call('chronyd', check_exit_code=[0, 1]), - mock.call('chronyc', 'add', 'server', '192.168.1.1'), - ]) + 'Failed to sync time using chrony to ntp ' + 'server:', utils.sync_clock) @mock.patch.object(utils, 'determine_time_method', autospec=True) def test_sync_clock_none(self, mock_time_method, mock_execute): diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index b5d0533a1..2fc60ff1c 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -835,21 +835,11 @@ def sync_clock(ignore_errors=False): raise errors.CommandExecutionError(msg) elif method == 'chronyd': try: - # 0 should be if chronyd started - # 1 if already running - execute('chronyd', check_exit_code=[0, 1]) - # NOTE(TheJulia): Once started, chronyd forks and stays in the - # background as a server service, it will continue to keep the - # clock in sync. - try: - execute('chronyc', 'add', 'server', CONF.ntp_server) - except processutils.ProcessExecutionError as e: - if 'Source already present' not in str(e): - msg = 'Error occured adding ntp server: %s' % e - LOG.error(msg) - raise errors.CommandExecutionError(msg) - # Force the clock to sync now. - execute('chronyc', 'makestep') + # stop chronyd, ignore if it ran before or not + execute('chronyc', 'shutdown', check_exit_code=[0, 1]) + # force a time sync now + query = "server " + CONF.ntp_server + " iburst" + execute("chronyd -q \'%s\'" % query, shell=True) LOG.debug('Set software clock using chrony') except (processutils.ProcessExecutionError, errors.CommandExecutionError) as e: diff --git a/releasenotes/notes/fix_chronyd_time_sync-626a14b66ca37677.yaml b/releasenotes/notes/fix_chronyd_time_sync-626a14b66ca37677.yaml new file mode 100644 index 000000000..cbe25dfc0 --- /dev/null +++ b/releasenotes/notes/fix_chronyd_time_sync-626a14b66ca37677.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where the NTP time sync at the IPA startup via chronyd is + not immediate (which can break time sensitive components such as the + generation of a TLS certificate).