Merge pull request #2818 from mtnbikenc/package-refactor

Refactor to use Ansible package module

docs/best_practices_guide.adoc

@@ -2,7 +2,7 @@
 
 = openshift-ansible Best Practices Guide
 
-The purpose of this guide is to describe the preferred patterns and best practices used in this repository (both in ansible and python).
+The purpose of this guide is to describe the preferred patterns and best practices used in this repository (both in Ansible and Python).
 
 It is important to note that this repository may not currently comply with all best practices, but the intention is that it will.
 
@@ -76,7 +76,7 @@ def add_person(first_name, last_name, age=None):
 
 
 === PyLint
-http://www.pylint.org/[PyLint] is used in an attempt to keep the python code as clean and as manageable as possible. The build bot runs each pull request through PyLint and any warnings or errors cause the build bot to fail the pull request.
+http://www.pylint.org/[PyLint] is used in an attempt to keep the Python code as clean and as manageable as possible. The build bot runs each pull request through PyLint and any warnings or errors cause the build bot to fail the pull request.
 
 '''
 [[PyLint-rules-MUST-NOT-be-disabled-on-a-whole-file]]
@@ -338,9 +338,9 @@ If an Ansible role requires certain variables to be set, it's best to check for
 [cols="2v,v"]
 |===
 | <<Ansible-tasks-SHOULD-NOT-be-used-in-ansible-playbooks-Instead-use-pre_tasks-and-post_tasks, Rule>>
-| Ansible tasks SHOULD NOT be used in ansible playbooks. Instead, use pre_tasks and post_tasks.
+| Ansible tasks SHOULD NOT be used in Ansible playbooks. Instead, use pre_tasks and post_tasks.
 |===
-An Ansible play is defined as a Yaml dictionary. Because of that, ansible doesn't know if the play's tasks list or roles list was specified first. Therefore Ansible always runs tasks after roles.
+An Ansible play is defined as a Yaml dictionary. Because of that, Ansible doesn't know if the play's tasks list or roles list was specified first. Therefore Ansible always runs tasks after roles.
 
 This can be quite confusing if the tasks list is defined in the playbook before the roles list because people assume in order execution in Ansible.
 
@@ -484,31 +484,23 @@ If you want to use default with variables that evaluate to false you have to set
 ----
 
 
-In other words, normally the `default` filter will only replace the value if it's undefined. By setting the second parameter to `true`, it will also replace the value if it defaults to a false value in python, so None, empty list, empty string, etc.
+In other words, normally the `default` filter will only replace the value if it's undefined. By setting the second parameter to `true`, it will also replace the value if it defaults to a false value in Python, so None, empty list, empty string, etc.
 
 This is almost always more desirable than an empty list, string, etc.
 
 === Yum and DNF
 '''
-[[Package-installation-MUST-use-ansible-action-module-to-abstract-away-dnf-yum]]
+[[Package-installation-MUST-use-ansible-package-module-to-abstract-away-dnf-yum]]
 [cols="2v,v"]
 |===
-| <<Package-installation-MUST-use-ansible-action-module-to-abstract-away-dnf-yum, Rule>>
-| Package installation MUST use ansible action module to abstract away dnf/yum.
+| <<Package-installation-MUST-use-ansible-package-module-to-abstract-away-dnf-yum, Rule>>
+| Package installation MUST use Ansible `package` module to abstract away dnf/yum.
 |===
 
-[[Package-installation-MUST-use-name-and-state-present-rather-than-pkg-and-state-installed-respectively]]
-[cols="2v,v"]
-|===
-| <<Package-installation-MUST-use-name-and-state-present-rather-than-pkg-and-state-installed-respectively, Rule>>
-| Package installation MUST use name= and state=present rather than pkg= and state=installed respectively.
-|===
+The Ansible `package` module calls the associated package manager for the underlying OS.
 
-This is done primarily because if you're registering the result of the
-installation and you have two conditional tasks based on whether or not yum or
-dnf are in use you'll end up inadvertently overwriting the value. It also
-reduces duplication. name= and state=present are common between dnf and yum
-modules.
+.Reference
+* https://docs.ansible.com/ansible/package_module.html[Ansible package module]
 
 .Bad:
 [source,yaml]
@@ -516,12 +508,12 @@ modules.
 ---
 # tasks.yml
 - name: Install etcd (for etcdctl)
-  yum: name=etcd state=latest"
+  yum: name=etcd state=latest
   when: "ansible_pkg_mgr == yum"
   register: install_result
 
 - name: Install etcd (for etcdctl)
