From 76db060afb071cd860fd0ca1161e0c3c9b71cf88 Mon Sep 17 00:00:00 2001
From: MarkusTeufelberger <mteufelberger@mgit.at>
Date: Wed, 24 Apr 2019 00:46:02 +0200
Subject: [PATCH] Define and implement specs for bootstrap-os (#4455)

* Add README to bootstrap-os role

* Rework bootstrap-os once more

* Document workarounds for bugs/deficiencies in Ansible modules
* Unify and document role variables
* Remove installation of additional packages and repositories
* Merge Ubuntu and Debian tasks
* Remove pipelining setting from default playbooks
* Fix OpenSUSE not running its required tasks
---
 cluster.yml                                   |  4 -
 roles/bootstrap-os/README.md                  | 59 ++++++++++++++
 roles/bootstrap-os/defaults/main.yml          | 24 +++---
 roles/bootstrap-os/tasks/bootstrap-centos.yml | 76 +++++--------------
 .../tasks/bootstrap-clearlinux.yml            | 14 ++--
 roles/bootstrap-os/tasks/bootstrap-coreos.yml | 33 ++------
 roles/bootstrap-os/tasks/bootstrap-debian.yml | 26 ++++---
 roles/bootstrap-os/tasks/bootstrap-fedora.yml | 40 ++++++++--
 .../bootstrap-os/tasks/bootstrap-opensuse.yml | 15 ++--
 roles/bootstrap-os/tasks/bootstrap-ubuntu.yml | 72 ------------------
 roles/bootstrap-os/tasks/main.yml             | 37 ++++-----
 scale.yml                                     |  2 -
 12 files changed, 174 insertions(+), 228 deletions(-)
 create mode 100644 roles/bootstrap-os/README.md
 delete mode 100644 roles/bootstrap-os/tasks/bootstrap-ubuntu.yml

diff --git a/cluster.yml b/cluster.yml
index ce8abbac7..d1ccb317c 100644
--- a/cluster.yml
+++ b/cluster.yml
@@ -22,10 +22,6 @@
 - hosts: k8s-cluster:etcd:calico-rr
   any_errors_fatal: "{{ any_errors_fatal | default(true) }}"
   gather_facts: false
-  vars:
-    # Need to disable pipelining for bootstrap-os as some systems have requiretty in sudoers set, which makes pipelining
-    # fail. bootstrap-os fixes this on these systems, so in later plays it can be enabled.
-    ansible_ssh_pipelining: false
   roles:
     - { role: kubespray-defaults}
     - { role: bootstrap-os, tags: bootstrap-os}
diff --git a/roles/bootstrap-os/README.md b/roles/bootstrap-os/README.md
new file mode 100644
index 000000000..c01611dfd
--- /dev/null
+++ b/roles/bootstrap-os/README.md
@@ -0,0 +1,59 @@
+# bootstrap-os
+
+Bootstrap an Ansible host to be able to run Ansible modules.
+
+This role will:
+  * configure the package manager (if applicable) to be able to fetch packages
+  * install Python
+  * install the necessary packages to use Ansible's package manager modules
+  * set the hostname of the host to `{{ inventory_hostname }}` when requested
+
+## Requirements
+
+A host running an operating system that is supported by Kubespray.
+See https://github.com/kubernetes-sigs/kubespray#supported-linux-distributions for a current list.
+
+SSH access to the host.
+
+## Role Variables
+
+Variables are listed with their default values, if applicable.
+
+### General variables
+
+  * `http_proxy`/`https_proxy`
+    The role will configure the package manager (if applicable) to download packages via a proxy.
+    This is currently implemented for CentOS/RHEL (`http_proxy` only) as well as Debian and Ubuntu (both `http_proxy` and `https_proxy` are respected)
+
+  * `override_system_hostname: true`
+    The role will set the hostname of the machine to the name it has according to Ansible's inventory (the variable `{{ inventory_hostname }}`).
+
+### Per distribution variables
+
+#### CoreOS
+
+* `coreos_locksmithd_disable: false`
+  Whether `locksmithd` (responsible for rolling restarts) should be disabled or be left alone.
+
+#### CentOS/RHEL
+
+* `centos_fastestmirror_enabled: false`
+  Whether the [fastestmirror](https://wiki.centos.org/PackageManagement/Yum/FastestMirror) yum plugin should be enabled.
+
+## Dependencies
+
+The `kubespray-defaults` role is expected to be run before this role.
+
+## Example Playbook
+
+Remember to disable fact gathering since Python might not be present on hosts.
+
+    - hosts: all
+      gather_facts: false  # not all hosts might be able to run modules yet
+      roles:
+         - kubespray-defaults
+         - bootstrap-os
+
+## License
+
+Apache 2.0
diff --git a/roles/bootstrap-os/defaults/main.yml b/roles/bootstrap-os/defaults/main.yml
index 2d86b534c..e9fee74c8 100644
--- a/roles/bootstrap-os/defaults/main.yml
+++ b/roles/bootstrap-os/defaults/main.yml
@@ -1,16 +1,14 @@
 ---
-pip_python_coreos_modules:
-  - httplib2
-  - six
-
-override_system_hostname: true
-coreos_auto_upgrade: true
-
+## CentOS/RHEL specific variables
 # Install epel repo on Centos/RHEL
-epel_enabled: false
+centos_epel_enabled: false
+# Use the fastestmirror yum plugin
+centos_fastestmirror_enabled: false
 
-# CentOS/RedHat Extras repo
-extras_rh_repo_base_url: "http://mirror.centos.org/centos/$releasever/extras/$basearch/"
-extras_rh_repo_gpgkey: "http://mirror.centos.org/centos/RPM-GPG-KEY-CentOS-7"
-# Caching extras packages after installation
-extras_rh_rpm_keepcache: 0
\ No newline at end of file
+## CoreOS specific variables
+# Disable locksmithd or leave it in its current state
+coreos_locksmithd_disable: false
+
+## General
+# Set the hostname to inventory_hostname
+override_system_hostname: true
diff --git a/roles/bootstrap-os/tasks/bootstrap-centos.yml b/roles/bootstrap-os/tasks/bootstrap-centos.yml
index 7f5d641f8..52e6df791 100644
--- a/roles/bootstrap-os/tasks/bootstrap-centos.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-centos.yml
@@ -1,10 +1,13 @@
 ---
-- name: Check if atomic host
+# CentOS ships with python installed
+
+- name: Check if this is an atomic host
   stat:
     path: /run/ostree-booted
   register: ostree
 
-- set_fact:
+- name: Store the fact if this is an atomic host
+  set_fact:
     is_atomic: "{{ ostree.stat.exists }}"
 
 - name: Check presence of fastestmirror.conf
@@ -12,79 +15,34 @@
     path: /etc/yum/pluginconf.d/fastestmirror.conf
   register: fastestmirror
 
-# fastestmirror plugin actually slows down Ansible deployments
-- name: Disable fastestmirror plugin
+# the fastestmirror plugin can actually slow down Ansible deployments
+- name: Disable fastestmirror plugin if requested
   lineinfile:
     dest: /etc/yum/pluginconf.d/fastestmirror.conf
     regexp: "^enabled=.*"
     line: "enabled=0"
     state: present
   become: true
-  when: fastestmirror.stat.exists
+  when:
+    - fastestmirror.stat.exists
+    - not centos_fastestmirror_enabled
 
 - name: Add proxy to /etc/yum.conf if http_proxy is defined
   lineinfile:
     path: "/etc/yum.conf"
     line: "proxy={{ http_proxy }}"
-    create: yes
-    state: present
-  become: true
-  when: http_proxy is defined
-
-- name: Install libselinux-python and yum-utils for bootstrap
-  yum:
-    name:
-      - libselinux-python
-      - yum-utils
+    create: true
     state: present
   become: true
   when:
-    - not is_atomic
-
-- name: Check python-pip package
-  yum:
-    list=python-pip
-  register: package_python_pip
-  when:
-    - not is_atomic
-
-- name: Install epel-release for bootstrap
-  yum:
-    name: epel-release
-    state: present
-  become: true
-  when:
-    - epel_enabled
-    - not is_atomic
-    - package_python_pip.results | length != 0
-
-- name: check python-httplib2 package
-  yum:
-    list: "python-httplib2"
-  register: package_python_httplib2
-  when:
-    - not is_atomic
-
-- name: Configure extras repository if python-httplib2 not avaiable in current repos
-  yum_repository:
-    name: extras
-    description: "CentOS-7 - Extras"
-    state: present
-    baseurl: "{{ extras_rh_repo_base_url }}"
-    file: "extras"
-    gpgcheck: yes
-    gpgkey: "{{extras_rh_repo_gpgkey}}"
-    keepcache: "{{ extras_rh_rpm_keepcache | default('1') }}"
-    proxy: " {{ http_proxy | default('_none_') }}"
-  when:
-    - not is_atomic
-    - package_python_httplib2.results | length == 0
+    - http_proxy is defined
 
-- name: Install pip for bootstrap
-  yum:
-    name: python-pip
+# libselinux-python is required on SELinux enabled hosts
+# See https://docs.ansible.com/ansible/latest/installation_guide/intro_installation.html#managed-node-requirements
+- name: Install libselinux-python
+  package:
+    name: libselinux-python
     state: present
   become: true
   when:
     - not is_atomic
-    - package_python_pip.results | length != 0
diff --git a/roles/bootstrap-os/tasks/bootstrap-clearlinux.yml b/roles/bootstrap-os/tasks/bootstrap-clearlinux.yml
index a43ee9ad3..de42e3cf1 100644
--- a/roles/bootstrap-os/tasks/bootstrap-clearlinux.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-clearlinux.yml
@@ -1,16 +1,16 @@
 ---
-- name: Install basic packages to run containers
+# ClearLinux ships with Python installed
+
+- name: Install basic package to run containers
   package:
-    name: "{{ item }}"
+    name: containers-basic
     state: present
-  with_items:
-    - containers-basic
 
 - name: Make sure docker service is enabled
   systemd:
     name: docker
-    masked: no
-    enabled: yes
-    daemon_reload: yes
+    masked: false
+    enabled: true
+    daemon_reload: true
     state: started
   become: true
diff --git a/roles/bootstrap-os/tasks/bootstrap-coreos.yml b/roles/bootstrap-os/tasks/bootstrap-coreos.yml
index 909e0e374..48371555d 100644
--- a/roles/bootstrap-os/tasks/bootstrap-coreos.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-coreos.yml
@@ -1,4 +1,6 @@
 ---
+# CoreOS ships without Python installed
+
 - name: Check if bootstrap is needed
   raw: stat /opt/bin/.bootstrapped
   register: need_bootstrap
@@ -16,39 +18,20 @@
 
 - name: Run bootstrap.sh
   script: bootstrap.sh
-  when: need_bootstrap.rc != 0
+  become: true
+  when:
+    - need_bootstrap.rc != 0
 
-- set_fact:
+- name: Set the ansible_python_interpreter fact
+  set_fact:
     ansible_python_interpreter: "{{ bin_dir }}/python"
   tags:
     - facts
 
-- name: Install pip3
-  command: "{{ ansible_python_interpreter }} -m ensurepip"
-  args:
-    creates: "{{ bin_dir }}/pypy3/bin/pip3"
-  register: pip_installed
-
-- name: Install pip3 link
-  file:
-    src: "{{ bin_dir }}/pypy3/bin/pip3"
-    dest: "{{ bin_dir }}/pip3"
-    mode: 0755
-    state: link
-  when: pip_installed.changed
-
-- name: Install required python modules
-  pip:
-    name: "{{ item }}"
-    extra_args: "{{ pip_extra_args | default(omit) }}"
-  with_items: "{{ pip_python_coreos_modules }}"
-  environment:
-    PATH: "{{ ansible_env.PATH }}:{{ bin_dir }}"
-
 - name: Disable auto-upgrade
   systemd:
     name: locksmithd.service
     masked: true
     state: stopped
   when:
-    - not coreos_auto_upgrade
+    - coreos_locksmithd_disable
diff --git a/roles/bootstrap-os/tasks/bootstrap-debian.yml b/roles/bootstrap-os/tasks/bootstrap-debian.yml
index 1cb9b273c..1df598385 100644
--- a/roles/bootstrap-os/tasks/bootstrap-debian.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-debian.yml
@@ -1,17 +1,16 @@
 ---
+# Some Debian based distros ship without Python installed
+
 - name: Check if bootstrap is needed
-  raw: which "{{ item }}"
+  raw: which python
   register: need_bootstrap
   failed_when: false
   changed_when: false
   # This command should always run, even in check mode
   check_mode: false
-  with_items:
-    - python
-    - pip
-    - dbus-daemon
   environment: {}
-  tags: facts
+  tags:
+    - facts
 
 - name: Check http::proxy in /etc/apt/apt.conf
   raw: grep -qsi 'Acquire::http::proxy' /etc/apt/apt.conf
@@ -51,15 +50,18 @@
     - https_proxy is defined
     - need_https_proxy.rc != 0
 
-- name: Install python, pip, and dbus
+- name: Install python
   raw:
     apt-get update && \
-    DEBIAN_FRONTEND=noninteractive apt-get install -y python-minimal python-pip dbus
+    DEBIAN_FRONTEND=noninteractive apt-get install -y python-minimal
   become: true
   environment: {}
   when:
-    need_bootstrap.results | map(attribute='rc') | sort | last | bool
+    - need_bootstrap.rc != 0
 
-- set_fact:
-    ansible_python_interpreter: "/usr/bin/python"
-  tags: facts
+# Workaround for https://github.com/ansible/ansible/issues/25543
+- name: Install dbus for the hostname module
+  package:
+    name: dbus
+    state: present
+  become: true
diff --git a/roles/bootstrap-os/tasks/bootstrap-fedora.yml b/roles/bootstrap-os/tasks/bootstrap-fedora.yml
index 292c2d34d..f25d2f0ff 100644
--- a/roles/bootstrap-os/tasks/bootstrap-fedora.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-fedora.yml
@@ -1,22 +1,46 @@
 ---
+# Some Fedora based distros ship without Python installed
+
+- name: Check if this is an atomic host
+  raw: stat /run/ostree-booted
+  register: ostree
+  environment: {}
+  failed_when: false
+  changed_when: false
+  tags:
+    - facts
+
+- name: Store the fact if this is an atomic host
+  set_fact:
+    is_atomic: "{{ ostree.rc == 0 }}"
+  tags:
+    - facts
+
 - name: Check if bootstrap is needed
-  raw: which "{{ item }}"
+  raw: which python
   register: need_bootstrap
   failed_when: false
   changed_when: false
-  with_items:
-    - python
   environment: {}
-  tags: facts
+  tags:
+    - facts
 
+# Fedora's policy as of Fedora 30 is to still install python2 as /usr/bin/python
+# See https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3 for the current status
 - name: Install python on fedora
-  raw: "dnf install --assumeyes --quiet python"
+  raw: "dnf install --assumeyes --quiet python2"
   become: true
   environment: {}
-  when: need_bootstrap.results | map(attribute='rc') | sort | last | bool
+  when:
+    - need_bootstrap.rc != 0
+    - not is_atomic
 
-- name: Install required python packages
-  dnf:
+# libselinux-python is required on SELinux enabled hosts
+# See https://docs.ansible.com/ansible/latest/installation_guide/intro_installation.html#managed-node-requirements
+- name: Install libselinux-python
+  package:
     name: libselinux-python
     state: present
   become: true
+  when:
+    - not is_atomic
diff --git a/roles/bootstrap-os/tasks/bootstrap-opensuse.yml b/roles/bootstrap-os/tasks/bootstrap-opensuse.yml
index 4eeae9fa9..a38f36684 100644
--- a/roles/bootstrap-os/tasks/bootstrap-opensuse.yml
+++ b/roles/bootstrap-os/tasks/bootstrap-opensuse.yml
@@ -1,13 +1,10 @@
 ---
-- name: Ensure zypper cache is updated (SUSE)
-  zypper_repository:
-    repo: "*"
-    runrefresh: yes
+# OpenSUSE ships with Python installed
 
-- name: Install required packages (SUSE)
-  package:
-    name: "{{ item }}"
+# Without this package, the get_url module fails when trying to handle https
+- name: Install python-cryptography
+  zypper:
+    name: python-cryptography
     state: present
-  with_items:
-    - python-cryptography
+    update_cache: true
   become: true
diff --git a/roles/bootstrap-os/tasks/bootstrap-ubuntu.yml b/roles/bootstrap-os/tasks/bootstrap-ubuntu.yml
deleted file mode 100644
index 893041ad0..000000000
--- a/roles/bootstrap-os/tasks/bootstrap-ubuntu.yml
+++ /dev/null
@@ -1,72 +0,0 @@
----
-- name: List ubuntu_packages
-  set_fact:
-    ubuntu_packages:
-      - python
-      - python-apt
-      - python-pip
-      - dbus
-
-- name: Check if bootstrap is needed
-  raw: dpkg -l | cut -d' ' -f3 | grep -e ^{{ item }}$
-  register: need_bootstrap
-  failed_when: false
-  changed_when: false
-  # This command should always run, even in check mode
-  check_mode: false
-  with_items: "{{ ubuntu_packages }}"
-  environment: {}
-  tags:
-    - facts
-
-- name: Check http::proxy in /etc/apt/apt.conf
-  raw: grep -qsi 'Acquire::http::proxy' /etc/apt/apt.conf
-  register: need_http_proxy
-  failed_when: false
-  changed_when: false
-  # This command should always run, even in check mode
-  check_mode: false
-  environment: {}
-  when:
-    - http_proxy is defined
-
-- name: Add http_proxy to /etc/apt/apt.conf if http_proxy is defined
-  raw: echo 'Acquire::http::proxy "{{ http_proxy }}";' >> /etc/apt/apt.conf
-  become: true
-  environment: {}
-  when:
-    - http_proxy is defined
-    - need_http_proxy.rc != 0
-
-- name: Check https::proxy in /etc/apt/apt.conf
-  raw: grep -qsi 'Acquire::https::proxy' /etc/apt/apt.conf
-  register: need_https_proxy
-  failed_when: false
-  changed_when: false
-  # This command should always run, even in check mode
-  check_mode: false
-  environment: {}
-  when:
-    - https_proxy is defined
-
-- name: Add https_proxy to /etc/apt/apt.conf if https_proxy is defined
-  raw: echo 'Acquire::https::proxy "{{ https_proxy }}";' >> /etc/apt/apt.conf
-  become: true
-  environment: {}
-  when:
-    - https_proxy is defined
-    - need_https_proxy.rc != 0
-
-- name: Install python and pip
-  raw:
-    apt-get update && \
-    DEBIAN_FRONTEND=noninteractive apt-get install -y {{ ubuntu_packages | join(" ") }}
-  become: true
-  environment: {}
-  when:
-    - need_bootstrap.results | map(attribute='rc') | sort | last | bool
-
-- set_fact:
-    ansible_python_interpreter: "/usr/bin/python"
-  tags:
-    - facts
diff --git a/roles/bootstrap-os/tasks/main.yml b/roles/bootstrap-os/tasks/main.yml
index 7c515b524..e758fca12 100644
--- a/roles/bootstrap-os/tasks/main.yml
+++ b/roles/bootstrap-os/tasks/main.yml
@@ -7,58 +7,61 @@
   check_mode: false
   environment: {}
 
-- include_tasks: bootstrap-ubuntu.yml
-  when: '"Ubuntu" in os_release.stdout'
+- include_tasks: bootstrap-centos.yml
+  when: '"CentOS" in os_release.stdout or "Red Hat Enterprise Linux" in os_release.stdout'
 
-- include_tasks: bootstrap-debian.yml
-  when: '"Debian" in os_release.stdout'
+- include_tasks: bootstrap-clearlinux.yml
+  when: '"Clear Linux OS" in os_release.stdout'
 
 - include_tasks: bootstrap-coreos.yml
   when: '"CoreOS" in os_release.stdout'
 
+- include_tasks: bootstrap-debian.yml
+  when: '"Debian" in os_release.stdout or "Ubuntu" in os_release.stdout'
+
 - include_tasks: bootstrap-fedora.yml
   when: '"Fedora" in os_release.stdout'
 
-- include_tasks: bootstrap-centos.yml
-  when: '"CentOS" in os_release.stdout or "Red Hat Enterprise Linux" in os_release.stdout'
-
 - include_tasks: bootstrap-opensuse.yml
   when: '"openSUSE" in os_release.stdout'
 
-- include_tasks: bootstrap-clearlinux.yml
-  when: '"Clear Linux OS" in os_release.stdout'
-
 - name: Create remote_tmp for it is used by another module
   file:
     path: "{{ ansible_remote_tmp | default('~/.ansible/tmp') }}"
     state: directory
     mode: 0700
 
-- name: Gather nodes hostnames
+# Workaround for https://github.com/ansible/ansible/issues/42726
+# (1/3)
+- name: Gather host facts to get ansible_os_family
   setup:
     gather_subset: '!all'
     filter: ansible_*
 
-- name: Assign inventory name to unconfigured hostnames (non-CoreOS and Tumbleweed)
+- name: Assign inventory name to unconfigured hostnames (non-CoreOS, Suse and ClearLinux)
   hostname:
     name: "{{ inventory_hostname }}"
   when:
     - override_system_hostname
-    - ansible_os_family not in ['Suse', 'CoreOS', 'Container Linux by CoreOS', 'ClearLinux']
+    - ansible_os_family not in ['Suse', 'Container Linux by CoreOS', 'ClearLinux']
 
-- name: Assign inventory name to unconfigured hostnames (CoreOS and Tumbleweed only)
+# (2/3)
+- name: Assign inventory name to unconfigured hostnames (CoreOS, Suse and ClearLinux only)
   command: "hostnamectl set-hostname {{ inventory_hostname }}"
   register: hostname_changed
+  changed_when: false
   when:
     - override_system_hostname
-    - ansible_os_family in ['Suse', 'CoreOS', 'Container Linux by CoreOS', 'ClearLinux']
+    - ansible_os_family in ['Suse', 'Container Linux by CoreOS', 'ClearLinux']
 
-- name: Update hostname fact (CoreOS and Tumbleweed only)
+# (3/3)
+- name: Update hostname fact (CoreOS, Suse and ClearLinux only)
   setup:
     gather_subset: '!all'
     filter: ansible_hostname
   when:
-    - hostname_changed.changed
+    - override_system_hostname
+    - ansible_os_family in ['Suse', 'Container Linux by CoreOS', 'ClearLinux']
 
 - name: "Install ceph-commmon package"
   package:
diff --git a/scale.yml b/scale.yml
index d5532ff1e..723debbb3 100644
--- a/scale.yml
+++ b/scale.yml
@@ -23,8 +23,6 @@
   hosts: kube-node
   any_errors_fatal: "{{ any_errors_fatal | default(true) }}"
   gather_facts: false
-  vars:
-    ansible_ssh_pipelining: false
   roles:
     - { role: kubespray-defaults}
     - { role: bootstrap-os, tags: bootstrap-os}
-- 
GitLab