From 097022f782d0f03e8031300c8c6dc7f1290a6a81 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 20 Jun 2026 15:31:42 +1000 Subject: [PATCH] Fix notification of individual services when related config changes, even when roles are grouped --- enroll/ansible.py | 86 +++++++++++++------ enroll/cm.py | 72 ++++++++++++++++ enroll/puppet.py | 44 +++++++--- enroll/salt.py | 102 ++++++++++++++++------ tests/test_cm.py | 67 +++++++++++++++ tests/test_manifest.py | 154 ++++++++++++++++++++++++++++++++++ tests/test_manifest_puppet.py | 4 + tests/test_manifest_salt.py | 5 ++ 8 files changed, 472 insertions(+), 62 deletions(-) diff --git a/enroll/ansible.py b/enroll/ansible.py index 1778fd9..7d7ba9b 100644 --- a/enroll/ansible.py +++ b/enroll/ansible.py @@ -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) diff --git a/enroll/cm.py b/enroll/cm.py index 4718b90..f452313 100644 --- a/enroll/cm.py +++ b/enroll/cm.py @@ -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 "") diff --git a/enroll/puppet.py b/enroll/puppet.py index 286ce47..8a7e950 100644 --- a/enroll/puppet.py +++ b/enroll/puppet.py @@ -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) } diff --git a/enroll/salt.py b/enroll/salt.py index 611e69b..8d29bbb 100644 --- a/enroll/salt.py +++ b/enroll/salt.py @@ -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 }}", diff --git a/tests/test_cm.py b/tests/test_cm.py index 89addbf..94b0431 100644 --- a/tests/test_cm.py +++ b/tests/test_cm.py @@ -38,3 +38,70 @@ def test_cm_module_uses_shared_state_io(tmp_path): assert CMModule.state_path(tmp_path) == written assert CMModule.load_state(tmp_path) == state assert CMModule._load_state(tmp_path) == state + + +def test_active_service_units_for_package_snapshot_is_conservative(): + entries = [ + { + "kind": "service", + "snapshot": { + "unit": "docker.service", + "role_name": "docker", + "packages": ["docker.io"], + "active_state": "active", + }, + }, + { + "kind": "service", + "snapshot": { + "unit": "docker-cleanup.service", + "role_name": "docker_cleanup", + "packages": ["docker.io"], + "active_state": "inactive", + }, + }, + ] + + by_package = CMModule.active_service_units_by_package(entries) + + assert by_package == { + "docker.io": [{"unit": "docker.service", "role_name": "docker"}] + } + assert CMModule.active_service_units_for_package_snapshot( + {"package": "docker.io", "role_name": "docker"}, by_package + ) == ["docker.service"] + + +def test_active_service_units_for_package_snapshot_avoids_ambiguous_restarts(): + entries = [ + { + "kind": "service", + "snapshot": { + "unit": "alpha.service", + "role_name": "alpha", + "packages": ["shared"], + "active_state": "active", + }, + }, + { + "kind": "service", + "snapshot": { + "unit": "beta.service", + "role_name": "beta", + "packages": ["shared"], + "active_state": "active", + }, + }, + ] + + by_package = CMModule.active_service_units_by_package(entries) + + assert ( + CMModule.active_service_units_for_package_snapshot( + {"package": "shared", "role_name": "shared"}, by_package + ) + == [] + ) + assert CMModule.active_service_units_for_package_snapshot( + {"package": "shared", "role_name": "beta"}, by_package + ) == ["beta.service"] diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 4c00c1d..cc6d045 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -284,6 +284,12 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): assert "foo_systemd_enabled: true" in defaults assert "foo_systemd_state: stopped" in defaults + handlers = (out / "roles" / "foo" / "handlers" / "main.yml").read_text( + encoding="utf-8" + ) + assert "- name: Restart service" in handlers + assert "state: restarted" in handlers + # Playbook should include users, etc_custom, packages, and services pb = (out / "playbook.yml").read_text(encoding="utf-8") assert "role: users" in pb @@ -626,6 +632,154 @@ def test_manifest_groups_systemd_units_into_common_role(tmp_path: Path): tasks = (out / "roles" / "net" / "tasks" / "main.yml").read_text(encoding="utf-8") assert "Ensure grouped unit enablement matches harvest" in tasks assert 'no_log: "{{ enroll_hide_systemd_status | default(true) | bool }}"' in tasks + assert "Restart managed services" not in tasks + + defaults_text = (out / "roles" / "net" / "defaults" / "main.yml").read_text( + encoding="utf-8" + ) + assert "notify:" in defaults_text + assert "- Restart managed service NetworkManager.service" in defaults_text + assert ( + "Restart managed service NetworkManager-dispatcher.service" not in defaults_text + ) + + handlers = (out / "roles" / "net" / "handlers" / "main.yml").read_text( + encoding="utf-8" + ) + assert "Run systemd daemon-reload" in handlers + assert "- name: Restart managed service NetworkManager.service" in handlers + assert "name: NetworkManager.service" in handlers + assert "state: restarted" in handlers + assert "Restart managed services" not in handlers + assert "Restart managed service NetworkManager-dispatcher.service" not in handlers + + +def test_manifest_common_package_file_notifies_matching_active_service(tmp_path: Path): + bundle = tmp_path / "bundle" + out = tmp_path / "ansible" + (bundle / "artifacts" / "docker" / "etc" / "docker").mkdir( + parents=True, exist_ok=True + ) + (bundle / "artifacts" / "docker" / "etc" / "docker" / "daemon.json").write_text( + '{"log-driver":"json-file"}\n', encoding="utf-8" + ) + + state = { + "schema_version": 3, + "host": {"hostname": "test", "os": "debian", "pkg_backend": "dpkg"}, + "inventory": { + "packages": { + "docker.io": { + "version": "1.0", + "arches": ["amd64"], + "installations": [ + {"version": "1.0", "arch": "amd64", "section": "admin"} + ], + "section": "admin", + "observed_via": [ + {"kind": "systemd_unit", "ref": "docker.service"}, + {"kind": "package_role", "ref": "docker"}, + ], + "roles": ["docker"], + } + } + }, + "roles": { + "users": { + "role_name": "users", + "users": [], + "managed_files": [], + "excluded": [], + "notes": [], + }, + "services": [ + { + "unit": "docker.service", + "role_name": "docker", + "packages": ["docker.io"], + "active_state": "active", + "sub_state": "running", + "unit_file_state": "enabled", + "condition_result": "yes", + "managed_files": [], + "managed_dirs": [], + "managed_links": [], + "excluded": [], + "notes": [], + } + ], + "packages": [ + { + "package": "docker.io", + "role_name": "docker", + "section": "admin", + "has_config": True, + "managed_files": [ + { + "path": "/etc/docker/daemon.json", + "src_rel": "etc/docker/daemon.json", + "owner": "root", + "group": "root", + "mode": "0644", + "reason": "modified_conffile", + } + ], + "managed_dirs": [], + "managed_links": [], + "excluded": [], + "notes": [], + } + ], + "apt_config": { + "role_name": "apt_config", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "dnf_config": { + "role_name": "dnf_config", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "etc_custom": { + "role_name": "etc_custom", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "usr_local_custom": { + "role_name": "usr_local_custom", + "managed_files": [], + "excluded": [], + "notes": [], + }, + "extra_paths": { + "role_name": "extra_paths", + "include_patterns": [], + "exclude_patterns": [], + "managed_files": [], + "excluded": [], + "notes": [], + }, + }, + } + _write_state(bundle, state) + + manifest.manifest(str(bundle), str(out)) + + defaults = (out / "roles" / "admin" / "defaults" / "main.yml").read_text( + encoding="utf-8" + ) + assert "dest: /etc/docker/daemon.json" in defaults + assert "- Restart managed service docker.service" in defaults + + handlers = (out / "roles" / "admin" / "handlers" / "main.yml").read_text( + encoding="utf-8" + ) + assert "- name: Restart managed service docker.service" in handlers + assert "name: docker.service" in handlers + assert "Restart managed services" not in handlers def test_manifest_fqdn_implies_no_common_roles(tmp_path: Path): diff --git a/tests/test_manifest_puppet.py b/tests/test_manifest_puppet.py index e336662..a4e9bff 100644 --- a/tests/test_manifest_puppet.py +++ b/tests/test_manifest_puppet.py @@ -184,6 +184,9 @@ def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path): assert node_data["foo::files"]["/etc/foo/foo.conf"]["source"] == ( "puppet:///modules/foo/nodes/test.example/etc/foo.conf" ) + assert node_data["foo::files"]["/etc/foo/foo.conf"]["notify"] == ( + "Service['foo.service']" + ) assert node_data["foo::services"]["foo.service"] == { "ensure": "running", "enable": True, @@ -483,6 +486,7 @@ def test_manifest_puppet_uses_default_node_and_common_package_modules(tmp_path: assert "package { 'foo':" in net_pp assert "file { '/etc/foo/foo.conf':" in net_pp assert "source => 'puppet:///modules/net/etc/foo.conf'" in net_pp + assert "notify => Service['foo.service']" in net_pp assert "service { 'foo.service':" in net_pp assert (out / "modules" / "net" / "files" / "etc" / "foo.conf").exists() assert not (out / "modules" / "curl").exists() diff --git a/tests/test_manifest_salt.py b/tests/test_manifest_salt.py index 02d6923..7de2fae 100644 --- a/tests/test_manifest_salt.py +++ b/tests/test_manifest_salt.py @@ -164,6 +164,8 @@ def test_manifest_salt_writes_single_site_state_tree(tmp_path: Path): assert '- name: "foo"' in net_sls assert '"/etc/foo/foo.conf":' in net_sls assert 'source: "salt://roles/net/files/etc/foo.conf"' in net_sls + assert "watch_in:" in net_sls + assert 'service: "enroll_service_net_foo_service_20435514"' in net_sls assert "file.symlink:" in net_sls assert "service.running:" in net_sls assert (out / "states" / "roles" / "net" / "files" / "etc" / "foo.conf").exists() @@ -531,6 +533,9 @@ def test_manifest_salt_uses_jinjaturtle_templates(monkeypatch, tmp_path: Path): pillar = yaml.safe_load(pillar_path.with_suffix(".sls").read_text(encoding="utf-8")) file_data = pillar["enroll"]["roles"]["foo"]["files"]["/etc/foo/foo.conf"] assert file_data["source"] == "salt://roles/foo/templates/etc/foo.conf.j2" + assert file_data["watch_in"] == [ + {"service": "enroll_service_foo_foo_service_20435514"} + ] assert file_data["template"] == "jinja" assert file_data["context"] == {"foo_setting": True}