Fix notification of individual services when related config changes, even when roles are grouped
All checks were successful
CI / test (push) Successful in 19m18s
Lint / test (push) Successful in 42s

This commit is contained in:
Miguel Jacq 2026-06-20 15:31:42 +10:00
parent 08066595f1
commit 097022f782
Signed by: mig5
GPG key ID: 03906B4110AAD3B8
8 changed files with 472 additions and 62 deletions

View file

@ -1147,16 +1147,24 @@ def _single_service_restart_handler_body(var_prefix: str) -> str:
"""
def _grouped_service_restart_handler_body(var_prefix: str) -> str:
return f"""- name: Restart managed services
def _service_restart_handler_name(unit: str) -> str:
return f"Restart managed service {unit}"
def _grouped_service_restart_handlers_body(role: AnsibleRole) -> str:
handlers: List[str] = []
for unit, svc in sorted(role.services.items()):
name = str(svc.get("name") or unit).strip()
if not name or str(svc.get("state") or "stopped") != "started":
continue
handlers.append(
f"""- name: {_service_restart_handler_name(name)}
ansible.builtin.service:
name: "{{{{ item.name }}}}"
name: {name}
state: restarted
loop: "{{{{ {var_prefix}_systemd_units | default([]) }}}}"
when:
- item.manage | default(false)
- (item.state | default('stopped')) == 'started'
"""
)
return "\n".join(_task_body(handler) for handler in handlers if _task_body(handler))
def _render_role_handlers(
@ -1165,6 +1173,7 @@ def _render_role_handlers(
systemd_reload: bool = False,
single_service: bool = False,
grouped_services: bool = False,
restart_grouped_services: bool = False,
sysctl: bool = False,
firewall_runtime: bool = False,
extra_handlers: str = "",
@ -1174,8 +1183,8 @@ def _render_role_handlers(
parts.append(_SYSTEMD_DAEMON_RELOAD_HANDLER)
if single_service:
parts.append(_single_service_restart_handler_body(role.var_prefix))
if grouped_services:
parts.append(_grouped_service_restart_handler_body(role.var_prefix))
if grouped_services and restart_grouped_services:
parts.append(_grouped_service_restart_handlers_body(role))
if sysctl:
parts.append(_render_sysctl_handlers(role.var_prefix))
if firewall_runtime:
@ -1242,7 +1251,7 @@ def _build_managed_files_var(
managed_files: List[Dict[str, Any]],
templated_src_rels: Set[str],
*,
notify_other: Optional[str] = None,
notify_other: Optional[Any] = None,
notify_systemd: Optional[str] = None,
) -> List[Dict[str, Any]]:
"""Convert enroll managed_files into an Ansible-friendly list of dicts.
@ -1261,19 +1270,23 @@ def _build_managed_files_var(
if is_unit and notify_systemd:
notify.append(notify_systemd)
if (not is_unit) and notify_other:
notify.append(notify_other)
out.append(
{
"dest": dest,
"src_rel": src_rel,
"owner": mf.get("owner") or "root",
"group": mf.get("group") or "root",
"mode": mf.get("mode") or "0644",
"kind": kind,
"is_systemd_unit": bool(is_unit),
"notify": notify,
}
)
if isinstance(notify_other, (list, tuple, set)):
notify.extend(str(item) for item in notify_other if str(item))
else:
notify.append(str(notify_other))
item = {
"dest": dest,
"src_rel": src_rel,
"owner": mf.get("owner") or "root",
"group": mf.get("group") or "root",
"mode": mf.get("mode") or "0644",
"kind": kind,
"is_systemd_unit": bool(is_unit),
}
if notify:
item["notify"] = notify
out.append(item)
return out
@ -1751,6 +1764,7 @@ def _role_managed_content_vars(
entries: List[Dict[str, Any]],
*,
notify_by_kind: Optional[Dict[str, Optional[str]]] = None,
notify_service_handlers: bool = False,
overwrite_templates: bool,
) -> Tuple[
List[Dict[str, Any]],
@ -1766,6 +1780,7 @@ def _role_managed_content_vars(
seen_files: Set[Tuple[Any, Any, Any]] = set()
seen_dirs: Set[Tuple[Any, Any, Any, Any]] = set()
seen_links: Set[Tuple[Any, Any]] = set()
service_units_by_package = CMModule.active_service_units_by_package(entries)
for entry in entries:
kind = str(entry.get("kind") or "package")
@ -1789,10 +1804,25 @@ def _role_managed_content_vars(
)
_copy_role_artifacts(ctx, role, source_role, exclude_rels=templated)
notify_other = (notify_by_kind or {}).get(kind)
if notify_service_handlers and kind == "service":
unit = str(snap.get("unit") or "").strip()
if unit and str(snap.get("active_state") or "") == "active":
notify_other = _service_restart_handler_name(unit)
else:
notify_other = None
elif notify_service_handlers and kind == "package":
notify_other = [
_service_restart_handler_name(unit)
for unit in CMModule.active_service_units_for_package_snapshot(
snap, service_units_by_package
)
]
for item in _build_managed_files_var(
managed_files,
templated,
notify_other=(notify_by_kind or {}).get(kind),
notify_other=notify_other,
notify_systemd="Run systemd daemon-reload",
):
key = (item.get("dest"), item.get("src_rel"), item.get("kind"))
@ -1896,6 +1926,8 @@ def _write_resource_ansible_role(
site_defaults: Optional[Dict[str, Any]] = None,
single_service: bool = False,
grouped_services: bool = False,
restart_grouped_services: bool = False,
notify_service_handlers: bool = False,
systemd_reload: bool = False,
) -> str:
files_var, dirs_var, links_var, jt_vars = _role_managed_content_vars(
@ -1903,6 +1935,7 @@ def _write_resource_ansible_role(
role.role_name,
role.entries,
notify_by_kind=notify_by_kind,
notify_service_handlers=notify_service_handlers,
overwrite_templates=overwrite_templates,
)
vars_map = _resource_role_vars(
@ -1930,6 +1963,7 @@ def _write_resource_ansible_role(
systemd_reload=systemd_reload,
single_service=single_service,
grouped_services=grouped_services,
restart_grouped_services=restart_grouped_services,
),
)
@ -1998,10 +2032,12 @@ def _render_common_ansible_roles(
_write_resource_ansible_role(
ctx,
role,
notify_by_kind={"service": "Restart managed services"},
notify_by_kind={"service": None},
overwrite_templates=True,
extra_vars={f"{role.var_prefix}_systemd_units": systemd_units},
grouped_services=True,
restart_grouped_services=True,
notify_service_handlers=True,
)
_add_role(rendered_roles, role.role_name)

View file

@ -411,6 +411,78 @@ class CMModule:
)
yield {"kind": "package", "snapshot": pkg, "role_label": role_label}
@staticmethod
def active_service_units_by_package(
entries: Iterable[Mapping[str, Any]],
) -> Dict[str, List[Dict[str, str]]]:
"""Return active service units keyed by the packages that produced them.
Renderers use this when a package-owned managed file should refresh the
service that package provides. The helper is deliberately conservative:
stopped/inactive services are not included, and ambiguous package->many
service mappings are left to the renderer/caller to resolve.
"""
by_package: Dict[str, List[Dict[str, str]]] = {}
for entry in entries:
if str(entry.get("kind") or "package") != "service":
continue
snap = entry.get("snapshot") or {}
if not isinstance(snap, Mapping):
continue
unit = str(snap.get("unit") or "").strip()
if not unit or str(snap.get("active_state") or "") != "active":
continue
role_name = str(snap.get("role_name") or unit).strip()
for pkg in snap.get("packages", []) or []:
package = str(pkg or "").strip()
if package:
by_package.setdefault(package, []).append(
{"unit": unit, "role_name": role_name}
)
for package, services in list(by_package.items()):
seen: Set[str] = set()
unique: List[Dict[str, str]] = []
for svc in services:
unit = svc.get("unit") or ""
if unit and unit not in seen:
seen.add(unit)
unique.append(svc)
by_package[package] = sorted(unique, key=lambda svc: svc.get("unit", ""))
return by_package
@staticmethod
def active_service_units_for_package_snapshot(
package_snapshot: Mapping[str, Any],
service_units_by_package: Mapping[str, List[Dict[str, str]]],
) -> List[str]:
"""Return active service units that a package snapshot can safely refresh.
If one active service is associated with the package, return it. If
several are associated, only return a role-name match; otherwise avoid
guessing and return no services. This prevents package-level config from
recreating the old broad-restart problem.
"""
package = str(package_snapshot.get("package") or "").strip()
if not package:
return []
services = list(service_units_by_package.get(package) or [])
if len(services) == 1:
unit = services[0].get("unit") or ""
return [unit] if unit else []
role_name = str(package_snapshot.get("role_name") or "").strip()
if role_name:
matched = [
svc.get("unit") or ""
for svc in services
if svc.get("role_name") == role_name and svc.get("unit")
]
if matched:
return sorted(set(matched))
return []
def add_user_flatpaks_snapshot(self, snap: Dict[str, Any]) -> None:
home_by_user = {
str(u.get("name")): str(u.get("home") or "")

View file

@ -134,6 +134,7 @@ class PuppetRole(CMModule):
artifact_role: str,
module_files_dir: Path,
file_prefix: Optional[str] = None,
notify_services: Optional[List[str]] = None,
) -> None:
for d in self.managed_dirs_from_snapshot(snap):
path = str(d.get("path") or "").strip()
@ -162,14 +163,17 @@ class PuppetRole(CMModule):
f"Skipped {path}: harvested artifact {artifact_role}/{src_rel} was not present."
)
continue
self.add_managed_file(
path,
owner=mf.get("owner") or "root",
group=mf.get("group") or "root",
mode=mf.get("mode") or "0644",
source=_source_uri(self.module_name, module_rel),
reason=mf.get("reason") or "managed_file",
)
attrs: Dict[str, Any] = {
"owner": mf.get("owner") or "root",
"group": mf.get("group") or "root",
"mode": mf.get("mode") or "0644",
"source": _source_uri(self.module_name, module_rel),
"reason": mf.get("reason") or "managed_file",
}
if notify_services and not path.startswith("/etc/systemd/system/"):
refs = [f"Service[{_pp_quote(unit)}]" for unit in notify_services]
attrs["notify"] = refs[0] if len(refs) == 1 else f"[{', '.join(refs)}]"
self.add_managed_file(path, **attrs)
for ml in self.managed_links_from_snapshot(snap):
path = str(ml.get("path") or "").strip()
@ -561,9 +565,16 @@ def _collect_puppet_roles(
file_prefix=node_file_prefix,
)
for entry in CMModule.package_service_entries(
roles, inventory_packages, use_common_roles=use_common_modules
):
package_service_entries = list(
CMModule.package_service_entries(
roles, inventory_packages, use_common_roles=use_common_modules
)
)
service_units_by_package = CMModule.active_service_units_by_package(
package_service_entries
)
for entry in package_service_entries:
snap = entry.get("snapshot") or {}
kind = str(entry.get("kind") or "package")
fallback = "service" if kind == "service" else "package"
@ -576,16 +587,24 @@ def _collect_puppet_roles(
fallback="package_group" if use_common_modules else fallback,
)
prole = ensure_role(role_name)
notify_services: List[str] = []
if kind == "service":
prole.add_service_snapshot(snap)
unit = str(snap.get("unit") or "").strip()
if unit and str(snap.get("active_state") or "") == "active":
notify_services = [unit]
else:
prole.add_package_snapshot(snap)
notify_services = CMModule.active_service_units_for_package_snapshot(
snap, service_units_by_package
)
prole.add_managed_content(
snap,
bundle_dir=bundle_dir,
artifact_role=str(snap.get("role_name") or original_role_name),
module_files_dir=modules_dir / prole.module_name / "files",
file_prefix=node_file_prefix,
notify_services=notify_services,
)
container_images = roles.get("container_images") or {}
@ -709,6 +728,7 @@ def _render_role_class(prole: PuppetRole) -> str:
("owner", _pp_quote(f.get("owner") or "root")),
("group", _pp_quote(f.get("group") or "root")),
("mode", _pp_quote(f.get("mode") or "0644")),
*([("notify", str(f.get("notify")))] if f.get("notify") else []),
],
)
@ -1011,7 +1031,7 @@ def _role_hiera_values(prole: PuppetRole) -> Dict[str, Any]:
path: _attrs_with_ensure(
prole.files[path],
"file",
allowed={"source", "owner", "group", "mode"},
allowed={"source", "owner", "group", "mode", "notify"},
)
for path in sorted(prole.files)
}

View file

@ -49,6 +49,11 @@ class SaltRole(CMModule):
self.add_service_snapshot_state(
snap, state_key="state", running="running", stopped="dead"
)
unit = self.service_unit_from_snapshot(snap)
if unit in self.services:
self.services[unit]["state_id"] = _state_id(
"service", unit, role=self.module_name
)
def add_users_snapshot(self, snap: Dict[str, Any]) -> None:
records = self.user_records_from_snapshot(snap)
@ -144,6 +149,7 @@ class SaltRole(CMModule):
jt_exe: Optional[str] = None,
jt_enabled: bool = False,
overwrite_templates: bool = True,
watch_services: Optional[List[str]] = None,
) -> None:
for d in self.managed_dirs_from_snapshot(snap):
path = str(d.get("path") or "").strip()
@ -174,17 +180,22 @@ class SaltRole(CMModule):
)
if template is not None:
tmpl_rel, context = template
self.add_managed_file(
path,
user=mf.get("owner") or "root",
group=mf.get("group") or "root",
mode=mf.get("mode") or "0644",
source=_template_source_uri(self.module_name, tmpl_rel),
template="jinja",
context=context,
makedirs=True,
reason=mf.get("reason") or "managed_file",
)
attrs: Dict[str, Any] = {
"user": mf.get("owner") or "root",
"group": mf.get("group") or "root",
"mode": mf.get("mode") or "0644",
"source": _template_source_uri(self.module_name, tmpl_rel),
"template": "jinja",
"context": context,
"makedirs": True,
"reason": mf.get("reason") or "managed_file",
}
if watch_services and not path.startswith("/etc/systemd/system/"):
attrs["watch_in"] = [
{"service": _state_id("service", unit, role=self.module_name)}
for unit in watch_services
]
self.add_managed_file(path, **attrs)
continue
role_rel = _copy_artifact(
@ -199,15 +210,20 @@ class SaltRole(CMModule):
f"Skipped {path}: harvested artifact {artifact_role}/{src_rel} was not present."
)
continue
self.add_managed_file(
path,
user=mf.get("owner") or "root",
group=mf.get("group") or "root",
mode=mf.get("mode") or "0644",
source=_source_uri(self.module_name, role_rel),
makedirs=True,
reason=mf.get("reason") or "managed_file",
)
attrs = {
"user": mf.get("owner") or "root",
"group": mf.get("group") or "root",
"mode": mf.get("mode") or "0644",
"source": _source_uri(self.module_name, role_rel),
"makedirs": True,
"reason": mf.get("reason") or "managed_file",
}
if watch_services and not path.startswith("/etc/systemd/system/"):
attrs["watch_in"] = [
{"service": _state_id("service", unit, role=self.module_name)}
for unit in watch_services
]
self.add_managed_file(path, **attrs)
for ml in self.managed_links_from_snapshot(snap):
path = str(ml.get("path") or "").strip()
@ -615,9 +631,16 @@ def _collect_salt_roles(
overwrite_templates=not bool(fqdn),
)
for entry in CMModule.package_service_entries(
roles, inventory_packages, use_common_roles=use_common_roles
):
package_service_entries = list(
CMModule.package_service_entries(
roles, inventory_packages, use_common_roles=use_common_roles
)
)
service_units_by_package = CMModule.active_service_units_by_package(
package_service_entries
)
for entry in package_service_entries:
snap = entry.get("snapshot") or {}
kind = str(entry.get("kind") or "package")
fallback = "service" if kind == "service" else "package"
@ -630,10 +653,17 @@ def _collect_salt_roles(
fallback="package_group" if use_common_roles else fallback,
)
srole = ensure_role(role_name)
watch_services: List[str] = []
if kind == "service":
srole.add_service_snapshot(snap)
unit = str(snap.get("unit") or "").strip()
if unit and str(snap.get("active_state") or "") == "active":
watch_services = [unit]
else:
srole.add_package_snapshot(snap)
watch_services = CMModule.active_service_units_for_package_snapshot(
snap, service_units_by_package
)
srole.add_managed_content(
snap,
bundle_dir=bundle_dir,
@ -643,6 +673,7 @@ def _collect_salt_roles(
jt_exe=jt_exe,
jt_enabled=jt_enabled,
overwrite_templates=not bool(fqdn),
watch_services=watch_services,
)
container_images = roles.get("container_images") or {}
@ -798,6 +829,12 @@ def _render_static_role(srole: SaltRole) -> str:
lines.append(f" - template: {_yaml_quote(attrs.get('template'))}")
if attrs.get("context"):
_append_yaml_value(lines, "context", attrs.get("context"), indent=4)
if attrs.get("watch_in"):
lines.append(" - watch_in:")
for req in attrs.get("watch_in") or []:
if isinstance(req, dict):
for req_kind, req_name in req.items():
lines.append(f" - {req_kind}: {_yaml_quote(req_name)}")
lines.append("")
for path, attrs in sorted(srole.links.items()):
@ -817,7 +854,7 @@ def _render_static_role(srole: SaltRole) -> str:
state_fun = "running" if svc.get("state") == "running" else "dead"
lines.extend(
[
f"{_state_id('service', name, role=srole.module_name)}:",
f"{svc.get('state_id') or _state_id('service', name, role=srole.module_name)}:",
f" service.{state_fun}:",
f" - name: {_yaml_quote(svc.get('name') or name)}",
f" - enable: {_yaml_bool(svc.get('enable', False))}",
@ -1054,6 +1091,9 @@ def _role_pillar_values(srole: SaltRole) -> Dict[str, Any]:
{"template": attrs.get("template")} if attrs.get("template") else {}
),
**({"context": attrs.get("context")} if attrs.get("context") else {}),
**(
{"watch_in": attrs.get("watch_in")} if attrs.get("watch_in") else {}
),
}
for path, attrs in sorted(srole.files.items())
}
@ -1072,6 +1112,8 @@ def _role_pillar_values(srole: SaltRole) -> Dict[str, Any]:
"name": svc.get("name") or name,
"state": "running" if svc.get("state") == "running" else "dead",
"enable": bool(svc.get("enable", False)),
"state_id": svc.get("state_id")
or _state_id("service", name, role=srole.module_name),
}
for name, svc in sorted(srole.services.items())
}
@ -1171,6 +1213,14 @@ def _render_pillar_role(srole: SaltRole) -> str:
"{% if attrs.get('context') %}",
" - context: {{ attrs.get('context')|yaml_encode }}",
"{% endif %}",
"{% if attrs.get('watch_in') %}",
" - watch_in:",
"{% for req in attrs.get('watch_in') %}",
"{% for req_kind, req_name in req.items() %}",
" - {{ req_kind }}: {{ req_name|yaml_dquote }}",
"{% endfor %}",
"{% endfor %}",
"{% endif %}",
"{% endfor %}",
"",
"{% for path, attrs in role.get('links', {}).items() %}",
@ -1182,7 +1232,9 @@ def _render_pillar_role(srole: SaltRole) -> str:
"{% endfor %}",
"",
"{% for service_id, svc in role.get('services', {}).items() %}",
f"enroll_service_{role_key}_{{{{ loop.index }}}}:",
"{{ svc.get('state_id') or ('enroll_service_"
+ role_key
+ "_' ~ loop.index|string) }}:",
" service.{{ 'running' if svc.get('state') == 'running' else 'dead' }}:",
" - name: {{ svc.get('name', service_id)|yaml_dquote }}",
" - enable: {{ svc.get('enable', False)|yaml_encode }}",