T6630: ntp: rename ptp-transport to ptp and use defaultValue for port

4 files changed, 35 insertions, 18 deletions

diff --git a/data/templates/chrony/chrony.conf.j2 b/data/templates/chrony/chrony.conf.j2
index 1fc488d24..79fa5e3a0 100644
--- a/data/templates/chrony/chrony.conf.j2
+++ b/data/templates/chrony/chrony.conf.j2
@@ -42,7 +42,7 @@ user {{ user }}
 {%         if config.pool is vyos_defined %}
 {%             set association = 'pool' %}
 {%         endif %}
-{{ association }} {{ server | replace('_', '-') }} iburst {{- ' nts' if config.nts is vyos_defined }} {{- ' noselect' if config.noselect is vyos_defined }} {{- ' prefer' if config.prefer is vyos_defined }} {{- ' xleave' if config.interleave is vyos_defined }} {{- ' port 319' if config.ptp_transport is vyos_defined }}
+{{ association }} {{ server | replace('_', '-') }} iburst {{- ' nts' if config.nts is vyos_defined }} {{- ' noselect' if config.noselect is vyos_defined }} {{- ' prefer' if config.prefer is vyos_defined }} {{- ' xleave' if config.interleave is vyos_defined }} {{- ' port ' ~ ptp.port if ptp.port is vyos_defined and config.ptp is vyos_defined }}
 {%     endfor %}
 {% endif %}
 
@@ -78,7 +78,7 @@ hwtimestamp {{ interface }} {{- ' rxfilter ' ~ config.receive_filter if config.r
 hwtimestamp *
 {% endif %}
 
-{% if ptp_transport is vyos_defined %}
+{% if ptp.port is vyos_defined %}
 # Enable sending and receiving NTP over PTP packets (PTP transport)
-ptpport 319
+ptpport {{ ptp.port }}
 {% endif %}
diff --git a/interface-definitions/service_ntp.xml.in b/interface-definitions/service_ntp.xml.in
index c4f3116ff..d6d3e0818 100644
--- a/interface-definitions/service_ntp.xml.in
+++ b/interface-definitions/service_ntp.xml.in
@@ -77,12 +77,17 @@
               </node>
             </children>
           </node>
-          <leafNode name="ptp-transport">
+          <node name="ptp">
             <properties>
-              <help>Enables the PTP transport for NTP packets</help>
-              <valueless/>
+              <help>Enable Precision Time Protocol (PTP) transport</help>
             </properties>
-          </leafNode>
+            <children>
+              #include <include/port-number.xml.i>
+              <leafNode name="port">
+                <defaultValue>319</defaultValue>
+              </leafNode>
+            </children>
+          </node>
           <leafNode name="leap-second">
             <properties>
               <help>Leap second behavior</help>
@@ -156,9 +161,9 @@
                   <valueless/>
                 </properties>
               </leafNode>
-              <leafNode name="ptp-transport">
+              <leafNode name="ptp">
                 <properties>
-                  <help>Use the PTP transport for the server</help>
+                  <help>Use Precision Time Protocol (PTP) transport for the server</help>
                   <valueless/>
                 </properties>
               </leafNode>
diff --git a/smoketest/scripts/cli/test_service_ntp.py b/smoketest/scripts/cli/test_service_ntp.py
index a39431c1b..02435bbfb 100755
--- a/smoketest/scripts/cli/test_service_ntp.py
+++ b/smoketest/scripts/cli/test_service_ntp.py
@@ -21,6 +21,7 @@ from base_vyostest_shim import VyOSUnitTestSHIM
 from vyos.configsession import ConfigSessionError
 from vyos.utils.process import cmd
 from vyos.utils.process import process_named_running
+from vyos.xml_ref import default_value
 
 PROCESS_NAME = 'chronyd'
 NTP_CONF = '/run/chrony/chrony.conf'
@@ -229,17 +230,19 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase):
         servers = ['192.0.2.1', '192.0.2.2']
         options = ['prefer']
 
+        default_ptp_port = default_value(base_path + ['ptp', 'port'])
+
         for server in servers:
             for option in options:
                 self.cli_set(base_path + ['server', server, option])
-            self.cli_set(base_path + ['server', server, 'ptp-transport'])
+            self.cli_set(base_path + ['server', server, 'ptp'])
 
         # commit changes (expected to fail)
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
 
         # add the required top-level option and commit
-        self.cli_set(base_path + ['ptp-transport'])
+        self.cli_set(base_path + ['ptp'])
         self.cli_commit()
 
         # Check generated configuration
@@ -254,9 +257,9 @@ class TestSystemNTP(VyOSUnitTestSHIM.TestCase):
         self.assertIn('leapsectz right/UTC', config)
 
         for server in servers:
-            self.assertIn(f'server {server} iburst ' + ' '.join(options) + ' port 319', config)
+            self.assertIn(f'server {server} iburst ' + ' '.join(options) + f' port {default_ptp_port}', config)
 
-        self.assertIn('ptpport 319', config)
+        self.assertIn(f'ptpport {default_ptp_port}', config)
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/service_ntp.py b/src/conf_mode/service_ntp.py
index f7dbc3776..32563aa0e 100755
--- a/src/conf_mode/service_ntp.py
+++ b/src/conf_mode/service_ntp.py
@@ -17,6 +17,7 @@
 import os
 
 from vyos.config import Config
+from vyos.config import config_dict_merge
 from vyos.configdict import is_node_changed
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_interface_exists
@@ -42,13 +43,21 @@ def get_config(config=None):
     if not conf.exists(base):
         return None
 
-    ntp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True, with_defaults=True)
+    ntp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
     ntp['config_file'] = config_file
     ntp['user'] = user_group
 
     tmp = is_node_changed(conf, base + ['vrf'])
     if tmp: ntp.update({'restart_required': {}})
 
+    # We have gathered the dict representation of the CLI, but there are default
+    # options which we need to update into the dictionary retrived.
+    default_values = conf.get_config_defaults(**ntp.kwargs, recursive=True)
+    # Only defined PTP default port, if PTP feature is in use
+    if 'ptp' not in ntp:
+        del default_values['ptp']
+
+    ntp = config_dict_merge(default_values, ntp)
     return ntp
 
 def verify(ntp):
@@ -89,10 +98,10 @@ def verify(ntp):
 
     if 'server' in ntp:
         for host, server in ntp['server'].items():
-            if 'ptp_transport' in server:
-                if 'ptp_transport' not in ntp:
-                    raise ConfigError('ptp-transport must be enabled on the service '\
-                                      f'before it can be used with server {host}')
+            if 'ptp' in server:
+                if 'ptp' not in ntp:
+                    raise ConfigError('PTP must be enabled for the NTP service '\
+                                      f'before it can be used for server "{host}"')
                 else:
                     break