VMware: Improve vmware_host_ntp module (#47273)

lib/ansible/modules/cloud/vmware/vmware_host_ntp.py

@@ -2,6 +2,7 @@
 # -*- coding: utf-8 -*-
 
 # Copyright: (c) 2018, Abhijeet Kasurde <akasurde@redhat.com>
+# Copyright: (c) 2018, Christian Kotte <christian.kotte@gmx.de>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -16,57 +17,80 @@ ANSIBLE_METADATA = {
 DOCUMENTATION = r'''
 ---
 module: vmware_host_ntp
-short_description: Manage NTP configurations about an ESXi host
+short_description: Manage NTP server configuration of an ESXi host
 description:
-- This module can be used to manage NTP configuration information about an ESXi host.
+- This module can be used to configure, add or remove NTP servers from an ESXi host.
+- If C(state) is not given, the NTP servers will be configured in the exact sequence.
 - User can specify an ESXi hostname or Cluster name. In case of cluster name, all ESXi hosts are updated.
 version_added: '2.5'
 author:
 - Abhijeet Kasurde (@Akasurde)
+- Christian Kotte (@ckotte)
 notes:
 - Tested on vSphere 6.5
 requirements:
 - python >= 2.6
 - PyVmomi
 options:
-  cluster_name:
-    description:
-    - Name of the cluster.
-    - NTP settings are applied to every ESXi host system in the given cluster.
-    - If C(esxi_hostname) is not given, this parameter is required.
   esxi_hostname:
     description:
-    - ESXi hostname.
+    - Name of the host system to work with.
-    - NTP settings are applied to this ESXi host system.
+    - This parameter is required if C(cluster_name) is not specified.
-    - If C(cluster_name) is not given, this parameter is required.
+    type: str
+  cluster_name:
+    description:
+    - Name of the cluster from which all host systems will be used.
+    - This parameter is required if C(esxi_hostname) is not specified.
+    type: str
   ntp_servers:
     description:
-    - "IP or FQDN of NTP server/s."
+    - "IP or FQDN of NTP server(s)."
     - This accepts a list of NTP servers. For multiple servers, please look at the examples.
+    type: dict
     required: True
   state:
     description:
-    - "present: Add NTP server/s, if it specified server/s are absent else do nothing."
+    - "present: Add NTP server(s), if specified server(s) are absent else do nothing."
-    - "absent: Remove NTP server/s, if specified server/s are present else do nothing."
+    - "absent: Remove NTP server(s), if specified server(s) are present else do nothing."
-    default: present
+    - Specified NTP server(s) will be configured if C(state) isn't specified.
     choices: [ present, absent ]
+    type: str
+  verbose:
+    description:
+    - Verbose output of the configuration change.
+    - Explains if an NTP server was added, removed, or if the NTP server sequence was changed.
+    type: bool
+    required: false
+    default: false
+    version_added: 2.8
 extends_documentation_fragment: vmware.documentation
 '''
 
 EXAMPLES = r'''
-- name: Set NTP setting for all ESXi Host in given Cluster
+- name: Configure NTP servers for an ESXi Host
+  vmware_host_ntp:
+    hostname: vcenter01.example.local
+    username: administrator@vsphere.local
+    password: SuperSecretPassword
+    esxi_hostname: esx01.example.local
+    ntp_servers:
+        - 0.pool.ntp.org
+        - 1.pool.ntp.org
+  delegate_to: localhost
+
+- name: Set NTP servers for all ESXi Host in given Cluster
   vmware_host_ntp:
     hostname: '{{ vcenter_hostname }}'
     username: '{{ vcenter_username }}'
     password: '{{ vcenter_password }}'
-    cluster_name: cluster_name
+    cluster_name: '{{ cluster_name }}'
     state: present
     ntp_servers:
         - 0.pool.ntp.org
         - 1.pool.ntp.org
   delegate_to: localhost
 
-- name: Set NTP setting for an ESXi Host
+- name: Set NTP servers for an ESXi Host
   vmware_host_ntp:
     hostname: '{{ vcenter_hostname }}'
     username: '{{ vcenter_username }}'
@@ -78,7 +102,7 @@ EXAMPLES = r'''
         - 1.pool.ntp.org
   delegate_to: localhost
 
-- name: Remove NTP setting for an ESXi Host
+- name: Remove NTP servers for an ESXi Host
   vmware_host_ntp:
     hostname: '{{ vcenter_hostname }}'
     username: '{{ vcenter_username }}'
@@ -90,7 +114,24 @@ EXAMPLES = r'''
   delegate_to: localhost
 '''
 
-RETURN = r'''#
+RETURN = r'''
+results:
+    description: metadata about host system's NTP configuration
+    returned: always
+    type: dict
+    sample: {
+        "esx01.example.local": {
+            "ntp_servers_changed": ["time1.example.local", "time2.example.local", "time3.example.local", "time4.example.local"],
+            "ntp_servers": ["time3.example.local", "time4.example.local"],
+            "ntp_servers_previous": ["time1.example.local", "time2.example.local"],
+        },
+        "esx02.example.local": {
+            "ntp_servers_changed": ["time3.example.local"],
+            "ntp_servers_current": ["time1.example.local", "time2.example.local", "time3.example.local"],
+            "state": "present",
+            "ntp_servers_previous": ["time1.example.local", "time2.example.local"],
+        },
+    }
 '''
 
 try:
@@ -104,94 +145,234 @@ from ansible.module_utils._text import to_native
 
 
 class VmwareNtpConfigManager(PyVmomi):
+    """Class to manage configured NTP servers"""
+
     def __init__(self, module):
         super(VmwareNtpConfigManager, self).__init__(module)
         cluster_name = self.params.get('cluster_name', None)
         esxi_host_name = self.params.get('esxi_hostname', None)
         self.ntp_servers = self.params.get('ntp_servers', list())
         self.hosts = self.get_all_host_objs(cluster_name=cluster_name, esxi_host_name=esxi_host_name)
+        if not self.hosts:
+            self.module.fail_json(msg="Failed to find host system.")
         self.results = {}
-        self.desired_state = module.params['state']
+        self.desired_state = self.params.get('state', None)
+        self.verbose = module.params.get('verbose', False)
 
-    def update_ntp_servers(self, host, ntp_servers, operation='add'):
+    def update_ntp_servers(self, host, ntp_servers_configured, ntp_servers_to_change, operation='overwrite'):
-        changed = False
+        """Update NTP server configuration"""
         host_date_time_manager = host.configManager.dateTimeSystem
         if host_date_time_manager:
-            available_ntp_servers = host_date_time_manager.dateTimeInfo.ntpConfig.server
+            # Prepare new NTP server list
-            available_ntp_servers = list(filter(None, available_ntp_servers))
+            if operation == 'overwrite':
-            if operation == 'add':
+                new_ntp_servers = list(ntp_servers_to_change)
-                available_ntp_servers = available_ntp_servers + ntp_servers
+            else:
-            elif operation == 'delete':
+                new_ntp_servers = list(ntp_servers_configured)
-                for server in ntp_servers:
+                if operation == 'add':
-                    if server in available_ntp_servers:
+                    new_ntp_servers = new_ntp_servers + ntp_servers_to_change
-                        available_ntp_servers.remove(server)
+                elif operation == 'delete':
+                    for server in ntp_servers_to_change:
+                        if server in new_ntp_servers:
+                            new_ntp_servers.remove(server)
+
+            # build verbose message
+            if self.verbose:
+                message = self.build_changed_message(
+                    ntp_servers_configured,
+                    new_ntp_servers,
+                    ntp_servers_to_change,
+                    operation
+                )
 
             ntp_config_spec = vim.host.NtpConfig()
-            ntp_config_spec.server = available_ntp_servers
+            ntp_config_spec.server = new_ntp_servers
             date_config_spec = vim.host.DateTimeConfig()
             date_config_spec.ntpConfig = ntp_config_spec
             try:
-                if self.module.check_mode:
+                if not self.module.check_mode:
-                    self.results[host.name]['after_change_ntp_servers'] = available_ntp_servers
-                else:
                     host_date_time_manager.UpdateDateTimeConfig(date_config_spec)
-                    self.results[host.name]['after_change_ntp_servers'] = host_date_time_manager.dateTimeInfo.ntpConfig.server
+                if self.verbose:
-                changed = True
+                    self.results[host.name]['msg'] = message
-            except vim.fault.HostConfigFault as e:
+            except vim.fault.HostConfigFault as config_fault:
-                self.results[host.name]['error'] = to_native(e.msg)
+                self.module.fail_json(
-            except Exception as e:
+                    msg="Failed to configure NTP for host '%s' due to : %s" %
-                self.results[host.name]['error'] = to_native(e)
+                    (host.name, to_native(config_fault.msg))
+                )
 
-        return changed
+            return new_ntp_servers
 
     def check_host_state(self):
+        """Check ESXi host configuration"""
         change_list = []
         changed = False
         for host in self.hosts:
-            ntp_servers_to_change = self.check_ntp_servers(host=host)
+            self.results[host.name] = dict()
-            self.results[host.name].update(dict(
+            ntp_servers_configured, ntp_servers_to_change = self.check_ntp_servers(host=host)
-                ntp_servers_to_change=ntp_servers_to_change,
+            # add/remove NTP servers
-                desired_state=self.desired_state,
+            if self.desired_state:
-            )
+                self.results[host.name]['state'] = self.desired_state
-            )
+                if ntp_servers_to_change:
-
+                    self.results[host.name]['ntp_servers_changed'] = ntp_servers_to_change
-            if not ntp_servers_to_change:
+                    operation = 'add' if self.desired_state == 'present' else 'delete'
-                change_list.append(False)
+                    new_ntp_servers = self.update_ntp_servers(
-                self.results[host.name]['current_state'] = self.desired_state
+                        host=host,
-            elif ntp_servers_to_change:
+                        ntp_servers_configured=ntp_servers_configured,
-                if self.desired_state == 'present':
+                        ntp_servers_to_change=ntp_servers_to_change,
-                    changed = self.update_ntp_servers(host=host, ntp_servers=ntp_servers_to_change)
+                        operation=operation
-                    change_list.append(changed)
+                    )
-                elif self.desired_state == 'absent':
+                    self.results[host.name]['ntp_servers_current'] = new_ntp_servers
-                    changed = self.update_ntp_servers(host=host, ntp_servers=ntp_servers_to_change, operation='delete')
+                    self.results[host.name]['changed'] = True
-                    change_list.append(changed)
+                    change_list.append(True)
-                self.results[host.name]['current_state'] = self.desired_state
+                else:
+                    self.results[host.name]['ntp_servers_current'] = ntp_servers_configured
+                    if self.verbose:
+                        self.results[host.name]['msg'] = (
+                            "NTP servers already added" if self.desired_state == 'present'
+                            else "NTP servers already removed"
+                        )
+                    self.results[host.name]['changed'] = False
+                    change_list.append(False)
+            # overwrite NTP servers
+            else:
+                self.results[host.name]['ntp_servers'] = self.ntp_servers
+                if ntp_servers_to_change:
+                    self.results[host.name]['ntp_servers_changed'] = self.get_differt_entries(
+                        ntp_servers_configured,
+                        ntp_servers_to_change
+                    )
+                    self.update_ntp_servers(
+                        host=host,
+                        ntp_servers_configured=ntp_servers_configured,
+                        ntp_servers_to_change=ntp_servers_to_change,
+                        operation='overwrite'
+                    )
+                    self.results[host.name]['changed'] = True
+                    change_list.append(True)
+                else:
+                    if self.verbose:
+                        self.results[host.name]['msg'] = "NTP servers already configured"
+                    self.results[host.name]['changed'] = False
+                    change_list.append(False)
 
         if any(change_list):
             changed = True
         self.module.exit_json(changed=changed, results=self.results)
 
     def check_ntp_servers(self, host):
+        """Check configured NTP servers"""
         update_ntp_list = []
         host_datetime_system = host.configManager.dateTimeSystem
         if host_datetime_system:
-            ntp_servers = host_datetime_system.dateTimeInfo.ntpConfig.server
+            ntp_servers_configured = host_datetime_system.dateTimeInfo.ntpConfig.server
-            self.results[host.name] = dict(available_ntp_servers=ntp_servers)
+            # add/remove NTP servers
-            for ntp_server in self.ntp_servers:
+            if self.desired_state:
-                if self.desired_state == 'present' and ntp_server not in ntp_servers:
+                for ntp_server in self.ntp_servers:
-                    update_ntp_list.append(ntp_server)
+                    if self.desired_state == 'present' and ntp_server not in ntp_servers_configured:
-                if self.desired_state == 'absent' and ntp_server in ntp_servers:
+                        update_ntp_list.append(ntp_server)
-                    update_ntp_list.append(ntp_server)
+                    if self.desired_state == 'absent' and ntp_server in ntp_servers_configured:
-        return update_ntp_list
+                        update_ntp_list.append(ntp_server)
+            # overwrite NTP servers
+            else:
+                if ntp_servers_configured != self.ntp_servers:
+                    for ntp_server in self.ntp_servers:
+                        update_ntp_list.append(ntp_server)
+            if update_ntp_list:
+                self.results[host.name]['ntp_servers_previous'] = ntp_servers_configured
+
+        return ntp_servers_configured, update_ntp_list
+
+    def build_changed_message(self, ntp_servers_configured, new_ntp_servers, ntp_servers_to_change, operation):
+        """Build changed message"""
+        check_mode = 'would be ' if self.module.check_mode else ''
+        if operation == 'overwrite':
+            # get differences
+            add = self.get_not_in_list_one(new_ntp_servers, ntp_servers_configured)
+            remove = self.get_not_in_list_one(ntp_servers_configured, new_ntp_servers)
+            diff_servers = list(ntp_servers_configured)
+            if add and remove:
+                for server in add:
+                    diff_servers.append(server)
+                for server in remove:
+                    diff_servers.remove(server)
+                if new_ntp_servers != diff_servers:
+                    message = (
+                        "NTP server %s %sadded and %s %sremoved and the server sequence %schanged as well" %
+                        (self.array_to_string(add), check_mode, self.array_to_string(remove), check_mode, check_mode)
+                    )
+                else:
+                    if new_ntp_servers != ntp_servers_configured:
+                        message = (
+                            "NTP server %s %sreplaced with %s" %
+                            (self.array_to_string(remove), check_mode, self.array_to_string(add))
+                        )
+                    else:
+                        message = (
+                            "NTP server %s %sremoved and %s %sadded" %
+                            (self.array_to_string(remove), check_mode, self.array_to_string(add), check_mode)
+                        )
+            elif add:
+                for server in add:
+                    diff_servers.append(server)
+                if new_ntp_servers != diff_servers:
+                    message = (
+                        "NTP server %s %sadded and the server sequence %schanged as well" %
+                        (self.array_to_string(add), check_mode, check_mode)
+                    )
+                else:
+                    message = "NTP server %s %sadded" % (self.array_to_string(add), check_mode)
+            elif remove:
+                for server in remove:
+                    diff_servers.remove(server)
+                if new_ntp_servers != diff_servers:
+                    message = (
+                        "NTP server %s %sremoved and the server sequence %schanged as well" %
+                        (self.array_to_string(remove), check_mode, check_mode)
+                    )
+                else:
+                    message = "NTP server %s %sremoved" % (self.array_to_string(remove), check_mode)
+            else:
+                message = "NTP server sequence %schanged" % check_mode
+        elif operation == 'add':
+            message = "NTP server %s %sadded" % (self.array_to_string(ntp_servers_to_change), check_mode)
+        elif operation == 'delete':
+            message = "NTP server %s %sremoved" % (self.array_to_string(ntp_servers_to_change), check_mode)
+
+        return message
+
+    @staticmethod
+    def get_not_in_list_one(list1, list2):
+        """Return entries that ore not in list one"""
+        return [x for x in list1 if x not in set(list2)]
+
+    @staticmethod
+    def array_to_string(array):
+        """Return string from array"""
+        if len(array) > 2:
+            string = (
+                ', '.join("'{0}'".format(element) for element in array[:-1]) + ', and '
+                + "'{0}'".format(str(array[-1]))
+            )
+        elif len(array) == 2:
+            string = ' and '.join("'{0}'".format(element) for element in array)
+        elif len(array) == 1:
+            string = "'{0}'".format(array[0])
+        return string
+
+    @staticmethod
+    def get_differt_entries(list1, list2):
+        """Return different entries of two lists"""
+        return [a for a in list1 + list2 if (a not in list1) or (a not in list2)]
 
 
 def main():
+    """Main"""
     argument_spec = vmware_argument_spec()
     argument_spec.update(
         cluster_name=dict(type='str', required=False),
         esxi_hostname=dict(type='str', required=False),
         ntp_servers=dict(type='list', required=True),
-        state=dict(type='str', default='present', choices=['absent', 'present']),
+        state=dict(type='str', choices=['absent', 'present']),
+        verbose=dict(type='bool', default=False, required=False)
     )
 
     module = AnsibleModule(

test/integration/targets/vmware_host_ntp/tasks/main.yml

@@ -55,7 +55,7 @@
 - debug: var=ccr1
 - debug: var=host1
 
-- name: Change facts about all hosts in given cluster
+- name: Add NTP server to a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -69,7 +69,7 @@
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster
+- name: Add another NTP server to a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -83,7 +83,7 @@
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster
+- name: Remove NTP server from a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -93,11 +93,11 @@
     ntp_server:
       - 1.pool.ntp.org
     validate_certs: no
-  register: present
+  register: absent_one
 
-- debug: var=present
+- debug: var=absent_one
 
-- name: Change facts about all hosts in given cluster
+- name: Remove NTP server from a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -111,7 +111,7 @@
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster
+- name: Add more NTP servers to a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -127,7 +127,7 @@
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster
+- name: Remove all NTP servers from a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -142,56 +142,42 @@
       - 4.pool.ntp.org
       - 6.pool.ntp.org
     validate_certs: no
-  register: present
+  register: absent_all
 
-- debug: var=present
+- debug: var=absent_all
 
-- name: Change facts about all hosts in given cluster in check mode
+- name: Configure NTP servers for a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
     password: "{{ vcsim_instance['json']['password'] }}"
     esxi_hostname: "{{ host1 }}"
-    state: present
     ntp_server:
       - 0.pool.ntp.org
-    validate_certs: no
-  register: present
-  check_mode: yes
-
-- debug: var=present
-
-- name: Change facts about all hosts in given cluster in check mode
-  vmware_host_ntp:
-    hostname: "{{ vcsim }}"
-    username: "{{ vcsim_instance['json']['username'] }}"
-    password: "{{ vcsim_instance['json']['password'] }}"
-    esxi_hostname: "{{ host1 }}"
-    state: present
-    ntp_server:
       - 1.pool.ntp.org
+      - 2.pool.ntp.org
     validate_certs: no
-  register: present
+  register: ntp_servers
-  check_mode: yes
 
-- debug: var=present
+- debug: var=ntp_servers
 
-- name: Change facts about all hosts in given cluster in check mode
+- name: Configure NTP servers for a host
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
     password: "{{ vcsim_instance['json']['password'] }}"
     esxi_hostname: "{{ host1 }}"
-    state: absent
     ntp_server:
-      - 1.pool.ntp.org
+      - 3.pool.ntp.org
+      - 4.pool.ntp.org
+      - 5.pool.ntp.org
+    verbose: true
     validate_certs: no
-  register: present
+  register: ntp_servers
-  check_mode: yes
 
-- debug: var=present
+- debug: var=ntp_servers
 
-- name: Change facts about all hosts in given cluster in check mode
+- name: Add NTP server to a host in check mode
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
@@ -199,46 +185,40 @@
     esxi_hostname: "{{ host1 }}"
     state: present
     ntp_server:
-      - 1.pool.ntp.org
+      - 0.pool.ntp.org
     validate_certs: no
   register: present
   check_mode: yes
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster in check mode
+- name: Remove NTP server to a host in check mode
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
     password: "{{ vcsim_instance['json']['password'] }}"
     esxi_hostname: "{{ host1 }}"
-    state: present
+    state: absent
     ntp_server:
-      - 2.pool.ntp.org
+      - 0.pool.ntp.org
-      - 3.pool.ntp.org
-      - 4.pool.ntp.org
     validate_certs: no
   register: present
   check_mode: yes
 
 - debug: var=present
 
-- name: Change facts about all hosts in given cluster in check mode
+- name: Configure NTP servers for a host in check mode
   vmware_host_ntp:
     hostname: "{{ vcsim }}"
     username: "{{ vcsim_instance['json']['username'] }}"
     password: "{{ vcsim_instance['json']['password'] }}"
     esxi_hostname: "{{ host1 }}"
-    state: absent
     ntp_server:
       - 0.pool.ntp.org
       - 1.pool.ntp.org
       - 2.pool.ntp.org
-      - 3.pool.ntp.org
-      - 4.pool.ntp.org
-      - 6.pool.ntp.org
     validate_certs: no
-  register: present
+  register: ntp_servers
   check_mode: yes
 
-- debug: var=present
+- debug: var=ntp_servers