Merge pull request #217 from stackhpc/upstream/2023.1-2023-12-08

Synchronise 2023.1 with upstream

ansible/kayobe-ansible-user.yml

@@ -13,28 +13,23 @@
     - kayobe-ansible-user
   tasks:
     - name: Check whether the host is accessible via SSH
-      local_action:
-        module: command ssh -o BatchMode=yes -p {{ ssh_port }} {{ ssh_user }}@{{ ssh_host }} hostname
-      failed_when: false
+      raw: hostname
+      ignore_unreachable: true
       changed_when: false
       check_mode: no
       register: ssh_result
-      vars:
-        ssh_user: "{{ ansible_user }}"
-        ssh_host: "{{ ansible_host | default(inventory_hostname) }}"
-        ssh_port: "{{ ansible_ssh_port | default('22') }}"
 
     - name: Group hosts requiring kayobe user bootstrapping
       group_by:
-        key: kayobe_user_bootstrap_required_{{ ssh_result.rc != 0 }}
+        key: kayobe_user_bootstrap_required_{{ ssh_result.unreachable | default(false) }}
       changed_when: false
 
     - name: Display a message when bootstrapping is required
       debug:
         msg: >
           Cannot access host via SSH using Kayobe Ansible user account -
           attempting bootstrap
-      when: ssh_result.rc != 0
+      when: ssh_result.unreachable | default(false)
 
 - name: Ensure python is installed
   hosts: kayobe_user_bootstrap_required_True

ansible/kolla-ansible.yml

@@ -108,7 +108,6 @@
         kolla_inspector_default_gateway: "{{ inspection_net_name | net_inspection_gateway or inspection_net_name | net_gateway }}"
         kolla_inspector_extra_kernel_options: "{{ inspector_extra_kernel_options }}"
         kolla_libvirt_tls: "{{ compute_libvirt_enable_tls | bool }}"
-        kolla_enable_host_ntp: false
         kolla_globals_paths_static:
           - "{{ kayobe_config_path }}"
         kolla_globals_paths_extra: "{{ kolla_globals_paths_static + kayobe_env_search_paths }}"

ansible/roles/kolla-ansible/defaults/main.yml

@@ -290,12 +290,6 @@ kolla_internal_tls_cert:
 # Desired SELinux state.
 kolla_selinux_state:
 
-###############################################################################
-# NTP
-
-# Whether to enable the NTP daemon.
-kolla_enable_host_ntp:
-
 ###############################################################################
 # Docker configuration.
 

ansible/roles/kolla-ansible/templates/kolla/globals.yml

@@ -527,10 +527,6 @@ grafana_admin_username: "{{ grafana_local_admin_user_name }}"
 selinux_state: {{ kolla_selinux_state }}
 {% endif %}
 
-{% if kolla_enable_host_ntp is not none %}
-enable_host_ntp: {{ kolla_enable_host_ntp | bool }}
-{% endif %}
-
 # Kayobe performs creation of the Kolla Ansible user account, so there is no
 # need for Kolla Ansible to repeat this.
 create_kolla_user: false

ansible/roles/kolla-bifrost/tasks/main.yml

@@ -1,4 +1,12 @@
 ---
+- name: Check if inspection allocation is defined
+  assert:
+    that:
+      - kolla_bifrost_dhcp_pool_start | length > 0
+      - kolla_bifrost_dhcp_pool_end | length > 0
+      - kolla_bifrost_dhcp_pool_mask | length > 0
+    fail_msg: "Inspection allocation pool for provisioning network is not properly defined"
+
 - name: Ensure the Kolla Bifrost configuration directories exist
   file:
     path: "{{ kolla_node_custom_config_path }}/bifrost"

dev/functions

