most remarks fixed

Author: Markus Koch

playbooks/README.md

@@ -0,0 +1,8 @@
+
+# List of playbooks
+
+- prepare-for-hana
+- prepare-for-netweaver
+- install-sap-hana
+- install-sap-hana-cluster
+- install-sap-hana-s4

roles/update_etc_hosts/README.md

@@ -1,7 +1,7 @@
 Role Name
 =========
 
-This role can be used to reliably update the /etc/hosts file
+This role can be used to reliably update the /etc/hosts file.
 
 <!---
 Requirements
@@ -13,7 +13,8 @@ Requirements
 Role Variables
 --------------
 
-This role needs a a dictonary `update_etc_hosts_list` which contains the parameters for the hostfile
+This role needs a a dictonary `update_etc_hosts_list` which contains the parameters for the hostfile.
+It can be used for adding and deleting entries from `/etc/hosts`
 
 <!---
 Dependencies
@@ -43,6 +44,7 @@ If you want to setup/add entries your etc hosts you can use this snippet
                   node_name: host2
                   state: absent
 ```
+
 If you have defined a cluster and the variable `sap_ha_pacemaker_cluster_cluster_nodes` or `sap_hana_cluster_nodes` is set, you can use the following play:
 
 ```[yaml]

roles/update_etc_hosts/meta/main.yml

@@ -7,11 +7,12 @@ galaxy_info:
 
   license: Apache-2.0
 
-  min_ansible_version: "2.12"
+  min_ansible_version: "2.11"
 
   platforms:
     - name: "EL"
       versions:
+        - "7"
         - "8"
         - "9"
     - name: "SLES"

roles/update_etc_hosts/tasks/main.yml

@@ -1,5 +1,18 @@
 ---
 # tasks file for update_etc_hosts
+- name: Ensure required defaults are read from setup module
+  when: >
+    not ((ansible_hostname is defined) and
+    (ansible_domain is defined) and
+    (ansible_default_ipv4 is defined))
+  ansible.builtin.setup:
+    filter:
+      - 'ansible_hostname'
+      - 'ansible_domain'
+      - 'ansible_default_ipv4'
+
+# TODO: Double Check that list entries with state present do not have duplicate IP adresses
+
 - name: Add entry to "{{ __update_etc_hosts_file }}"
   loop: "{{ update_etc_hosts_list }}"
   loop_control:

roles/update_etc_hosts/tasks/update_host_absent.yml

@@ -14,6 +14,8 @@
   when:
     - thishost.node_ip is defined
     - not ansible_check_mode
+  become_user: root
+  become: true
 
 - name: Ensure that the entry all enries in hosts file are removed with name {{ thishost.node_name | d('undefined') }}
   ansible.builtin.lineinfile:
@@ -25,6 +27,8 @@
     - thishost.node_name is defined
     - thishost.node_domain is undefined
     - not ansible_check_mode
+  become_user: root
+  become: true
 
 - name: Ensure that the entry all enries in hosts file are removed with FQDN
   ansible.builtin.lineinfile:
@@ -36,3 +40,5 @@
     - thishost.node_name is defined
     - thishost.node_domain is defined
     - not ansible_check_mode
+  become_user: root
+  become: true

roles/update_etc_hosts/tasks/update_host_present.yml

@@ -1,9 +1,8 @@
----
-# TODO CHECK/SET Variables
+--- # trunk-ignore(checkov/CKV2_ANSIBLE_3)
 - name: Verify that variable node_ip is set
   ansible.builtin.assert:
     that: not( ( thishost.node_ip is undefined) or ( thishost.node_ip is none) or ( thishost.node_ip | trim == '') )
-    msg: "Variable 'node_ip' is undefined or empty. Please define it your host list"
+    msg: "Variable 'node_ip' is undefined or empty. Please define it in your host list."
 
 - name: Verify that variable node_ip is in the correct format
   ansible.builtin.assert:
@@ -17,15 +16,16 @@
 
 - name: Ensure node_domain is set
   ansible.builtin.set_fact:
-    __update_etc_hosts_domain: "{{ thishost.node_domain | default(sap_domain | default(ansible_domain)) }}"
+    __update_etc_hosts_domain: "{{ thishost.node_domain | default(sap_domain) | default(ansible_domain) }}"
 
+# Necessary, if defaults are both undefined
 - name: Verify that variable domain_name is set
   ansible.builtin.assert:
     that: >
       not( ( __update_etc_hosts_domain is undefined) or
       ( __update_etc_hosts_domain is none) or
       ( __update_etc_hosts_domain | trim == '') )