-  dnf: name=etcd state=latest"
+  dnf: name=etcd state=latest
   when: "ansible_pkg_mgr == dnf"
   register: install_result
 ----
@@ -533,6 +525,6 @@ modules.
 ---
 # tasks.yml
 - name: Install etcd (for etcdctl)
-  action: "{{ ansible_pkg_mgr }} name=etcd state=latest"
+  package: name=etcd state=latest
   register: install_result
-  ----
+----

playbooks/adhoc/uninstall.yml

@@ -84,7 +84,7 @@
     - firewalld
 
   - name: Remove packages
-    action: "{{ ansible_pkg_mgr }} name={{ item }} state=absent"
+    package: name={{ item }} state=absent
     when: not is_atomic | bool
     with_items:
     - atomic-enterprise
@@ -114,7 +114,7 @@
     - tuned-profiles-origin-node
 
   - name: Remove flannel package
-    action: "{{ ansible_pkg_mgr }} name=flannel state=absent"
+    package: name=flannel state=absent
     when: openshift_use_flannel | default(false) | bool and not is_atomic | bool
 
   - shell: systemctl reset-failed
@@ -247,7 +247,7 @@
     - atomic-openshift-master
 
   - name: Remove packages
-    action: "{{ ansible_pkg_mgr }} name={{ item }} state=absent"
+    package: name={{ item }} state=absent
     when: not is_atomic | bool
     with_items:
     - atomic-enterprise
@@ -349,7 +349,7 @@
     failed_when: false
 
   - name: Remove packages
-    action: "{{ ansible_pkg_mgr }} name={{ item }} state=absent"
+    package: name={{ item }} state=absent
     when: not is_atomic | bool
     with_items:
     - etcd
@@ -388,7 +388,7 @@
     - firewalld
 
   - name: Remove packages
-    action: "{{ ansible_pkg_mgr }} name={{ item }} state=absent"
+    package: name={{ item }} state=absent
     when: not is_atomic | bool
     with_items:
     - haproxy

playbooks/common/openshift-cluster/upgrades/docker/upgrade.yml

@@ -35,7 +35,7 @@
 - service: name=docker state=stopped
 
 - name: Upgrade Docker
-  action: "{{ ansible_pkg_mgr }} name=docker{{ '-' + docker_version }} state=present"
+  package: name=docker{{ '-' + docker_version }} state=present
 
 - service: name=docker state=started
 

playbooks/common/openshift-cluster/upgrades/etcd/backup.yml

@@ -42,7 +42,7 @@
     when: (embedded_etcd | bool) and (etcd_disk_usage.stdout|int > avail_disk.stdout|int)
 
   - name: Install etcd (for etcdctl)
-    action: "{{ ansible_pkg_mgr }} name=etcd state=present"
+    package: name=etcd state=present
     when: not openshift.common.is_atomic | bool
 
   - name: Generate etcd backup

playbooks/common/openshift-cluster/upgrades/etcd/containerized_tasks.yml

@@ -30,7 +30,7 @@
 ## will fail on atomic host. We need to revisit how to do etcd backups there as
 ## the container may be newer than etcdctl on the host. Assumes etcd3 obsoletes etcd (7.3.1)
 - name: Upgrade etcd for etcdctl when not atomic
-  action: "{{ ansible_pkg_mgr }} name=etcd ensure=latest"
+  package: name=etcd state=latest
   when: not openshift.common.is_atomic | bool
 
 - name: Verify cluster is healthy

playbooks/common/openshift-cluster/upgrades/rpm_upgrade.yml

@@ -1,9 +1,10 @@
+---
 # We verified latest rpm available is suitable, so just yum update.
 - name: Upgrade packages
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}-{{ component }}{{ openshift_pkg_version }} state=present"
+  package: "name={{ openshift.common.service_type }}-{{ component }}{{ openshift_pkg_version }} state=present"
 
 - name: Ensure python-yaml present for config upgrade
-  action: "{{ ansible_pkg_mgr }} name=PyYAML state=present"
+  package: name=PyYAML state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Restart node service

roles/cockpit/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install cockpit-ws
-  action: "{{ ansible_pkg_mgr }} name={{ item }} state=present"
+  package: name={{ item }} state=present
   with_items:
     - cockpit-ws
     - cockpit-shell

roles/dns/tasks/main.yml

@@ -1,5 +1,6 @@
+---
 - name: Install Bind