@@ -232,22 +232,22 @@ function upgrade_kayobe_venv {
 # Deployment
 
 function is_deploy_image_built_locally {
-    ipa_build_images=$(kayobe configuration dump --host controllers[0] --var-name ipa_build_images)
+    ipa_build_images=$(kayobe configuration dump --host localhost --var-name ipa_build_images)
     to_bool "$ipa_build_images"
 }
 
 function is_ironic_enabled {
-    ironic_enabled=$(kayobe configuration dump --host controllers[0] --var-name kolla_enable_ironic)
+    ironic_enabled=$(kayobe configuration dump --host localhost --var-name kolla_enable_ironic)
     to_bool "$ironic_enabled"
 }
 
 function is_overcloud_host_image_built_by_dib {
-    overcloud_dib_build_host_images=$(kayobe configuration dump --host controllers[0] --var-name overcloud_dib_build_host_images)
+    overcloud_dib_build_host_images=$(kayobe configuration dump --host localhost --var-name overcloud_dib_build_host_images)
     to_bool "$overcloud_dib_build_host_images"
 }
 
 function is_cinder_enabled {
-    flag="$(run_kayobe configuration dump --host controllers[0] --var-name kolla_enable_cinder)"
+    flag="$(run_kayobe configuration dump --host localhost --var-name kolla_enable_cinder)"
     to_bool "$flag"
 }
 

doc/source/installation.rst

@@ -158,7 +158,7 @@ Editable source installation
 ----------------------------
 
 From Kayobe 5.0.0 onwards it is possible to create an `editable install
-<https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs>`__
+<https://pip.pypa.io/en/stable/topics/local-project-installs/#editable-installs>`__
 of Kayobe. In an editable install, any changes to the Kayobe source tree will
 immediately be visible when running any Kayobe commands.  To create an editable
 install, add the ``-e`` flag::

kayobe/ansible.py

@@ -22,6 +22,7 @@
 import tempfile
 
 import ansible.constants
+from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode
 
 from kayobe import exception
 from kayobe import utils
@@ -299,6 +300,18 @@ def run_playbook(parsed_args, playbook, *args, **kwargs):
     return run_playbooks(parsed_args, [playbook], *args, **kwargs)
 
 
+def _sanitise_hostvar(var):
+    """Sanitise a host variable."""
+    if isinstance(var, AnsibleVaultEncryptedUnicode):
+        return "******"
+    # Recursively sanitise dicts and lists.
+    if isinstance(var, dict):
+        return {k: _sanitise_hostvar(v) for k, v in var.items()}
+    if isinstance(var, list):
+        return [_sanitise_hostvar(v) for v in var]
+    return var
+
+
 def config_dump(parsed_args, host=None, hosts=None, var_name=None,
                 facts=None, extra_vars=None, tags=None, verbose_level=None):
     dump_dir = tempfile.mkdtemp()
@@ -324,15 +337,16 @@ def config_dump(parsed_args, host=None, hosts=None, var_name=None,
             LOG.debug("Found dump file %s", path)
             inventory_hostname, ext = os.path.splitext(path)
             if ext == ".yml":
-                hvars = utils.read_yaml_file(os.path.join(dump_dir, path))
+                dump_file = os.path.join(dump_dir, path)
+                hvars = utils.read_config_dump_yaml_file(dump_file)
                 if host:
                     return hvars
                 else:
                     hostvars[inventory_hostname] = hvars
             else:
                 LOG.warning("Unexpected extension on config dump file %s",
                             path)
-        return hostvars
+        return {k: _sanitise_hostvar(v) for k, v in hostvars.items()}
     finally:
         shutil.rmtree(dump_dir)
 

kayobe/tests/unit/test_ansible.py

@@ -583,7 +583,7 @@ def test_run_playbooks_failure(self, mock_validate, mock_vars, mock_run):
                           ansible.run_playbooks, parsed_args, ["command"])
 
     @mock.patch.object(shutil, 'rmtree')
-    @mock.patch.object(utils, 'read_yaml_file')
+    @mock.patch.object(utils, 'read_config_dump_yaml_file')
     @mock.patch.object(os, 'listdir')
     @mock.patch.object(ansible, 'run_playbook')
     @mock.patch.object(tempfile, 'mkdtemp')
@@ -621,6 +621,70 @@ def test_config_dump(self, mock_mkdtemp, mock_run, mock_listdir, mock_read,
             mock.call(os.path.join(dump_dir, "host2.yml")),
         ])
 
