Implement granular Ansible verbosity control via configuration file

Introduce a new `verbosity` field under the `ansible` section in the
global YAML configuration to allow fine-grained control over Ansible's
log level, replacing the previous reliance on the global `--verbose`
CLI flag for subprocess execution.
Add `verbosity` validation logic to ensure values are integers
between 1 and 6.
Update Ansible command generation to dynamically set the `-v` flag
count based on the new configuration field.
Update `sap_hana_install` documentation to clarify the relationship
between `sap_hana_install_update_firewall` and the global `firewall_cfg`.

Author: mpagot

ansible/playbooks/roles/sap_hana_install/README.md

@@ -147,8 +147,9 @@ or if it will add further hosts to an existing SAP HANA system as specified by v
 `sap_hana_install_addhosts`. Default is `yes` for a fresh SAP HANA installation.
 
 The role can be configured to also set the required firewall ports for SAP HANA. If this is desired, set
-the variable `sap_hana_install_update_firewall` to `true` (default is `false` that means the role will not touch
-any firewall related system settings, leaving whatever the system has unchanged). The firewall ports are defined
+the variable `sap_hana_install_update_firewall` to `true` (default is `false`). Note that in this project,
+this variable is automatically influenced by the global `firewall_cfg` variable defined in `hana_vars.yaml`.
+If `firewall_cfg` is set to `enable`, the firewall ports will be managed. The firewall ports are defined
 in a variable which is compatible with the variable structure used by Linux System Role `firewall`.
 The firewall ports for SAP HANA are defined in member `port` of the first field of variable
 `sap_hana_install_firewall` (`sap_hana_install_firewall[0].port`), see file `defaults/main.yml`. If the

scripts/qesap/lib/cmds.py

@@ -140,15 +140,14 @@ def cmd_configure(configure_data, base_project, dryrun):
     return Status("ok")
 
 
-def cmd_deploy(configure_data, base_project, dryrun=False, verbose=False):
+def cmd_deploy(configure_data, base_project, dryrun=False):
     """Main executor for the deploy sub-command
 
     Args:
         configure_data (obj): configuration structure
         base_project (str): base project path where to
                       look for the Terraform and Ansible files
         dryrun (bool): enable dryrun execution mode
-        verbose (bool): enable more verbosity
 
     Returns:
         int: execution result, 0 means OK. It is mind to be used as script exit code
@@ -161,26 +160,25 @@ def cmd_deploy(configure_data, base_project, dryrun=False, verbose=False):
     )
     if res != 0:
         return res
-    return cmd_ansible(configure_data, base_project, dryrun, verbose, destroy=False)
+    return cmd_ansible(configure_data, base_project, dryrun, destroy=False)
 
 
-def cmd_destroy(configure_data, base_project, dryrun=False, verbose=False):
+def cmd_destroy(configure_data, base_project, dryrun=False):
     """Main executor for the deploy sub-command
 
     Args:
         configure_data (obj): configuration structure
         base_project (str): base project path where to
                       look for the Terraform and Ansible files
         dryrun (bool): enable dryrun execution mode
-        verbose (bool): enable more verbosity
 
     Returns:
         int: execution result, 0 means OK. It is mind to be used as script exit code
     """
     config = CONF(configure_data)
     if not config.validate():
         return Status(f"Invalid configuration file content in {configure_data}")
-    res = cmd_ansible(configure_data, base_project, dryrun, verbose, destroy=True)
+    res = cmd_ansible(configure_data, base_project, dryrun, destroy=True)
     if res != 0:
         return res
     return cmd_terraform(
@@ -294,7 +292,6 @@ def ansible_command_sequence(
     admin_user,
     base_project,
     sequence,
-    verbose,
     inventory,
     profile,
     junit,
@@ -308,7 +305,6 @@ def ansible_command_sequence(
         base_project (str): base project path where to
                       look for the Ansible files
         sequence (str): 'create' or 'destroy'
-        verbose (bool): enable more verbosity
         inventory (str): inventory.yaml file path
         profile (bool): enable task profile
         junit (str): enable junit report and provide folder where to store report
@@ -319,8 +315,7 @@ def ansible_command_sequence(
         list of strings, each of them is an anslble or ansible-playbook command
     """
 
-    # 1. Create the environment variable set
-    #    that will be used by any command
+    # Create the environment variable set that will be used by any command
     original_env = dict(os.environ)
     original_env["ANSIBLE_PIPELINING"] = "True"
     original_env["ANSIBLE_TIMEOUT"] = "20"
