Update configuration reconciliation logic (#286)

* Add reconcile_service_config() function with check/retry
* Update to use reconcile_service_config utility
* Allow test connections to skip TLS verification
* Update resolve_parameter_updates() to filter out redacted values
* Update reconcile_service_config() to allow for redacted value filtering
* Remove parcel activation from host module
* Add redacted_skipped flag to service module
* Update role config group creation logic to handle multiple base groups
* Rename resolve_parameter_updates() to resolve_parameter_changeset()
* Update skip_redacted parameter
* Add reconcile_config_list_updates() function to replace ConfigListUpdates class
* Switch from ConfigListUpdates class to reconcile_config_list_updates() function
* Update function name change for resolve_parameter_changeset()
* Skip tests for deprecated modules
* Update service_type_info tests to use fixture factory
* Add skip_redacted and message parameters throughout logic
* Convert to use reconcile_config_list_updates() function

Signed-off-by: Webster Mudge <[email protected]>

Author: wmudge

plugins/module_utils/cm_utils.py

@@ -130,8 +130,11 @@ def _normalize(value):
     return {k: _normalize(v) for k, v in add.items()}
 
 
-def resolve_parameter_updates(
-    current: dict, incoming: dict, purge: bool = False
+def resolve_parameter_changeset(
+    current: dict,
+    incoming: dict,
+    purge: bool = False,
+    skip_redacted: bool = False,
 ) -> dict:
     """Produce a change set between two parameter dictionaries.
 
@@ -142,6 +145,7 @@ def resolve_parameter_updates(
         current (dict): Existing parameters
         incoming (dict): Declared parameters
         purge (bool, optional): Flag to reset any current parameters not found in the declared set. Defaults to False.
+        skip_redacted (bool, optional): Flag to not include parameters with REDACTED values in the changeset. Defaults to False.
 
     Returns:
         dict: A change set of the updates
@@ -159,7 +163,9 @@ def resolve_parameter_updates(
         updates = {
             k: v
             for k, v in diff[1].items()
-            if k in current or (k not in current and v is not None)
+            if (k in current and not skip_redacted)
+            or (k in current and (skip_redacted and current[k] != "REDACTED"))
+            or (k not in current and v is not None)
         }
 
         if purge:
@@ -172,6 +178,40 @@ def resolve_parameter_updates(
     return updates
 
 
+def reconcile_config_list_updates(
+    existing: ApiConfigList,
+    config: dict,
+    purge: bool = False,
+    skip_redacted: bool = False,
+) -> tuple[ApiConfigList, dict, dict]:
+    """Return a reconciled configuration list and the change deltas.
+
+    The function will normalize parameter values to remove whitespace from strings and
+    convert integers and Booleans to their string representations. Optionally, the
+    function will reset undeclared parameters or skip parameters that it is unable to
+    resolve, i.e. REDACTED parameter values.
+
+    Args:
+        existing (ApiConfigList): Existing parameters
+        config (dict): Declared parameters
+        purge (bool, optional): Flag to reset any current parameters not found in the declared set. Defaults to False.
+        skip_redacted (bool, optional): Flag to not include parameters with REDACTED values in the changeset. Defaults to False.
+    Returns:
+        tuple[ApiConfigList, dict, dict]: Updated configuration list and the before and after deltas
+    """
+    current = {r.name: r.value for r in existing.items}
+    changeset = resolve_parameter_changeset(current, config, purge, skip_redacted)
+
+    before = {k: current[k] if k in current else None for k in changeset.keys()}
+    after = changeset
+
+    reconciled_config = ApiConfigList(
+        items=[ApiConfig(name=k, value=v) for k, v in changeset.items()]
+    )
+
+    return (reconciled_config, before, after)
+
+
 def resolve_tag_updates(
     current: dict, incoming: dict, purge: bool = False
 ) -> tuple[dict, dict]:
@@ -224,7 +264,7 @@ def changed(self) -> bool:
 class ConfigListUpdates(object):
     def __init__(self, existing: ApiConfigList, updates: dict, purge: bool) -> None:
         current = {r.name: r.value for r in existing.items}
-        changeset = resolve_parameter_updates(current, updates, purge)
+        changeset = resolve_parameter_changeset(current, updates, purge)
 
         self.diff = dict(
             before={k: current[k] if k in current else None for k in changeset.keys()},

plugins/module_utils/host_utils.py

@@ -44,9 +44,9 @@
 
 from ansible_collections.cloudera.cluster.plugins.module_utils.cm_utils import (
     normalize_output,
+    reconcile_config_list_updates,
     wait_command,
     wait_bulk_commands,
-    ConfigListUpdates,
 )
 from ansible_collections.cloudera.cluster.plugins.module_utils.host_template_utils import (
     HostTemplateException,
@@ -243,6 +243,7 @@ def reconcile_host_role_configs(
     role_configs: list[dict],  # service, type, and config (optional)
     purge: bool,
     check_mode: bool,
+    skip_redacted: bool,
     message: str = None,
 ) -> tuple[list[dict], list[dict]]:
 
@@ -275,15 +276,22 @@ def reconcile_host_role_configs(
         if incoming_role_config["config"] or purge:
             incoming_config = incoming_role_config.get("config", dict())
 
-            updates = ConfigListUpdates(current_role.config, incoming_config, purge)
+            (
+                updated_config,
+                config_before,
+                config_after,
+            ) = reconcile_config_list_updates(
+                current_role.config,
+                incoming_config,
+                purge,
+                skip_redacted,
+            )
 
-            if updates.changed:
-                diff_before.append(
-                    dict(name=current_role.name, config=current_role.config)
-                )
-                diff_after.append(dict(name=current_role.name, config=updates.config))
+            if config_before or config_after:
+                diff_before.append(dict(name=current_role.name, config=config_before))
+                diff_after.append(dict(name=current_role.name, config=config_after))
 
-                current_role.config = updates.config
+                current_role.config = updated_config
 
                 if not check_mode:
                     role_api.update_role_config(
@@ -303,6 +311,7 @@ def reconcile_host_role_config_groups(
     host: ApiHost,
     role_config_groups: list[dict],  # service, type (optional), name (optional)
     purge: bool,
+    skip_redacted: bool,
     check_mode: bool,
 ) -> tuple[list[dict], list[dict]]:
 
@@ -350,6 +359,7 @@ def reconcile_host_role_config_groups(
         cluster_rcgs=cluster_rcgs,
     )
 
+    # TODO Validate if the parcel staging check is still needed
     # Read the parcel states for the cluster until all are at a stable stage
     wait_parcel_staging(
         api_client=api_client,

plugins/module_utils/role_config_group_utils.py

@@ -14,7 +14,7 @@
 
 from ansible_collections.cloudera.cluster.plugins.module_utils.cm_utils import (
     normalize_output,
-    ConfigListUpdates,
+    reconcile_config_list_updates,
 )
 from ansible_collections.cloudera.cluster.plugins.module_utils.role_utils import (
     InvalidRoleTypeException,
@@ -120,6 +120,7 @@ def update_role_config_group(
     display_name: str = None,
     config: dict = None,
     purge: bool = False,
+    skip_redacted: bool = False,
 ) -> tuple[ApiRoleConfigGroup, dict, dict]:
     before, after = dict(), dict()
 
@@ -134,12 +135,14 @@ def update_role_config_group(
         if config is None:
             config = dict()
 
-        updates = ConfigListUpdates(role_config_group.config, config, purge)
+        (updated_config, config_before, config_after) = reconcile_config_list_updates(
+            role_config_group.config, config, purge, skip_redacted
+        )
 
-        if updates.changed:
-            before.update(config=updates.diff["before"])
-            after.update(config=updates.diff["after"])
-            role_config_group.config = updates.config
+        if config_before or config_after:
+            before.update(config=config_before)
+            after.update(config=config_after)
+            role_config_group.config = updated_config
 
     return (role_config_group, before, after)
 

plugins/module_utils/service_utils.py

@@ -18,10 +18,10 @@
 
 from ansible_collections.cloudera.cluster.plugins.module_utils.cm_utils import (
     normalize_output,
-    resolve_parameter_updates,
+    reconcile_config_list_updates,
+    resolve_parameter_changeset,
     wait_command,
     wait_commands,
-    ConfigListUpdates,
     TagUpdates,
 )
 from ansible_collections.cloudera.cluster.plugins.module_utils.role_config_group_utils import (
@@ -388,10 +388,80 @@ def read_cm_service(api_client: ApiClient) -> ApiService:
     return service
 
 
+def reconcile_service_config(
+    api_client: ApiClient,
+    service: ApiService,
+    config: dict,
+    purge: bool,
+    check_mode: bool,
+    skip_redacted: bool,
+    message: str,
+) -> tuple[dict, dict]:
+    service_api = ServicesResourceApi(api_client)
+
+    def _handle_config(
+        existing: ApiServiceConfig,
+    ) -> tuple[ApiServiceConfig, dict, dict]:
+        current = {r.name: r.value for r in existing.items}
+        changeset = resolve_parameter_changeset(current, config, purge, skip_redacted)
+
+        before = {k: current[k] if k in current else None for k in changeset.keys()}
+        after = changeset
+
+        reconciled_config = ApiServiceConfig(
+            items=[ApiConfig(name=k, value=v) for k, v in changeset.items()]
+        )
+
+        return (reconciled_config, before, after)
+
+    initial_before = dict()
+    initial_after = dict()
+    retry = 0
+
+    while retry < 3:
+        existing_config = service_api.read_service_config(
+            cluster_name=service.cluster_ref.cluster_name,
+            service_name=service.name,
+        )
+
+        (updated_config, before, after) = _handle_config(existing_config)
+
+        if (before or after) and not check_mode:
+            if retry == 0:
+                initial_before, initial_after = before, after
+
+            service_api.update_service_config(
+                cluster_name=service.cluster_ref.cluster_name,
+                service_name=service.name,
+                message=message,
+                body=updated_config,
+            )
+
+            config_check = service_api.read_service_config(
+                cluster_name=service.cluster_ref.cluster_name,
+                service_name=service.name,
+            )
+
+            (_, checked_before, checked_after) = _handle_config(config_check)
+
+            if not checked_before or not checked_after:
+                return (initial_before, initial_after)
+            else:
+                retry += 1
+        else:
+            return (before, after)
+
+    raise ServiceException(
+        f"Unable to reconcile service-wide configuration for '{service.name}' in cluster '{service.cluster_ref.cluster_name}",
+        before,
+        after,
+    )
+
+
 class ServiceConfigUpdates(object):
     def __init__(self, existing: ApiServiceConfig, updates: dict, purge: bool) -> None:
         current = {r.name: r.value for r in existing.items}
-        changeset = resolve_parameter_updates(current, updates, purge)
+        changeset = resolve_parameter_changeset(current, updates, purge)
 
         self.before = {
             k: current[k] if k in current else None for k in changeset.keys()
@@ -436,7 +506,10 @@ def reconcile_service_role_config_groups(
     role_config_groups: list[dict],
     purge: bool,
     check_mode: bool,
+    skip_redacted: bool,
+    message: str,
 ) -> tuple[dict, dict]:
+
     # Map the current role config groups by name and by base role type
     base_rcg_map, rcg_map = dict(), dict()
     for rcg in service.role_config_groups:
@@ -463,6 +536,7 @@ def reconcile_service_role_config_groups(
                     display_name=incoming_rcg["display_name"],
                     config=incoming_rcg["config"],
                     purge=purge,
+                    skip_redacted=skip_redacted,
                 )
 
                 if before or after:
@@ -475,6 +549,7 @@ def reconcile_service_role_config_groups(
                             service_name=service.name,
                             role_config_group_name=current_rcg.name,
                             body=updated_rcg,
+                            message=message,
                         )
 
             # Else create the new custom role config group
@@ -499,6 +574,7 @@ def reconcile_service_role_config_groups(
                 display_name=incoming_rcg["display_name"],
                 config=incoming_rcg["config"],
                 purge=purge,
+                skip_redacted=skip_redacted,
             )
 
             if before or after:
@@ -511,6 +587,7 @@ def reconcile_service_role_config_groups(
                         service_name=service.name,
                         role_config_group_name=current_rcg.name,
                         body=updated_rcg,
+                        message=message,
                     )
 
     # Process role config group additions
@@ -529,6 +606,7 @@ def reconcile_service_role_config_groups(
             (updated_rcg, before, after) = update_role_config_group(
                 role_config_group=current_rcg,
                 purge=purge,
+                skip_redacted=skip_redacted,
             )
 
             if before or after:
@@ -541,6 +619,7 @@ def reconcile_service_role_config_groups(
                         service_name=service.name,
                         role_config_group_name=current_rcg.name,
                         body=updated_rcg,
+                        message=message,
                     )
 
         # Reset to base and remove any remaining custom role config groups
@@ -578,6 +657,8 @@ def reconcile_service_roles(
     roles: list[dict],
     purge: bool,
     check_mode: bool,
+    skip_redacted: bool,
+    message: str,
     # maintenance: bool,
     # state: str,
 ) -> tuple[dict, dict]:
@@ -656,22 +737,30 @@ def reconcile_service_roles(
                         if incoming_config is None:
                             incoming_config = dict()
 
-                        updates = ConfigListUpdates(
-                            current_role.config, incoming_config, purge
+                        (
+                            updated_config,
+                            config_before,
+                            config_after,
+                        ) = reconcile_config_list_updates(
+                            current_role.config,
+                            incoming_config,
+                            purge,
+                            skip_redacted,
                         )
 
-                        if updates.changed:
-                            instance_role_before.update(config=current_role.config)
-                            instance_role_after.update(config=updates.config)
+                        if config_before or config_after:
+                            instance_role_before.update(config=config_before)
+                            instance_role_after.update(config=config_after)
 
-                            current_role.config = updates.config
+                            current_role.config = updated_config
 
                         if not check_mode:
                             role_api.update_role_config(
                                 cluster_name=service.cluster_ref.cluster_name,
                                 service_name=service.name,
                                 role_name=current_role.name,
                                 body=current_role.config,
+                                message=message,
                             )
 
                     # Reconcile role tags