+    @mock.patch.object(shutil, 'rmtree')
+    @mock.patch.object(utils, 'read_file')
+    @mock.patch.object(os, 'listdir')
+    @mock.patch.object(ansible, 'run_playbook')
+    @mock.patch.object(tempfile, 'mkdtemp')
+    def test_config_dump_vaulted(self, mock_mkdtemp, mock_run, mock_listdir,
+                                 mock_read, mock_rmtree):
+        parser = argparse.ArgumentParser()
+        parsed_args = parser.parse_args([])
+        dump_dir = "/path/to/dump"
+        mock_mkdtemp.return_value = dump_dir
+        mock_listdir.return_value = ["host1.yml", "host2.yml"]
+        config = """---
+key1: !vault |
+  $ANSIBLE_VAULT;1.1;AES256
+  633230623736383232323862393364323037343430393530316636363961626361393133646437
+  643438663261356433656365646138666133383032376532310a63323432306431303437623637
+  346236316161343635636230613838316566383933313338636237616338326439616536316639
+  6334343462333062363334300a3930313762313463613537626531313230303731343365643766
+  666436333037
+key2: value2
+key3:
+  - !vault |
+    $ANSIBLE_VAULT;1.1;AES256
+    633230623736383232323862393364323037343430393530316636363961626361393133646437
+    643438663261356433656365646138666133383032376532310a63323432306431303437623637
+    346236316161343635636230613838316566383933313338636237616338326439616536316639
+    6334343462333062363334300a3930313762313463613537626531313230303731343365643766
+    666436333037
+"""
+        config_nested = """---
+key1:
+  key2: !vault |
+    $ANSIBLE_VAULT;1.1;AES256
+    633230623736383232323862393364323037343430393530316636363961626361393133646437
+    643438663261356433656365646138666133383032376532310a63323432306431303437623637
+    346236316161343635636230613838316566383933313338636237616338326439616536316639
+    6334343462333062363334300a3930313762313463613537626531313230303731343365643766
+    666436333037
+"""
+        mock_read.side_effect = [config, config_nested]
+        result = ansible.config_dump(parsed_args)
+        expected_result = {
+            "host1": {"key1": "******", "key2": "value2", "key3": ["******"]},
+            "host2": {"key1": {"key2": "******"}},
+        }
+        self.assertEqual(result, expected_result)
+        dump_config_path = utils.get_data_files_path(
+            "ansible", "dump-config.yml")
+        mock_run.assert_called_once_with(parsed_args,
+                                         dump_config_path,
+                                         extra_vars={
+                                             "dump_path": dump_dir,
+                                         },
+                                         check_output=True, tags=None,
+                                         verbose_level=None, check=False,
+                                         list_tasks=False, diff=False)
+        mock_rmtree.assert_called_once_with(dump_dir)
+        mock_listdir.assert_any_call(dump_dir)
+        mock_read.assert_has_calls([
+            mock.call(os.path.join(dump_dir, "host1.yml")),
+            mock.call(os.path.join(dump_dir, "host2.yml")),
+        ])
+
     @mock.patch.object(utils, 'galaxy_role_install', autospec=True)
     @mock.patch.object(utils, 'is_readable_file', autospec=True)
     @mock.patch.object(os, 'makedirs', autospec=True)

kayobe/tests/unit/test_utils.py

@@ -17,6 +17,7 @@
 import unittest
 from unittest import mock
 
+from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode
 import yaml
 
 from kayobe import exception
@@ -127,6 +128,59 @@ def test_read_yaml_file_not_yaml(self, mock_read):
         mock_read.return_value = "[1{!"
         self.assertRaises(SystemExit, utils.read_yaml_file, "/path/to/file")
 
+    @mock.patch.object(utils, "read_file")
+    def test_read_config_dump_yaml_file(self, mock_read):
+        config = """---
+key1: value1
+key2: value2
+"""
+        mock_read.return_value = config
+        result = utils.read_config_dump_yaml_file("/path/to/file")
+        self.assertEqual(result, {"key1": "value1", "key2": "value2"})
+        mock_read.assert_called_once_with("/path/to/file")
+
+    @mock.patch.object(utils, "read_file")
+    def test_read_config_dump_yaml_file_vaulted(self, mock_read):
+        config = """---
+key1: !vault |
+  $ANSIBLE_VAULT;1.1;AES256
+  633230623736383232323862393364323037343430393530316636363961626361393133646437
+  643438663261356433656365646138666133383032376532310a63323432306431303437623637
+  346236316161343635636230613838316566383933313338636237616338326439616536316639
+  6334343462333062363334300a3930313762313463613537626531313230303731343365643766
+  666436333037
+key2: value2
+key3:
+  - !vault |
+    $ANSIBLE_VAULT;1.1;AES256
+    633230623736383232323862393364323037343430393530316636363961626361393133646437
+    643438663261356433656365646138666133383032376532310a63323432306431303437623637
+    346236316161343635636230613838316566383933313338636237616338326439616536316639
+    6334343462333062363334300a3930313762313463613537626531313230303731343365643766
+    666436333037
+"""
+        mock_read.return_value = config
+        result = utils.read_config_dump_yaml_file("/path/to/file")
+        # Can't read the value without an encryption key, so just check type.
+        self.assertTrue(isinstance(result["key1"],
+                                   AnsibleVaultEncryptedUnicode))
+        self.assertEqual(result["key2"], "value2")
+        self.assertTrue(isinstance(result["key3"][0],
+                                   AnsibleVaultEncryptedUnicode))
+        mock_read.assert_called_once_with("/path/to/file")
+
+    @mock.patch.object(utils, "read_file")
+    def test_read_config_dump_yaml_file_open_failure(self, mock_read):
+        mock_read.side_effect = IOError
+        self.assertRaises(SystemExit, utils.read_config_dump_yaml_file,
+                          "/path/to/file")
+
+    @mock.patch.object(utils, "read_file")
+    def test_read_config_dump_yaml_file_not_yaml(self, mock_read):
+        mock_read.return_value = "[1{!"
+        self.assertRaises(SystemExit, utils.read_config_dump_yaml_file,
+                          "/path/to/file")
+
     @mock.patch.object(subprocess, "check_call")
     def test_run_command(self, mock_call):
         output = utils.run_command(["command", "to", "run"])

