From 9fbd65958d4863fd9cfc226d19586a913c19a58c Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Thu, 9 Jul 2020 20:04:23 -0500 Subject: [PATCH] hostname: hostnamectl check -> SystemdStrategy (#70532) Change: - Move hostnamectl check out of GenericStrategy because it was incorrect for everything except the SystemdStrategy which is where it belongs. - Add some initial tests for the hostname module, though we are limited by the fact that we can't do much testing with it in containers. Test Plan: - new hostname integration tests Signed-off-by: Rick Elrod Co-authored-by: Sam Doran --- lib/ansible/modules/hostname.py | 14 +- test/integration/targets/hostname/aliases | 4 + .../targets/hostname/tasks/RedHat.yml | 15 +++ .../targets/hostname/tasks/check_mode.yml | 20 +++ .../targets/hostname/tasks/default.yml | 2 + .../targets/hostname/tasks/main.yml | 121 ++++++++++++++++++ .../targets/hostname/vars/FreeBSD.yml | 1 + .../targets/hostname/vars/RedHat.yml | 1 + .../targets/hostname/vars/default.yml | 1 + 9 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 test/integration/targets/hostname/aliases create mode 100644 test/integration/targets/hostname/tasks/RedHat.yml create mode 100644 test/integration/targets/hostname/tasks/check_mode.yml create mode 100644 test/integration/targets/hostname/tasks/default.yml create mode 100644 test/integration/targets/hostname/tasks/main.yml create mode 100644 test/integration/targets/hostname/vars/FreeBSD.yml create mode 100644 test/integration/targets/hostname/vars/RedHat.yml create mode 100644 test/integration/targets/hostname/vars/default.yml diff --git a/lib/ansible/modules/hostname.py b/lib/ansible/modules/hostname.py index 189656cea78..6d8a0722e08 100644 --- a/lib/ansible/modules/hostname.py +++ b/lib/ansible/modules/hostname.py @@ -20,7 +20,7 @@ requirements: [ hostname ] description: - Set system's hostname, supports most OSs/Distributions, including those using systemd. - Note, this module does *NOT* modify C(/etc/hosts). You need to modify it yourself using other modules like template or replace. - - Windows, HP-UX and AIX are not currently supported. + - Windows, macOS, HP-UX and AIX are not currently supported. options: name: description: @@ -29,8 +29,8 @@ options: use: description: - Which strategy to use to update the hostname. - - If not set we try to autodetect, but this can be problematic, specially with containers as they can present misleading information. - choices: ['generic', 'debian','sles', 'redhat', 'alpine', 'systemd', 'openrc', 'openbsd', 'solaris', 'freebsd'] + - If not set we try to autodetect, but this can be problematic, particularly with containers as they can present misleading information. + choices: ['generic', 'debian', 'sles', 'redhat', 'alpine', 'systemd', 'openrc', 'openbsd', 'solaris', 'freebsd'] version_added: '2.9' ''' @@ -155,9 +155,7 @@ class GenericStrategy(object): def __init__(self, module): self.module = module self.changed = False - self.hostname_cmd = self.module.get_bin_path('hostnamectl', False) - if not self.hostname_cmd: - self.hostname_cmd = self.module.get_bin_path('hostname', True) + self.hostname_cmd = self.module.get_bin_path('hostname', True) def update_current_and_permanent_hostname(self): self.update_current_hostname() @@ -374,6 +372,10 @@ class SystemdStrategy(GenericStrategy): the hostnamectl command. """ + def __init__(self, module): + super(SystemdStrategy, self).__init__(module) + self.hostname_cmd = self.module.get_bin_path('hostnamectl', True) + def get_current_hostname(self): cmd = [self.hostname_cmd, '--transient', 'status'] rc, out, err = self.module.run_command(cmd) diff --git a/test/integration/targets/hostname/aliases b/test/integration/targets/hostname/aliases new file mode 100644 index 00000000000..1580c8a5c3e --- /dev/null +++ b/test/integration/targets/hostname/aliases @@ -0,0 +1,4 @@ +shippable/posix/group1 +destructive +skip/aix # currently unsupported by hostname module +skip/osx # same, see #54439, #32214 diff --git a/test/integration/targets/hostname/tasks/RedHat.yml b/test/integration/targets/hostname/tasks/RedHat.yml new file mode 100644 index 00000000000..1f61390bf00 --- /dev/null +++ b/test/integration/targets/hostname/tasks/RedHat.yml @@ -0,0 +1,15 @@ +- name: Make sure we used SystemdStrategy... + lineinfile: + path: "{{ _hostname_file }}" + line: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: etc_hostname + failed_when: etc_hostname is changed + +- name: ...and not RedhatStrategy + lineinfile: + path: /etc/sysconfig/network + line: HOSTNAME=crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: etc_sysconfig_network + failed_when: etc_sysconfig_network is not changed diff --git a/test/integration/targets/hostname/tasks/check_mode.yml b/test/integration/targets/hostname/tasks/check_mode.yml new file mode 100644 index 00000000000..e25df97b61d --- /dev/null +++ b/test/integration/targets/hostname/tasks/check_mode.yml @@ -0,0 +1,20 @@ +# These are less useful (check_mode only) but run even in containers +- block: + - name: Get current hostname + command: hostname + register: original + + - name: Change hostname (check_mode) + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: hn + + - name: Get current hostname again + command: hostname + register: after_hn + + - assert: + that: + - hn is changed + - original.stdout == after_hn.stdout diff --git a/test/integration/targets/hostname/tasks/default.yml b/test/integration/targets/hostname/tasks/default.yml new file mode 100644 index 00000000000..b3082391c99 --- /dev/null +++ b/test/integration/targets/hostname/tasks/default.yml @@ -0,0 +1,2 @@ +- debug: + msg: No distro-specific tests defined for this distro. diff --git a/test/integration/targets/hostname/tasks/main.yml b/test/integration/targets/hostname/tasks/main.yml new file mode 100644 index 00000000000..cbb6711f237 --- /dev/null +++ b/test/integration/targets/hostname/tasks/main.yml @@ -0,0 +1,121 @@ +# Setting the hostname in our test containers doesn't work currently +- when: ansible_facts.virtualization_type != 'docker' + block: + - name: Include distribution specific variables + include_vars: "{{ lookup('first_found', params) }}" + vars: + params: + files: + - "{{ ansible_facts.distribution }}.yml" + - "{{ ansible_facts.os_family }}.yml" + - default.yml + paths: + - "{{ role_path }}/vars" + + - name: Get current hostname + command: hostname + register: original + + - name: Run hostname module in check_mode + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: hn1 + + - name: Get current hostname again + command: hostname + register: after_hn + + - assert: + that: + - hn1 is changed + - original.stdout == after_hn.stdout + + - when: _hostname_file is defined and _hostname_file + block: + - name: See if current hostname file exists + stat: + path: "{{ _hostname_file }}" + register: hn_stat + + - name: Move the current hostname file if it exists + command: mv {{ _hostname_file }} {{ _hostname_file }}.orig + when: hn_stat.stat.exists + + - name: Run hostname module in check_mode + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: hn + + - stat: + path: /etc/rc.conf.d/hostname + register: hn_stat_checkmode + + - assert: + that: + # TODO: This is a legitimate bug and will be fixed in another PR. + # - not hn_stat_checkmode.stat.exists + - hn is changed + + - name: Get hostname again + command: hostname + register: current_after_cm + + - assert: + that: + - original.stdout == current_after_cm.stdout + + - name: Run hostname module for real now + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + register: hn2 + + - name: Get hostname + command: hostname + register: current_after_hn2 + + - name: Run hostname again to ensure it is idempotent + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + register: hnidem + + - name: Get hostname + command: hostname + register: current_after_hnidem + + - assert: + that: + - hn2 is changed + - hnidem is not changed + - current_after_hn2.stdout == 'crocodile.ansible.test.doesthiswork.net.example.com' + - current_after_hn2.stdout == current_after_hnidem.stdout + + - name: Include distribution specific tasks + include_tasks: + file: "{{ lookup('first_found', files) }}" + vars: + files: + - "{{ ansible_facts.distribution }}.yml" + - default.yml + always: + # Reset back to original hostname + - name: Move back original file if it existed + command: mv -f {{ _hostname_file }}.orig {{ _hostname_file }} + when: hn_stat is defined and hn_stat.stat.exists + + - name: Delete the file if it never existed + file: + path: "{{ _hostname_file }}" + state: absent + when: hn_stat is defined and not hn_stat.stat.exists + + # Reset back to original hostname + - hostname: + name: "{{ original.stdout }}" + register: revert + + # And make sure we really do + - assert: + that: + - revert is changed diff --git a/test/integration/targets/hostname/vars/FreeBSD.yml b/test/integration/targets/hostname/vars/FreeBSD.yml new file mode 100644 index 00000000000..b63a16ed27f --- /dev/null +++ b/test/integration/targets/hostname/vars/FreeBSD.yml @@ -0,0 +1 @@ +_hostname_file: /etc/rc.conf.d/hostname diff --git a/test/integration/targets/hostname/vars/RedHat.yml b/test/integration/targets/hostname/vars/RedHat.yml new file mode 100644 index 00000000000..08d883b9e31 --- /dev/null +++ b/test/integration/targets/hostname/vars/RedHat.yml @@ -0,0 +1 @@ +_hostname_file: /etc/hostname diff --git a/test/integration/targets/hostname/vars/default.yml b/test/integration/targets/hostname/vars/default.yml new file mode 100644 index 00000000000..a50b3f19251 --- /dev/null +++ b/test/integration/targets/hostname/vars/default.yml @@ -0,0 +1 @@ +_hostname_file: ~