-  action: "{{ ansible_pkg_mgr }} name=bind"
+  package: name=bind state=present
   when: not openshift.common.is_containerized | bool
 
 - name: Create docker build dir

roles/docker/tasks/main.yml

@@ -40,7 +40,7 @@
 # Make sure Docker is installed, but does not update a running version.
 # Docker upgrades are handled by a separate playbook.
 - name: Install Docker
-  action: "{{ ansible_pkg_mgr }} name=docker{{ '-' + docker_version if docker_version is defined else '' }} state=present"
+  package: name=docker{{ '-' + docker_version if docker_version is defined else '' }} state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Ensure docker.service.d directory exists

roles/etcd/tasks/etcdctl.yml

@@ -1,5 +1,6 @@
+---
 - name: Install etcd for etcdctl
-  action: "{{ ansible_pkg_mgr }} name=etcd state=present"
+  package: name=etcd state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Configure etcd profile.d alises

roles/etcd/tasks/main.yml

@@ -7,7 +7,7 @@
     etcd_ip: "{{ etcd_ip }}"
 
 - name: Install etcd
-  action: "{{ ansible_pkg_mgr }} name=etcd state=present"
+  package: name=etcd state=present
   when: not etcd_is_containerized | bool
 
 - name: Pull etcd container

roles/etcd_ca/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install openssl
-  action: "{{ ansible_pkg_mgr }} name=openssl state=present"
+  package: name=openssl state=present
   when: not etcd_is_atomic | bool
   delegate_to: "{{ etcd_ca_host }}"
   run_once: true

roles/etcd_server_certificates/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install etcd
-  action: "{{ ansible_pkg_mgr }} name=etcd state=present"
+  package: name=etcd state=present
   when: not etcd_is_containerized | bool
 
 - name: Check status of etcd certificates

roles/flannel/tasks/main.yml

@@ -1,7 +1,7 @@
 ---
 - name: Install flannel
   become: yes
-  action: "{{ ansible_pkg_mgr }} name=flannel state=present"
+  package: name=flannel state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Set flannel etcd options

roles/kube_nfs_volumes/tasks/main.yml

@@ -4,7 +4,10 @@
   when: openshift.common.is_atomic | bool
 
 - name: Install pyparted (RedHat/Fedora)
-  action: "{{ ansible_pkg_mgr }} name=pyparted,python-httplib2 state=present"
+  package: name={{ item }} state=present
+  with_items:
+    - pyparted
+    - python-httplib2
   when: not openshift.common.is_containerized | bool
 
 - name: partition the drives

roles/kube_nfs_volumes/tasks/nfs.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install NFS server
-  action: "{{ ansible_pkg_mgr }} name=nfs-utils state=present"
+  package: name=nfs-utils state=present
   when: not openshift.common.is_containerized | bool
 
 - name: Start rpcbind on Fedora/Red Hat

roles/nickhammond.logrotate/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: nickhammond.logrotate | Install logrotate
-  action: "{{ ansible_pkg_mgr }} name=logrotate state=present"
+  package: name=logrotate state=present
   when: not openshift.common.is_atomic | bool
 
 - name: nickhammond.logrotate | Setup logrotate.d scripts

roles/nuage_ca/tasks/main.yaml

@@ -1,6 +1,6 @@
 ---
 - name: Install openssl
-  action: "{{ ansible_pkg_mgr }} name=openssl state=present"
+  package: name=openssl state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Create CA directory
@@ -41,6 +41,6 @@
   delegate_to: "{{ nuage_ca_master }}"
 
 - name: Copy SSL config file
-  copy: src=openssl.cnf dest="{{ nuage_ca_dir }}/openssl.cnf" 
+  copy: src=openssl.cnf dest="{{ nuage_ca_dir }}/openssl.cnf"
   run_once: true
   delegate_to: "{{ nuage_ca_master }}"

roles/openshift_ca/tasks/main.yml

@@ -8,7 +8,9 @@
   when: openshift_master_ca_certificate is defined and ('certfile' not in openshift_master_ca_certificate or 'keyfile' not in openshift_master_ca_certificate)
 
 - name: Install the base package for admin tooling
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
+  package:
+    name: "{{ openshift.common.service_type }}{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }}"
+    state: present
   when: not openshift.common.is_containerized | bool
   register: install_result
   delegate_to: "{{ openshift_ca_host }}"

roles/openshift_cli/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install clients
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}-clients state=present"
+  package: name={{ openshift.common.service_type }}-clients state=present
   when: not openshift.common.is_containerized | bool
 
 - name: Pull CLI Image