@@ -335,7 +330,7 @@ def ansible_command_sequence(
     if "roles_path" in configure_data_ansible:
         original_env["ANSIBLE_ROLES_PATH"] = configure_data_ansible["roles_path"]
 
-    # 2. Verify that the two needed binaries are usable
+    # Verify that the two needed binaries are usable
     ansible_bin_paths = {}
     for ansible_bin in ["ansible", "ansible-playbook"]:
         binpath = shutil.which(ansible_bin)
@@ -344,14 +339,13 @@ def ansible_command_sequence(
             return False, f"Missing binary {ansible_bin}"
         ansible_bin_paths[ansible_bin] = binpath
 
-    # 3. Compose common arguments applicable to both 'ansible' and 'ansible-playbook'.
+    # Compose common arguments applicable to both 'ansible' and 'ansible-playbook'.
     ansible_common = "-vv"
-    if verbose:
-        # add two more 'v' without any space
-        ansible_common += "vv"
+    if "verbosity" in configure_data_ansible:
+        ansible_common = "-" + "v" * int(configure_data_ansible["verbosity"])
     ansible_common += f" -i {inventory}"
 
-    # 4. Start composing and accumulating all needed commands in a list
+    # Start composing and accumulating all needed commands in a list
     ansible_cmd_seq = []
 
     if junit and not os.path.isdir(junit):
@@ -494,7 +488,6 @@ def cmd_ansible(
     configure_data,
     base_project,
     dryrun,
-    verbose,
     destroy=False,
     profile=False,
     junit=False,
@@ -507,7 +500,6 @@ def cmd_ansible(
         base_project (str): base project path where to
                       look for the Ansible files
         dryrun (bool): enable dryrun execution mode
-        verbose (bool): enable more verbosity
         destroy (bool): select the playbook list
         profile (bool): enable task profile
         junit (str): enable junit report and provide folder where to store it
@@ -560,7 +552,6 @@ def cmd_ansible(
         admin_user,
         base_project,
         selected_sequence,
-        verbose,
         inventory,
         profile,
         junit,

scripts/qesap/lib/config.py

@@ -319,6 +319,45 @@ def get_playbooks(self, sequence):
             return self.conf["ansible"][sequence]
         return self.conf["ansible"]["sequences"][sequence]
 
+    def _validate_ansible_sequence(self, sequence):
+        """
+        Validate the sequence part of the ansible configure.yaml
+        """
+        if not sequence:
+            return True
+        selected_seq = (
+            self.conf["ansible"]
+            if self.conf["apiver"] < 4
+            else self.conf["ansible"].get("sequences", {})
+        )
+        if sequence not in selected_seq or selected_seq[sequence] is None:
+            log.error(
+                "No Ansible playbooks to play in %s for sequence:%s",
+                self.conf["ansible"],
+                sequence,
+            )
+            return False
+        return True
+
+    @staticmethod
+    def validate_ansible_verbosity(ansible_conf):
+        """
+        Validate the verbosity part of the ansible configure.yaml
+        """
+        if "verbosity" in ansible_conf:
+            verbosity = ansible_conf["verbosity"]
+            try:
+                verbosity = int(verbosity)
+            except (ValueError, TypeError):
+                log.error("ansible verbosity must be an integer, got: %r", verbosity)
+                return False
+            if not 1 <= verbosity <= 6:
+                log.error(
+                    "ansible verbosity must be between 1 and 6, got: %d", verbosity
+                )
+                return False
+        return True
+
     def validate_ansible_config(self, sequence):
         """
         Validate the ansible part of the internal structure of the config.yaml
@@ -339,25 +378,15 @@ def validate_ansible_config(self, sequence):
             log.error("Ansible media configuration")
             return False
 
-        if sequence:
-            selected_seq = None
-            if self.conf["apiver"] < 4:
-                selected_seq = self.conf["ansible"]
-            else:
-                selected_seq = self.conf["ansible"]["sequences"]
-            if sequence not in selected_seq or selected_seq[sequence] is None:
-                log.error(
-                    "No Ansible playbooks to play in %s for sequence:%s",
-                    self.conf["ansible"],
-                    sequence,
-                )
-                return False
+        if not self._validate_ansible_sequence(sequence):
+            return False
 
         if "hana_vars" in self.conf["ansible"] and not validate_ansible_hana_var(
             self.conf["ansible"]["hana_vars"]
         ):
             return False
-        return True
+
+        return self.validate_ansible_verbosity(self.conf["ansible"])
 
     def validate_basedir(self, basedir):
         """

scripts/qesap/qesap.py

@@ -180,10 +180,10 @@ def run_subcommand(args):
         return cmd_configure(args.configdata, args.basedir, args.dryrun)
     if args.command == "deploy":
         log.info("Deploying...")
-        return cmd_deploy(args.configdata, args.basedir, args.dryrun, args.verbose)
+        return cmd_deploy(args.configdata, args.basedir, args.dryrun)
     if args.command == "destroy":
         log.info("Destroying...")
-        return cmd_destroy(args.configdata, args.basedir, args.dryrun, args.verbose)
+        return cmd_destroy(args.configdata, args.basedir, args.dryrun)
     if args.command == "terraform":
         log.info("Running Terraform...")
         return cmd_terraform(
@@ -200,7 +200,6 @@ def run_subcommand(args):
             args.configdata,
             args.basedir,
             args.dryrun,
-            args.verbose,
             destroy=args.destroy,
             profile=args.profile,
             junit=args.junit,

scripts/qesap/test/conftest.py

@@ -176,7 +176,7 @@ def _callback(playbook_list):
 
 @pytest.fixture
 def ansible_config():
-    def _callback(provider, playbooks, apiver=3):
+    def _callback(provider, playbooks, apiver=3, verbosity=None):
         config_content = f"""---
 apiver: {apiver}
 provider: {provider}
@@ -187,6 +187,9 @@ def _callback(provider, playbooks, apiver=3):
     hana_media:
     - pippo"""
 
+        if verbosity is not None:
+            config_content += f"\n    verbosity: {verbosity}"
+
         if apiver < 4:
             for seq in playbooks:
                 config_content += f"\n    {seq}:"

scripts/qesap/test/e2e/ansible_tests.sh

@@ -365,3 +365,28 @@ test_split
 echo "Run the script again without dryrun"
 PATH=$TROOT:$PATH qesap.py -b ${QESAPROOT} -c ${QESAP_CFG} ansible -d |& tee "${THIS_LOG}"
 [[ ! -f "${FILE_TOUCH_BY_ANSIBLE}" ]] || test_die "File ${FILE_TOUCH_BY_ANSIBLE} has to be deleted by ribes_nero.yaml"
+
+#######################################################################
+QESAP_CFG=test_11.yaml
+test_step "[${QESAP_CFG}] Run Ansible with verbosity 0"
+THIS_LOG="${QESAPROOT}/ansible_v0.log"
+reset_root
+cp sambuconero.yaml "${QESAPROOT}/ansible/playbooks/"
+cp inventory.yaml "${TEST_PROVIDER}/"
+qesap.py -b ${QESAPROOT} -c ${QESAP_CFG} --dryrun ansible |& tee "${THIS_LOG}"
+set +e
+grep -E "ansible.*-v" "${THIS_LOG}"
+rc=$?; [[ $rc -ne 0 ]] || test_die "${QESAP_CFG} should not have -v in ansible commands"
+set -e
+rm "${THIS_LOG}"
+
+#######################################################################
+QESAP_CFG=test_12.yaml
+test_step "[${QESAP_CFG}] Run Ansible with verbosity 4"
+THIS_LOG="${QESAPROOT}/ansible_v4.log"
+reset_root
+cp sambuconero.yaml "${QESAPROOT}/ansible/playbooks/"
+cp inventory.yaml "${TEST_PROVIDER}/"
+qesap.py -b ${QESAPROOT} -c ${QESAP_CFG} --dryrun ansible |& tee "${THIS_LOG}"
+grep -E "ansible.*-vvvv" "${THIS_LOG}" || test_die "${QESAP_CFG} should have -vvvv in ansible commands"
+rm "${THIS_LOG}"

scripts/qesap/test/e2e/test_10.yaml

@@ -5,6 +5,7 @@ terraform:
   variables:
     fruit: lampone
 ansible:
+  verbosity: 4
   hana_media:
     - mora
     - corniolo

scripts/qesap/test/e2e/test_11.yaml

@@ -0,0 +1,17 @@
+---
+apiver: 3
+provider: fragola
+terraform:
+  variables:
+    fruit: lampone
+ansible:
+  verbosity: 0
+  hana_media:
+    - mora
+    - corniolo
+  hana_urls: mirtillo
+  az_storage_account_name: ribes
+  az_container_name: uvaspina
+  az_sas_token: '***'
+  create:
+    - sambuconero.yaml

scripts/qesap/test/e2e/test_12.yaml

@@ -0,0 +1,17 @@
+---
+apiver: 3
+provider: fragola
+terraform:
+  variables:
+    fruit: lampone
+ansible:
+  verbosity: 4
+  hana_media:
+    - mora
+    - corniolo
+  hana_urls: mirtillo
+  az_storage_account_name: ribes
+  az_container_name: uvaspina
+  az_sas_token: '***'
+  create:
+    - sambuconero.yaml

scripts/qesap/test/unit/test_qesap_ansible.py

@@ -83,18 +83,17 @@ def test_ansible_verbose(
     mock_call_ansibleplaybook,
 ):
     """
-    run with -vvvv if qesap ansible --verbose
-    (probably not supported in qesap deploy/destroy)
+    run with -vvvv if config.yaml has verbosity: 4
     """
     provider = "grilloparlante"
     playbooks = {"create": ["get_cherry_wood"]}
 
-    config_content = ansible_config(provider, playbooks)
+    config_content = ansible_config(provider, playbooks, verbosity=4)
     config_file_name = str(tmpdir / "config.yaml")
     with open(config_file_name, "w", encoding="utf-8") as file:
         file.write(config_content)
 
-    args = base_args(None, config_file_name, True)
+    args = base_args(None, config_file_name, False)
     args.append("ansible")
     run.return_value = (0, [])