Improved comments and pythonisms from PR review

XSConsoleConstants.py

@@ -0,0 +1,2 @@
+# Default NTP domains are <int>.[centos|xenserver].pool.ntp.org
+DEFAULT_NTP_DOMAINS = [".centos.pool.ntp.org", ".xenserver.pool.ntp.org"]

XSConsoleData.py

@@ -490,9 +490,9 @@ def UpdateFromNTPConf(self):
         if not 'ntp' in self.data:
             self.data['ntp'] = {}
 
-        (status, output) = getstatusoutput("/bin/cat /etc/chrony.conf")
-        if status == 0:
-            self.ScanNTPConf(output.split("\n"))
+        if os.path.isfile("/etc/chrony.conf"):
+            with open("/etc/chrony.conf", "r") as confFile:
+                self.ScanNTPConf(confFile.read().splitlines())
 
         self.data['ntp']['method'] = ""
         chronyPerm = os.stat("/etc/dhcp/dhclient.d/chrony.sh").st_mode
@@ -507,7 +507,7 @@ def UpdateFromNTPConf(self):
 
             servers = self.data['ntp']['servers']
             if len(servers) == 4 and all(
-                "centos.pool.ntp.org" in server
+                inDefaultNTPDomains(server)
                 for server in self.data['ntp']['servers']
             ):
                 self.data['ntp']['method'] = "Default"
@@ -563,17 +563,10 @@ def SaveToNTPConf(self):
 
         # Force chronyd to update the time
         if self.data['ntp']['method'] != "Disabled":
-            servers = self.data['ntp']['servers']
-            if self.data['ntp']['method'] == "DHCP":
-                servers = self.GetDHClientInterfaces()
-
-            if servers:
-                self.StopService("chronyd")
-                # Single shot time update like: `ntpdate servers[0]`
-                getoutput("chronyd -q 'server %s iburst'" % servers[0])
-                # Write the system time (set by the single shot NTP) to the HW clock
-                getoutput("hwclock -w")
-                self.StartService("chronyd")
+            # Prompt chrony to update the time immediately
+            getoutput("chronyc makestep")
+            # Write the system time (set by the single shot NTP) to the HW clock
+            getoutput("hwclock -w")
 
     def AddDHCPNTP(self):
         # Double-check authentication
@@ -590,36 +583,49 @@ def AddDHCPNTP(self):
             with open("/var/lib/dhclient/chrony.servers.%s" % interface, "w") as chronyFile:
                 chronyFile.write("%s iburst prefer\n" % ntpServer)
 
+        # Ensure chrony is enabled
+        self.EnableService("chronyd")
+        self.EnableService("chrony-wait")
+
     def ResetDefaultNTPServers(self):
         # Double-check authentication
         Auth.Inst().AssertAuthenticated()
 
         Data.Inst().NTPServersSet(
-            [
-                "0.centos.pool.ntp.org",
-                "1.centos.pool.ntp.org",
-                "2.centos.pool.ntp.org",
-                "3.centos.pool.ntp.org",
-            ]
+            ["%d%s" % (i, DEFAULT_NTP_DOMAINS[0]) for i in range(4)]
         )
 
     def GetDHClientInterfaces(self):
-        (status, output) = getstatusoutput("ls /var/lib/xcp/ | grep leases")
-        if status != 0:
+        try:
+            leases = [filename for filename in os.listdir("/var/lib/xcp") if "leases" in filename]
+        except OSError:
             return []
 
-        dhclientFiles = output.splitlines()
         pattern = "dhclient-(.*).leases"
         interfaces = []
-        for dhclientFile in dhclientFiles:
+        for dhclientFile in leases:
             match = re.match(pattern, dhclientFile)
             if match:
                 interfaces.append(match.group(1))
 
         return interfaces
 
     def GetDHCPNTPServer(self, interface):
-        ntpServer = getoutput("grep ntp-servers /var/lib/xcp/dhclient-%s.leases | tail -1 | awk '{{ print ( $3 ) }}'" % interface).strip()[:-1]
+        expectedLeaseFile = "/var/lib/xcp/dhclient-%s.leases" % interface
+        if not os.path.isfile(expectedLeaseFile):
+            return None
+
+        with open(expectedLeaseFile, "r") as leaseFile:
+            data = leaseFile.read().splitlines()
+
+        ntpServerLines = [line for line in data if "ntp-servers" in line]
+
+        # Extract <ip-addr> from "  option ntp-servers <ip-addr>;"
+        try:
+            ntpServer = ntpServerLines[-1].split()[-1][:-1]
+        except:
+            ntpServer = None
+
         return ntpServer
 
     def RemoveDHCPNTP(self):

XSConsoleStandard.py

@@ -18,6 +18,7 @@
 from XSConsoleAuth import *
 from XSConsoleBases import *
 from XSConsoleConfig import *
+from XSConsoleConstants import *
 from XSConsoleData import *
 from XSConsoleDataUtils import *
 from XSConsoleDialogueBases import *

XSConsoleUtils.py

@@ -17,6 +17,7 @@
 from pprint import pprint
 
 from XSConsoleBases import *
+from XSConsoleConstants import *
 from XSConsoleLang import *
 
 # Utils that need to access Data must go in XSConsoleDataUtils,
@@ -330,3 +331,7 @@ def SRSizeString(cls, inBytes):
     def DiskSizeString(cls, inBytes):
         return cls.BinarySizeString(inBytes)+' ('+cls.DecimalSizeString(inBytes)+')'
 
+
+def inDefaultNTPDomains(ntpServer):
+    return any(ntpServer.endswith(defaultDomain) for defaultDomain in DEFAULT_NTP_DOMAINS)
+

plugins-base/XSFeatureNTP.py

@@ -303,7 +303,7 @@ def HandleKeyADD(self, inKey):
 
                 data=Data.Inst()
                 if data.ntp.method("") == "Default":
-                    data.NTPServersSet([server for server in data.ntp.servers([]) if "centos.pool.ntp.org" not in server])
+                    data.NTPServersSet([server for server in data.ntp.servers([]) if not inDefaultNTPDomains(server)])
 
                 data.RemoveDHCPNTP()