@@ -20,5 +20,5 @@
   openshift_facts:
 
 - name: Install bash completion for oc tools
-  action: "{{ ansible_pkg_mgr }} name=bash-completion state=present"
+  package: name=bash-completion state=present
   when: not openshift.common.is_containerized | bool

roles/openshift_clock/tasks/main.yaml

@@ -6,7 +6,7 @@
       enabled: "{{ openshift_clock_enabled | default(None) }}"
 
 - name: Install ntp package
-  action: "{{ ansible_pkg_mgr }} name=ntp state=present"
+  package: name=ntp state=present
   when: openshift.clock.enabled | bool and not openshift.clock.chrony_installed | bool
 
 - name: Start and enable ntpd/chronyd

roles/openshift_common/tasks/main.yml

@@ -29,7 +29,9 @@
       use_dnsmasq: "{{ openshift_use_dnsmasq | default(None) }}"
 
 - name: Install the base package for versioning
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
+  package:
+    name: "{{ openshift.common.service_type }}{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }}"
+    state: present
   when: not openshift.common.is_containerized | bool
 
 - name: Set version facts

roles/openshift_expand_partition/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Ensure growpart is installed
-  action: "{{ ansible_pkg_mgr }} name=cloud-utils-growpart state=present"
+  package: name=cloud-utils-growpart state=present
   when: not openshift.common.is_containerized | bool
 
 - name: Determine if growpart is installed

roles/openshift_facts/tasks/main.yml

@@ -10,12 +10,11 @@
 - set_fact:
     l_is_containerized: "{{ (l_is_atomic | bool) or (containerized | default(false) | bool) }}"
 
-- name: Ensure PyYaml is installed
-  action: "{{ ansible_pkg_mgr }} name=PyYAML state=present"
-  when: not l_is_atomic | bool
-
-- name: Ensure yum-utils is installed
-  action: "{{ ansible_pkg_mgr }} name=yum-utils state=present"
+- name: Ensure PyYaml and yum-utils are installed
+  package: name={{ item }} state=present
+  with_items:
+    - PyYAML
+    - yum-utils
   when: not l_is_atomic | bool
 
 - name: Gather Cluster facts and set is_containerized if needed

roles/openshift_loadbalancer/tasks/main.yml

@@ -3,7 +3,7 @@
   when: openshift.common.is_containerized | bool
 
 - name: Install haproxy
-  action: "{{ ansible_pkg_mgr }} name=haproxy state=present"
+  package: name=haproxy state=present
 
 - name: Configure systemd service directory for haproxy
   file:

roles/openshift_master/tasks/main.yml