-    msg: "Variable 'node_name' is undefined or empty. Please define it your host list"
+    msg: "Variable 'domain_name' is undefined or empty. Please define it your host list"
 
 - name: Set default values
   ansible.builtin.set_fact:
@@ -37,6 +37,17 @@
   ansible.builtin.set_fact:
     __update_etc_hosts_comment: "# {{ __update_etc_hosts_comment }}"
 
+# The following block reads the existing aliases of a host from /etc/hosts
+# and merges it with the defined aliases in the struct
+#
+# 1. select the line, where the first entry is the ip-adress thishost.node_ip
+# 2. loop over all hostname entries in the selected line (2 bis NF=last element in line)
+# 3. stop looping when a comment sign is found (because these are comments)
+# 4. print an element, if it is not the hostname or FQDN we want to add
+#
+# => __update_etc_hosts_register_aliases.stdout contains a list of aliases of thishost.node_ip
+#
+# trunk-ignore(checkov/CKV2_ANSIBLE_3)
 - name: Merge existing aliases with new alis list
   when: __update_etc_hosts_alias_mode != "overwrite"
   block:
@@ -50,6 +61,7 @@
         }' "{{ __update_etc_hosts_file }}"
       register: __update_etc_hosts_register_aliases
       changed_when: false
+
     - name: Add defined aliases
       ansible.builtin.set_fact:
         __update_etc_hosts_aliases: "{{ (__update_etc_hosts_register_aliases.stdout.split(' ') + thishost.aliases | d([])) | unique | join(' ') }}"
@@ -101,6 +113,8 @@
   when:
     - not ansible_check_mode
     - __update_etc_hosts_register_duplicate_ip_check.rc == 0
+  become_user: root
+  become: true
 
 - name: Check for duplicate or missing entries of hostname and fqdn in {{ __update_etc_hosts_file }}
   ansible.builtin.shell: |
@@ -111,8 +125,8 @@
       exit 1
     fi
   loop:
-    - '{{ thishost.node_name }}.{{ __update_etc_hosts_domain }}'
-    - '{{ thishost.node_name }}'
+    - "{{ thishost.node_name }}.{{ __update_etc_hosts_domain }}"
+    - "{{ thishost.node_name }}"
   changed_when: false
   loop_control:
     loop_var: line_item
@@ -127,13 +141,14 @@
 
     - name: Display the output of the hosts file completeness check
       ansible.builtin.debug:
-        var: __update_etc_hosts_register_ipv4_fqdn_sap_hostname_once_check.stdout_lines,
-              __update_etc_hosts_register_ipv4_fqdn_sap_hostname_once_check.stderr_lines
+        var:
+          __update_etc_hosts_register_ipv4_fqdn_sap_hostname_once_check.stdout_lines,
+          __update_etc_hosts_register_ipv4_fqdn_sap_hostname_once_check.stderr_lines
 
     - name: Display the expected output of the hosts file completeness check
       ansible.builtin.debug:
         msg:
-          - "Expected:"
+          - "The minimum expected entries in {{ __update_etc_hosts_file }} are:"
           - "{{ thishost.node_ip }} {{ thishost.node_name }}.{{ __update_etc_hosts_domain }} {{ thishost.node_name }}"
       when:
         - __update_etc_hosts_register_ipv4_fqdn_sap_hostname_once_check.stdout != "1"

roles/update_etc_hosts/tests/test.yml

@@ -1,23 +1,23 @@
 ---
 # you can run 'ansible-playbook test.yml -e __update_etc_hosts_file=./test.hosts' for testing this role
-- name: test play
+- name: Test play
   hosts: localhost
   remote_user: root
   become: true
 
   vars:
-   update_etc_hosts_list:
-              - node_ip: 1.2.3.4
-                node_name: host1
-                node_domain: abc.de
-                aliases:
-                  - alias1
-                  - anotheralias2
-                node_comment: "Here comes text after hashsign"
-                state: present
-              - node_ip: 1.2.3.5
-                node_name: host2
-                state: absent
+    update_etc_hosts_list:
+      - node_ip: 1.2.3.4
+        node_name: host1
+        node_domain: abc.de
+        aliases:
+          - alias1
+          - anotheralias2
+        node_comment: "Here comes text after hashsign"
+        state: present
+      - node_ip: 1.2.3.5
+        node_name: host2
+        state: absent
 
   roles:
     - update_etc_hosts