diff --git a/ci/playbooks/collections/ansible_collections/cifmw/general/plugins/filter/from_ini.py b/ci/playbooks/collections/ansible_collections/cifmw/general/plugins/filter/from_ini.py new file mode 120000 index 000000000..2565fa761 --- /dev/null +++ b/ci/playbooks/collections/ansible_collections/cifmw/general/plugins/filter/from_ini.py @@ -0,0 +1 @@ +../../../../../../../../plugins/filter/from_ini.py \ No newline at end of file diff --git a/hooks/playbooks/ceph.yml b/hooks/playbooks/ceph.yml index b3113044c..f48557e29 100644 --- a/hooks/playbooks/ceph.yml +++ b/hooks/playbooks/ceph.yml @@ -416,10 +416,12 @@ when: adoption_deploy_ceph_for_tripleo | default (false) no_log: "{{ cifmw_nolog | default(true) | bool }}" - # public network always exist because is provided by the ceph_spec role + # Use the snapshot exported by cifmw_ceph_spec, not storage_network_range + # and not the extra-var name: extra-vars beat set_fact of the same name, + # and uni03gamma (OSPRH-6675) overrides the role param with ctlplane. - name: Get Storage network range ansible.builtin.set_fact: - cifmw_cephadm_rgw_network: "{{ lookup('ansible.builtin.ini', 'public_network section=global file=' ~ cifmw_cephadm_bootstrap_conf) }}" + cifmw_cephadm_rgw_network: "{{ hostvars['localhost']['cifmw_ceph_spec_rendered_public_network'] | mandatory }}" - name: Set IP address of first monitor ansible.builtin.set_fact: @@ -491,7 +493,7 @@ name: cifmw_cephadm tasks_from: monitoring vars: - cifmw_cephadm_monitoring_network: "{{ lookup('ansible.builtin.ini', 'public_network section=global file=' ~ cifmw_cephadm_bootstrap_conf) }}" + cifmw_cephadm_monitoring_network: "{{ cifmw_cephadm_rgw_network }}" cifmw_cephadm_dashboard_crt: "{{ cifmw_cephadm_certificate }}" cifmw_cephadm_dashboard_key: "{{ cifmw_cephadm_key }}" diff --git a/playbooks/update.yml b/playbooks/update.yml index 46ae9f8c4..44b7d7736 100644 --- a/playbooks/update.yml +++ b/playbooks/update.yml @@ -102,7 +102,7 @@ - name: Extract the CephFSID from ceph.conf ansible.builtin.set_fact: - ceph_fsid: "{{ lookup('ansible.builtin.ini', 'fsid', section='global', file='/tmp/ceph.conf') }}" + ceph_fsid: "{{ (cephconf.content | b64decode) | cifmw.general.from_ini('fsid') }}" - name: Perform a Ceph Update ansible.builtin.import_role: diff --git a/plugins/filter/from_ini.py b/plugins/filter/from_ini.py new file mode 100644 index 000000000..acf67e0a0 --- /dev/null +++ b/plugins/filter/from_ini.py @@ -0,0 +1,93 @@ +#!/usr/bin/python3 + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + + +DOCUMENTATION = """ + name: from_ini + short_description: Read a key from INI-formatted text + description: + - Parse INI content with Python's C(configparser) using C(read_file), + which is safe on Python 3.12 (C(readfp) was removed). + - Use this instead of the C(ansible.builtin.ini) lookup when the + controller Python is 3.12+ or when the INI text is already in a + variable (for example after C(slurp)). + options: + _input: + description: INI document as a string (or bytes). + type: str + required: true + key: + description: Option name to return. + type: str + required: true + section: + description: Section that contains the key. + type: str + default: global + default: + description: Value to return when the section or key is missing. + type: str + default: "" +""" + +EXAMPLES = """ + - name: Read public_network from a rendered Ceph conf + ansible.builtin.set_fact: + public_network: >- + {{ + lookup('ansible.builtin.file', conf_path) + | cifmw.general.from_ini('public_network') + }} + + - name: Read fsid from slurped ceph.conf + ansible.builtin.set_fact: + ceph_fsid: >- + {{ (cephconf.content | b64decode) | cifmw.general.from_ini('fsid') }} +""" + +RETURN = """ + _value: + description: The option value, or I(default) when the key is absent. + type: str +""" + +import configparser +import io + +from ansible.errors import AnsibleFilterError, AnsibleFilterTypeError +from ansible.module_utils._text import to_native, to_text + + +class FilterModule: + + @staticmethod + def __from_ini(content, key, section="global", default=""): + if not isinstance(content, (str, bytes)): + raise AnsibleFilterTypeError( + "from_ini requires INI content as a string, got %s" % type(content) + ) + if not isinstance(key, str) or not key: + raise AnsibleFilterTypeError( + "from_ini requires a non-empty key name as a string, got %s" % type(key) + ) + + parser = configparser.ConfigParser(interpolation=None) + try: + parser.read_file(io.StringIO(to_text(content))) + except configparser.Error as exc: + raise AnsibleFilterError( + "from_ini - failed to parse INI content: %s" % to_native(exc), + orig_exc=exc, + ) from exc + + if not parser.has_section(section) or not parser.has_option(section, key): + return default + return parser.get(section, key) + + def filters(self): + return { + "from_ini": self.__from_ini, + } diff --git a/roles/cifmw_ceph_spec/README.md b/roles/cifmw_ceph_spec/README.md index 8c4d8f39c..7a0f8a7a3 100644 --- a/roles/cifmw_ceph_spec/README.md +++ b/roles/cifmw_ceph_spec/README.md @@ -28,6 +28,18 @@ None which is created by the `cifmw_block_device` role) * `cifmw_ceph_spec_path`: path of the rendered spec file (default `/tmp/ceph_spec.yml`) +* `cifmw_ceph_spec_path_initial_conf`: path of the rendered initial Ceph + conf file (default `/tmp/initial_ceph.conf`) +* `cifmw_ceph_spec_public_network`: CIDR written as `public_network` + (default `192.168.122.0/24`). Extra-vars override the role parameter + (uni03gamma uses this for OSPRH-6675). After the templates are written + the resolved CIDR is exported as `cifmw_ceph_spec_rendered_public_network` + on the spec host so later plays can read it from hostvars instead of + parsing the file. A distinct name is required because extra-vars beat + `set_fact` of the same name. +* `cifmw_ceph_spec_private_network`: CIDR written as `cluster_network` + (default empty, omits the key). Exported as + `cifmw_ceph_spec_rendered_private_network`. * `cifmw_ceph_spec_encryption`: Produce an initial Ceph configuration file with both over-the-wire ([msgr2 secure mode](https://docs.ceph.com/en/latest/rados/configuration/msgr2/)) diff --git a/roles/cifmw_ceph_spec/molecule/default/tasks/verify_conf.yml b/roles/cifmw_ceph_spec/molecule/default/tasks/verify_conf.yml index 81eacf31d..14e65699f 100644 --- a/roles/cifmw_ceph_spec/molecule/default/tasks/verify_conf.yml +++ b/roles/cifmw_ceph_spec/molecule/default/tasks/verify_conf.yml @@ -26,16 +26,24 @@ when: - cifmw_ceph_spec_path_initial_conf_stat.stat.exists is not defined +- name: Read rendered initial Ceph conf + ansible.builtin.slurp: + src: "{{ cifmw_ceph_spec_path_initial_conf }}" + register: cifmw_ceph_spec_rendered_conf + become: true + - name: Extract public_network ansible.builtin.set_fact: - found_public_network: "{{ lookup('ansible.builtin.ini', 'public_network section=global file=' ~ cifmw_ceph_spec_path_initial_conf) }}" + found_public_network: "{{ (cifmw_ceph_spec_rendered_conf.content | b64decode) | cifmw.general.from_ini('public_network') }}" - name: Extract cluster_network ansible.builtin.set_fact: - found_cluster_network: "{{ lookup('ansible.builtin.ini', 'cluster_network section=global file=' ~ cifmw_ceph_spec_path_initial_conf) }}" + found_cluster_network: "{{ (cifmw_ceph_spec_rendered_conf.content | b64decode) | cifmw.general.from_ini('cluster_network') }}" - name: Assert expected values about public and private network ansible.builtin.assert: that: - found_public_network == cifmw_ceph_spec_public_network - found_cluster_network == cifmw_ceph_spec_private_network + - cifmw_ceph_spec_rendered_public_network == found_public_network + - cifmw_ceph_spec_rendered_private_network == found_cluster_network diff --git a/roles/cifmw_ceph_spec/tasks/main.yml b/roles/cifmw_ceph_spec/tasks/main.yml index 48101f154..9c76792d0 100644 --- a/roles/cifmw_ceph_spec/tasks/main.yml +++ b/roles/cifmw_ceph_spec/tasks/main.yml @@ -40,3 +40,13 @@ dest: "{{ cifmw_ceph_spec_path_initial_conf }}" mode: "0644" force: true + +# Snapshot under a *new* name. Extra-vars always beat set_fact of the same +# name (ansible/ansible#19934), so reusing cifmw_ceph_spec_public_network +# cannot capture the value. Role params also do not leak to later plays. +# ceph.yml reads these facts instead of ansible.builtin.ini (broken on +# Python 3.12 / ConfigParser.readfp). +- name: Export networks written into the initial Ceph conf + ansible.builtin.set_fact: + cifmw_ceph_spec_rendered_public_network: "{{ cifmw_ceph_spec_public_network }}" + cifmw_ceph_spec_rendered_private_network: "{{ cifmw_ceph_spec_private_network }}" diff --git a/tests/integration/targets/filter_from_ini/tasks/main.yml b/tests/integration/targets/filter_from_ini/tasks/main.yml new file mode 100644 index 000000000..0f2333898 --- /dev/null +++ b/tests/integration/targets/filter_from_ini/tasks/main.yml @@ -0,0 +1,39 @@ +--- +- name: Sample Ceph initial conf + ansible.builtin.set_fact: + _ceph_conf: | + [global] + osd pool default size = 1 + public_network = 192.168.122.0/24 + cluster_network = 172.18.0.0/24 + fsid = 5b5fd001-8e21-4a0a-ba87-c12e77892b2c + path = /var/lib/ceph/%s/osd + [mon] + mon_warn_on_pool_no_redundancy = false + +- name: Test from_ini extracts Ceph keys + ansible.builtin.assert: + that: + - _ceph_conf | cifmw.general.from_ini('public_network') == '192.168.122.0/24' + - _ceph_conf | cifmw.general.from_ini('cluster_network') == '172.18.0.0/24' + - _ceph_conf | cifmw.general.from_ini('fsid') == '5b5fd001-8e21-4a0a-ba87-c12e77892b2c' + - _ceph_conf | cifmw.general.from_ini('osd pool default size') == '1' + - _ceph_conf | cifmw.general.from_ini('path') == '/var/lib/ceph/%s/osd' + - _ceph_conf | cifmw.general.from_ini('mon_warn_on_pool_no_redundancy', 'mon') == 'false' + - _ceph_conf | cifmw.general.from_ini('missing') == '' + - _ceph_conf | cifmw.general.from_ini('missing', 'global', 'unset') == 'unset' + +- name: Test from_ini bad argument + vars: + input: + - "not ini" + ansible.builtin.debug: + var: "input | cifmw.general.from_ini('public_network')" + register: _bad_from_ini_argument + ignore_errors: true + +- name: Verify from_ini showed an error message + ansible.builtin.assert: + that: + - _bad_from_ini_argument is failed + - "'from_ini requires INI content as a string' in _bad_from_ini_argument.msg" diff --git a/tests/unit/plugins/filter/test_from_ini.py b/tests/unit/plugins/filter/test_from_ini.py new file mode 100644 index 000000000..e0697a1fd --- /dev/null +++ b/tests/unit/plugins/filter/test_from_ini.py @@ -0,0 +1,122 @@ +# Copyright: (c) 2026, Red Hat + +# GNU General Public License v3.0+ (see COPYING or +# https://www.gnu.org/licenses/gpl-3.0.txt) +from __future__ import absolute_import, division, print_function + +import inspect +import unittest + +from ansible.errors import AnsibleFilterError, AnsibleFilterTypeError + +from ansible_collections.cifmw.general.plugins.filter.from_ini import ( + FilterModule, +) + + +CEPH_CONF = """[global] +osd pool default size = 1 +public_network = 192.168.122.0/24 +cluster_network = 172.18.0.0/24 +fsid = 5b5fd001-8e21-4a0a-ba87-c12e77892b2c +path = /var/lib/ceph/%s/osd +[mon] +mon_warn_on_pool_no_redundancy = false +""" + +# uni03gamma / OSPRH-6675 writes ctlplane, not the storage network range +OSPRH_6675_CONF = """[global] +public_network = 192.168.122.0/24 +""" + + +class TestFromIni(unittest.TestCase): + def setUp(self): + self.from_ini = FilterModule().filters()["from_ini"] + + def test_reads_public_network(self): + self.assertEqual( + self.from_ini(CEPH_CONF, "public_network"), + "192.168.122.0/24", + ) + + def test_reads_cluster_network(self): + self.assertEqual( + self.from_ini(CEPH_CONF, "cluster_network"), + "172.18.0.0/24", + ) + + def test_reads_fsid(self): + self.assertEqual( + self.from_ini(CEPH_CONF, "fsid"), + "5b5fd001-8e21-4a0a-ba87-c12e77892b2c", + ) + + def test_reads_option_with_spaces_in_name(self): + self.assertEqual( + self.from_ini(CEPH_CONF, "osd pool default size"), + "1", + ) + + def test_missing_key_returns_default(self): + self.assertEqual(self.from_ini(CEPH_CONF, "missing"), "") + self.assertEqual( + self.from_ini(CEPH_CONF, "missing", default="unset"), + "unset", + ) + + def test_missing_section_returns_default(self): + self.assertEqual( + self.from_ini(CEPH_CONF, "public_network", section="osd"), + "", + ) + + def test_other_section(self): + self.assertEqual( + self.from_ini( + CEPH_CONF, + "mon_warn_on_pool_no_redundancy", + section="mon", + ), + "false", + ) + + def test_interpolation_disabled(self): + # %s must not be treated as a ConfigParser interpolation marker + self.assertEqual( + self.from_ini(CEPH_CONF, "path"), + "/var/lib/ceph/%s/osd", + ) + + def test_bytes_input(self): + self.assertEqual( + self.from_ini(CEPH_CONF.encode("utf-8"), "public_network"), + "192.168.122.0/24", + ) + + def test_osp_rh_6675_ctlplane_not_storage(self): + self.assertEqual( + self.from_ini(OSPRH_6675_CONF, "public_network"), + "192.168.122.0/24", + ) + self.assertEqual( + self.from_ini(OSPRH_6675_CONF, "cluster_network"), + "", + ) + + def test_rejects_non_string_content(self): + with self.assertRaises(AnsibleFilterTypeError): + self.from_ini({"public_network": "x"}, "public_network") + + def test_rejects_empty_key(self): + with self.assertRaises(AnsibleFilterTypeError): + self.from_ini(CEPH_CONF, "") + + def test_invalid_ini_raises(self): + with self.assertRaises(AnsibleFilterError): + self.from_ini("[global\npublic_network = x", "public_network") + + def test_implementation_uses_read_file_not_readfp(self): + source = inspect.getsource(FilterModule) + self.assertIn("read_file", source) + self.assertNotIn("readfp", source)