@@ -24,7 +24,9 @@
   when: openshift_master_ha | bool and openshift_master_cluster_method == "pacemaker" and openshift.common.is_containerized | bool
 
 - name: Install Master package
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}-master{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
+  package:
+    name: "{{ openshift.common.service_type }}-master{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }}"
+    state: present
   when: not openshift.common.is_containerized | bool
 
 - name: Pull master image
@@ -77,7 +79,7 @@
   - restart master controllers
 
 - name: Install httpd-tools if needed
-  action: "{{ ansible_pkg_mgr }} name=httpd-tools state=present"
+  package: name=httpd-tools state=present
   when: (item.kind == 'HTPasswdPasswordIdentityProvider') and
         not openshift.common.is_atomic | bool
   with_items: "{{ openshift.master.identity_providers }}"
@@ -292,7 +294,7 @@
   when: openshift_master_ha | bool and openshift.master.cluster_method == 'native'
 
 - name: Install cluster packages
-  action: "{{ ansible_pkg_mgr }} name=pcs state=present"
+  package: name=pcs state=present
   when: openshift_master_ha | bool and openshift.master.cluster_method == 'pacemaker'
     and not openshift.common.is_containerized | bool
   register: install_result

roles/openshift_node/tasks/main.yml

@@ -35,7 +35,9 @@
 # We have to add tuned-profiles in the same transaction otherwise we run into depsolving
 # problems because the rpms don't pin the version properly. This was fixed in 3.1 packaging.
 - name: Install Node package
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}-node{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }},tuned-profiles-{{ openshift.common.service_type }}-node{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
+  package:
+    name: "{{ openshift.common.service_type }}-node{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }},tuned-profiles-{{ openshift.common.service_type }}-node{{ openshift_pkg_version | default('') | oo_image_tag_to_rpm_version(include_dash=True) }}"
+    state: present
   when: not openshift.common.is_containerized | bool
 
 - name: Check for tuned package
@@ -49,7 +51,9 @@
   when: tuned_installed.rc == 0 and openshift.common.is_atomic | bool
 
 - name: Install sdn-ovs package
-  action: "{{ ansible_pkg_mgr }} name={{ openshift.common.service_type }}-sdn-ovs{{ openshift_pkg_version | oo_image_tag_to_rpm_version(include_dash=True) }} state=present"
+  package:
+    name: "{{ openshift.common.service_type }}-sdn-ovs{{ openshift_pkg_version | oo_image_tag_to_rpm_version(include_dash=True) }}"
+    state: present
   when: openshift.common.use_openshift_sdn and not openshift.common.is_containerized | bool
 
 - name: Pull node image

roles/openshift_node/tasks/storage_plugins/ceph.yml

@@ -1,4 +1,4 @@
 ---
 - name: Install Ceph storage plugin dependencies
-  action: "{{ ansible_pkg_mgr }} name=ceph-common state=present"
-  when: not openshift.common.is_atomic | bool
+  package: name=ceph-common state=present
+  when: not openshift.common.is_atomic | bool

roles/openshift_node/tasks/storage_plugins/glusterfs.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install GlusterFS storage plugin dependencies
-  action: "{{ ansible_pkg_mgr }} name=glusterfs-fuse state=present"
+  package: name=glusterfs-fuse state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Check for existence of virt_use_fusefs seboolean

roles/openshift_node/tasks/storage_plugins/iscsi.yml

@@ -1,4 +1,4 @@
 ---
 - name: Install iSCSI storage plugin dependencies
-  action: "{{ ansible_pkg_mgr }} name=iscsi-initiator-utils state=present"
+  package: name=iscsi-initiator-utils state=present
   when: not openshift.common.is_atomic | bool

roles/openshift_node/tasks/storage_plugins/nfs.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install NFS storage plugin dependencies
-  action: "{{ ansible_pkg_mgr }} name=nfs-utils state=present"
+  package: name=nfs-utils state=present
   when: not openshift.common.is_atomic | bool
 
 - name: Check for existence of seboolean

roles/openshift_node_dnsmasq/tasks/main.yml

@@ -10,7 +10,7 @@
     network_manager_active: "{{ True if 'ActiveState=active' in nm_show.stdout else False }}"
 
 - name: Install dnsmasq
-  action: "{{ ansible_pkg_mgr }} name=dnsmasq state=installed"
+  package: name=dnsmasq state=installed
   when: not openshift.common.is_atomic | bool
 
 - name: Install dnsmasq configuration

roles/openshift_repos/tasks/main.yaml

@@ -12,7 +12,7 @@
   when: not openshift.common.is_containerized | bool
 
 - name: Ensure libselinux-python is installed
-  action: "{{ ansible_pkg_mgr }} name=libselinux-python state=present"
+  package: name=libselinux-python state=present
   when: not openshift.common.is_containerized | bool
 
 - name: Create any additional repos that are defined

roles/openshift_storage_nfs/tasks/main.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install nfs-utils
-  action: "{{ ansible_pkg_mgr }} name=nfs-utils state=present"
+  package: name=nfs-utils state=present
 
 - name: Configure NFS
   lineinfile:

roles/openshift_storage_nfs_lvm/tasks/nfs.yml

@@ -1,8 +1,8 @@
 ---
 - name: Install NFS server
-  action: "{{ ansible_pkg_mgr }} name=nfs-utils state=present"
+  package: name=nfs-utils state=present
   when: not openshift.common.is_containerized | bool
-  
+
 - name: Start rpcbind
   service: name=rpcbind state=started enabled=yes
 

roles/os_firewall/tasks/firewall/firewalld.yml

@@ -1,6 +1,6 @@
 ---
 - name: Install firewalld packages
-  action: "{{ ansible_pkg_mgr }} name=firewalld state=present"
+  package: name=firewalld state=present
   when: not openshift.common.is_containerized | bool
   register: install_result
 

roles/os_firewall/tasks/firewall/iptables.yml

@@ -25,7 +25,7 @@
   ignore_errors: yes
 
 - name: Install iptables packages
-  action: "{{ ansible_pkg_mgr }} name={{ item }} state=present"
+  package: name={{ item }} state=present
   with_items:
   - iptables
   - iptables-services

roles/os_update_latest/tasks/main.yml

@@ -1,3 +1,3 @@
 ---
 - name: Update all packages
-  action: "{{ ansible_pkg_mgr }} name=* state=latest"
+  package: name=* state=latest