kayobe/utils.py

@@ -24,6 +24,7 @@
 import subprocess
 import sys
 
+from ansible.parsing.yaml.loader import AnsibleLoader
 import yaml
 
 from kayobe import exception
@@ -153,11 +154,28 @@ def read_yaml_file(path):
     try:
         content = read_file(path)
     except IOError as e:
-        print("Failed to open config dump file %s: %s" %
+        print("Failed to open YAML file %s: %s" %
               (path, repr(e)))
         sys.exit(1)
     try:
         return yaml.safe_load(content)
+    except yaml.YAMLError as e:
+        print("Failed to decode YAML file %s: %s" %
+              (path, repr(e)))
+        sys.exit(1)
+
+
+def read_config_dump_yaml_file(path):
+    """Read and decode a configuration dump YAML file."""
+    try:
+        content = read_file(path)
+    except IOError as e:
+        print("Failed to open config dump file %s: %s" %
+              (path, repr(e)))
+        sys.exit(1)
+    try:
+        # AnsibleLoader supports loading vault encrypted variables.
+        return AnsibleLoader(content).get_single_data()
     except yaml.YAMLError as e:
         print("Failed to decode config dump YAML file %s: %s" %
               (path, repr(e)))

playbooks/kayobe-overcloud-host-configure-base/tests/test_overcloud_host_configure.py

@@ -319,6 +319,19 @@ def test_firewalld_rules(host):
             assert expected_line in perm_info
 
 
+@pytest.mark.skipif(not _is_dnf(),
+                    reason="SELinux only supported on CentOS/Rocky")
+def test_selinux(host):
+    selinux = host.check_output("sestatus")
+    selinux = selinux.splitlines()
+    # Remove duplicate whitespace characters in output
+    selinux = [" ".join(x.split()) for x in selinux]
+
+    assert "SELinux status: enabled" in selinux
+    assert "Current mode: permissive" in selinux
+    assert "Mode from config file: permissive" in selinux
+
+
 def test_swap(host):
     swapon = host.check_output("swapon -s")
     swapon = swapon.splitlines()

releasenotes/notes/add-custom-user-data-content-d188c7c197b50acb.yaml

@@ -2,5 +2,5 @@
 features:
   - |
     Adds a new ``kolla_bifrost_deploy_image_user_data_content`` variable used
-    to define the custom user_data content used by the cloud-init for overcloud
+    to define custom ``user_data`` content used by cloud-init for overcloud
     provision.

releasenotes/notes/config-dump-vault-edc615e475f234ac.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue where ``kayobe configuration dump`` would fail when
+    variables are encrypted using Ansible Vault. Encrypted variables are now
+    sanitised in the dump output. `LP#2031390
+    <https://bugs.launchpad.net/kayobe/+bug/2031390>`__

releasenotes/notes/honor-ssh-args-for-bootstrap-ba894df14ba58167.yaml

@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    When determining whether or not a host needs bootstrapping, we attempt to
+    connect to the host using ``ansible_user``, if the login fails, we then
+    assume that the host needs bootstrapping. In previous releases we used a
+    manually crafted ``ssh`` command. This did not respect any customisations
+    to the SSH arguments made through Ansible configuration. We now use the raw
+    module so that these customisations are used when connecting to the host.
+    One possible use case is to configure a jump host between the control host
+    and the target hosts. If bootstrapping was needed, hosts will now show as
+    unreachable in the summary stats at the end of the run. This can safely be
+    ignored.