hostname: hostnamectl check -> SystemdStrategy

[relrod]

SUMMARY

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

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

hostname

[samdoran]

Code changes look good. I think we could refactor the tests a bit, but that's not a blocker to merging this.

[samdoran]

The block can be moved into the main.yml tasks.

[samdoran]

I think we could put most of the distribution specific tasks in main.yml since they are 90% the same. We just need to add files in vars for each distribution that sets _hostname_file to the correct value.

With conditionals on blocks, I find it's best from a readability perspective to put it at the beginning of the block rather than the end.

Suggested change

	- 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']
	# 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', files) }}"
	vars:
	files:
	- "{{ ansible_facts.distribution }}.yml"
	- "{{ ansible_facts.os_family }}.yml"
	- default.yml
	- 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: Get current hostname again
	command: hostname
	register: after_hn
	- assert:
	that:
	- hn is changed
	- original.stdout == after_hn.stdout
	- 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: 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: Include distribution specific tasks
	include_tasks:
	file: "{{ lookup('first_found', files) }}"
	vars:
	files:
	- "{{ ansible_facts.distribution }}.yml"
	- none.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.stat.exists
	- name: Delete the file if it never existed
	file:
	path: "{{ _hostname_file }}"
	state: absent
	when: 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