hostname: hostnamectl check -> SystemdStrategy

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 <rick@elrod.me>

lib/ansible/modules/hostname.py

@@ -20,7 +20,7 @@
 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 @@
     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, especially 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)

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

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

test/integration/targets/hostname/tasks/freebsd.yml

@@ -0,0 +1,75 @@
+- block:
+    - name: Get current hostname
+      command: hostname
+      register: original
+
+    - name: See if current hostname file exists
+      stat:
+        path: /etc/rc.conf.d/hostname
+      register: hn_stat
+
+    - name: Move the current hostname file if it exists
+      command: mv /etc/rc.conf.d/hostname /etc/rc.conf.d/hostname.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: hostname1
+
+    - name: Get hostname
+      command: hostname
+      register: current_after_hostname1
+
+    - name: Run hostname again to ensure it is idempotent
+      hostname:
+        name: crocodile.ansible.test.doesthiswork.net.example.com
+      register: hostname2
+
+    - name: Get hostname
+      command: hostname
+      register: current_after_hostname2
+
+    - assert:
+        that:
+          - hostname1 is changed
+          - hostname2 is not changed
+          - current_after_hostname1.stdout == 'crocodile.ansible.test.doesthiswork.net.example.com'
+          - current_after_hostname1.stdout == current_after_hostname2.stdout
+  always:
+    # Reset back to original hostname
+    - name: Move back original file if it existed
+      command: mv -f /etc/rc.conf.d/hostname.orig /etc/rc.conf.d/hostname
+      when: hn_stat.stat.exists
+
+    - hostname:
+        name: "{{ original.stdout }}"
+      register: revert
+
+    - assert:
+        that:
+          - revert is changed

test/integration/targets/hostname/tasks/hostname.yml

@@ -0,0 +1,40 @@
+- set_fact:
+    hostname_file: /etc/hostname
+  when: ansible_os_family == 'Debian'
+
+- set_fact:
+    hostname_file: /etc/rc.conf.d/hostname
+  when: ansible_os_family == 'FreeBSD'
+
+- hostname:
+    name: foobartest
+
+#- block:
+#    - name: Move any existing hostname file aside
+#      command: mv {{ hostname_file }} {{ hostname_file }}.orig
+#      # file might not exist
+#      ignore_errors: true
+#
+#    - name: Ensure no file is created in check_mode when the OS uses files for storing it
+#      hostname:
+#        name: crocodile.ansible.test.doesthiswork.net.example.com
+#      check_mode: true
+#      register: hn_checkmode
+#
+#    - name: Stat the file
+#      stat:
+#        path: "{{ hostname_file }}"
+#      register:
+#        hn_stat
+#
+#    - assert:
+#        that:
+#          - not hn_stat.stat.exists
+#  always:
+#    - name: Move original hostname file back if it existed
+#      command: mv {{ hostname_file }}.orig {{ hostname_file }}
+#      # file might not exist
+#      ignore_errors: true
+#
+#  # Only run these tasks on systems that we know _might_ create files.
+#  when: hostname_file is defined

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

@@ -0,0 +1,15 @@
+# We can't change hostnames in containers, so limit it to systems where
+# we get a full VM. Then realize:
+# intersection(
+#   systems with full VM in CI,
+#   systems hostname module currently supports) == just RHEL and FreeBSD
+
+- include_tasks: rhel.yml
+  when: ansible_distribution == 'RedHat'
+
+- include_tasks: freebsd.yml
+  when: ansible_distribution == 'FreeBSD'
+
+# Even check_mode fails in a container. :-(
+- import_tasks: check_mode.yml
+  when: ansible_distribution in ['RedHat', 'FreeBSD']

test/integration/targets/hostname/tasks/rhel.yml

@@ -0,0 +1,79 @@
+- block:
+    - 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
+
+    - name: Make sure no file was altered
+      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
+
+    - name: Ensure hostname was not altered either
+      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: hostname1
+
+    - name: Get hostname
+      command: hostname
+      register: current_after_hostname1
+
+    - name: Run hostname again to ensure it is idempotent
+      hostname:
+        name: crocodile.ansible.test.doesthiswork.net.example.com
+      register: hostname2
+
+    - name: Get hostname
+      command: hostname
+      register: current_after_hostname2
+
+    - assert:
+        that:
+          - hostname1 is changed
+          - hostname2 is not changed
+          - current_after_hostname1.stdout == 'crocodile.ansible.test.doesthiswork.net.example.com'
+          - current_after_hostname1.stdout == current_after_hostname2.stdout
+
+    - name: Make sure we used SystemdStrategy...
+      lineinfile:
+        path: /etc/hostname
+        line: crocodile.ansible.test.doesthiswork.net.example.com
+      check_mode: true
+      register: etc_hostname
+
+    - 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
+
+    - assert:
+        that:
+          - etc_hostname is not changed
+          - etc_sysconfig_network is changed
+  always:
+    # Reset back to original hostname
+    - hostname:
+        name: "{{ original.stdout }}"
+      register: revert
+
+    # And make sure we really do
+    - assert:
+        that:
+          - revert is changed