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

Co-authored-by: Sam Doran <sdoran@redhat.com>

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.
+            - 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']
+        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)
+        self.hostname_cmd = self.module.get_bin_path('hostname', True)
-        if not self.hostname_cmd:
-            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/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

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/default.yml

@@ -0,0 +1,2 @@
+- debug:
+    msg: No distro-specific tests defined for this distro.

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

test/integration/targets/hostname/vars/FreeBSD.yml

@@ -0,0 +1 @@
+_hostname_file: /etc/rc.conf.d/hostname

test/integration/targets/hostname/vars/RedHat.yml

@@ -0,0 +1 @@
+_hostname_file: /etc/hostname

test/integration/targets/hostname/vars/default.yml

@@ -0,0 +1 @@
+_hostname_file: ~