From e51e5fb2fb4b42984d3e87bbd67b5165c2532270 Mon Sep 17 00:00:00 2001 From: Roland Date: Wed, 22 Jul 2026 19:41:17 -0600 Subject: [PATCH] controller: group services into Core vs Optional & Maintenance The Controller listed every wrolpi-* service in one flat table, which was confusing for new users who couldn't tell always-on infrastructure from one-off/dev/maintenance units. Services are now split into two groups computed once in the backend and exposed as `service.group`: * Core Services - persistent services enabled at boot (api, app, caddy, kiwix, ...), the normal running system. * Optional & Maintenance - one-off/dev/maintenance tasks (bootstrap, repair, upgrade, *-dev, discovered wrolpi-*), plus persistent services the user has disabled at boot (e.g. Samba with no shares configured). Rule: core iff kind == "persistent" and enabled; otherwise optional. Group membership only changes when the user toggles Boot, never on start/stop or polling, so rows stay put. Within each group, failed/stopped services sort first so problems stay visible. Feature parity across all three render paths: the React ControllerPage, the server-side Jinja template, and the client-side poll refresh. Docker containers have no group field and fall back to a single ungrouped list. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/src/components/admin/ControllerPage.js | 71 ++++++-- .../components/admin/ControllerPage.test.js | 57 +++++++ controller/defaults.py | 3 + controller/lib/systemd.py | 53 +++++- controller/main.py | 9 +- controller/templates/index.html | 161 ++++++++++++------ controller/test/test_systemd.py | 72 ++++++++ 7 files changed, 353 insertions(+), 73 deletions(-) diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index 2de5ea61..6d51f929 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -42,6 +42,30 @@ const statusColors = { unknown: 'yellow', }; +// Sort order within a group: problems first so they stay visible. +const statusSortOrder = { + failed: 0, + unknown: 1, + stopped: 2, + running: 3, +}; + +// Partition services into the two Controller groups. The backend computes each +// service's `group` ("core" | "optional"); a missing group (e.g. Docker +// containers) is treated as core so nothing is hidden. Within each group, +// failed/stopped services sort first, then alphabetically. +export const groupServices = (services) => { + const byStatusThenName = (a, b) => { + const sa = statusSortOrder[a.status] ?? 1; + const sb = statusSortOrder[b.status] ?? 1; + if (sa !== sb) return sa - sb; + return a.name.localeCompare(b.name); + }; + const core = services.filter(s => s.group !== 'optional').sort(byStatusThenName); + const optional = services.filter(s => s.group === 'optional').sort(byStatusThenName); + return {core, optional}; +}; + const linesOptions = [ {key: 100, text: '100 lines', value: 100}, @@ -480,6 +504,36 @@ function ServicesSection() { ); + const {core, optional} = groupServices(services); + + // Full-width labeled separator between the two service groups. + const groupLabelRow = (key, title, colSpan) => ( + + + {title} + + + ); + + const groupedRows = (RowComponent, colSpan, extraProps = {}) => { + const rows = []; + // Only label the Core group when there is also an Optional group to + // distinguish it from; otherwise a lone header adds noise. + if (core.length > 0 && optional.length > 0) { + rows.push(groupLabelRow('label-core', 'Core Services', colSpan)); + } + core.forEach(service => rows.push( + + )); + if (optional.length > 0) { + rows.push(groupLabelRow('label-optional', 'Optional & Maintenance', colSpan)); + optional.forEach(service => rows.push( + + )); + } + return rows; + }; + return (
@@ -496,13 +550,7 @@ function ServicesSection() { - {services.map(service => ( - - ))} + {groupedRows(MobileServiceRow, 2)} {restartButton(2)} @@ -519,14 +567,7 @@ function ServicesSection() { - {services.map(service => ( - - ))} + {groupedRows(DesktopServiceRow, dockerized ? 3 : 4, {dockerized})} {restartButton(dockerized ? 4 : 5)} diff --git a/app/src/components/admin/ControllerPage.test.js b/app/src/components/admin/ControllerPage.test.js index 5de2faa5..0e82c82b 100644 --- a/app/src/components/admin/ControllerPage.test.js +++ b/app/src/components/admin/ControllerPage.test.js @@ -14,6 +14,8 @@ jest.mock('../../api/controller', () => ({ viewable: true, view_path: '/docs', enabled: true, + kind: 'persistent', + group: 'core', }, { name: 'wrolpi-app', @@ -22,6 +24,28 @@ jest.mock('../../api/controller', () => ({ viewable: true, view_path: '', enabled: true, + kind: 'persistent', + group: 'core', + }, + { + name: 'smbd', + port: 445, + status: 'stopped', + viewable: false, + view_path: '', + enabled: false, + kind: 'persistent', + group: 'optional', + }, + { + name: 'wrolpi-repair', + port: null, + status: 'stopped', + viewable: false, + view_path: '', + enabled: false, + kind: 'task', + group: 'optional', }, ]), startService: jest.fn().mockResolvedValue({success: true}), @@ -150,6 +174,39 @@ describe('ControllerPage', () => { expect(screen.getByTestId('shutdown-button')).toBeInTheDocument(); }); }); + +}); + +describe('groupServices', () => { + const {groupServices} = require('./ControllerPage'); + + test('splits core from optional by group field', () => { + const {core, optional} = groupServices([ + {name: 'wrolpi-api', status: 'running', group: 'core'}, + {name: 'smbd', status: 'stopped', group: 'optional'}, + {name: 'wrolpi-repair', status: 'stopped', group: 'optional'}, + ]); + expect(core.map(s => s.name)).toEqual(['wrolpi-api']); + expect(optional.map(s => s.name)).toEqual(['smbd', 'wrolpi-repair']); + }); + + test('treats a missing group (Docker containers) as core', () => { + const {core, optional} = groupServices([ + {name: 'api', status: 'running'}, + {name: 'db', status: 'running'}, + ]); + expect(core).toHaveLength(2); + expect(optional).toHaveLength(0); + }); + + test('sorts problems first, then alphabetically, within a group', () => { + const {core} = groupServices([ + {name: 'caddy', status: 'running', group: 'core'}, + {name: 'wrolpi-api', status: 'failed', group: 'core'}, + {name: 'wrolpi-app', status: 'running', group: 'core'}, + ]); + expect(core.map(s => s.name)).toEqual(['wrolpi-api', 'caddy', 'wrolpi-app']); + }); }); describe('ControllerPage in Docker mode', () => { diff --git a/controller/defaults.py b/controller/defaults.py index 604607ee..33a47cf9 100644 --- a/controller/defaults.py +++ b/controller/defaults.py @@ -41,6 +41,7 @@ DEFAULT_CONFIG = { "viewable": True, "use_https": False, "description": "Python API (Development)", + "kind": "task", "show_only_when_running": True, }, { @@ -58,6 +59,7 @@ DEFAULT_CONFIG = { "viewable": False, "use_https": False, "description": "React frontend (Development)", + "kind": "task", "show_only_when_running": True, }, { @@ -110,6 +112,7 @@ DEFAULT_CONFIG = { "viewable": False, "use_https": False, "description": "WROLPi upgrade process", + "kind": "task", "show_only_when_running": True, }, ], diff --git a/controller/lib/systemd.py b/controller/lib/systemd.py index 9cc0dfbc..5e63d98b 100644 --- a/controller/lib/systemd.py +++ b/controller/lib/systemd.py @@ -40,6 +40,45 @@ def get_service_config(name: str) -> Optional[dict]: return None +def get_service_kind(service_config: dict) -> str: + """ + Classify a service as 'persistent' or 'task'. + + 'persistent' services are the always-on parts of a running WROLPi (api, + app, caddy, kiwix, ...). 'task' services are one-off/dev/maintenance units + (bootstrap, repair, upgrade, *-dev) that run on demand and are not expected + to stay running. + + An explicit "kind" in the service config wins. Otherwise services flagged + show_only_when_running (dev/upgrade units) are treated as tasks, and + everything else defaults to persistent. + """ + kind = service_config.get("kind") + if kind: + return kind + if service_config.get("show_only_when_running"): + return "task" + return "persistent" + + +def classify_service_group(kind: str, enabled: bool) -> str: + """ + Group a service for display in the Controller UI. + + Returns: + "core" - a persistent service enabled at boot: the normal running + system the user cares about day-to-day. + "optional" - one-off/dev/maintenance tasks, or persistent services the + user has disabled at boot (e.g. Samba with no shares). + + Membership only changes when the user toggles Boot for a persistent + service, never on start/stop or polling, so rows stay put. + """ + if kind == "persistent" and enabled: + return "core" + return "optional" + + def _run_systemctl(command: str, service: str, timeout: int = 30) -> dict: """ Run a systemctl command. @@ -118,6 +157,7 @@ def get_service_status(name: str) -> dict: ) load_state = load_result.stdout.strip() except (subprocess.TimeoutExpired, FileNotFoundError): + kind = get_service_kind(service_config) return { "name": name, "systemd_name": systemd_name, @@ -130,6 +170,8 @@ def get_service_status(name: str) -> dict: "view_path": service_config.get("view_path", ""), "use_https": service_config.get("use_https", False), "description": service_config.get("description", ""), + "kind": kind, + "group": classify_service_group(kind, False), "error": "systemctl not available", } @@ -145,18 +187,22 @@ def get_service_status(name: str) -> dict: else: status = "unknown" + enabled = is_enabled == "enabled" + kind = get_service_kind(service_config) return { "name": name, "systemd_name": systemd_name, "status": status, "installed": load_state not in ("not-found", ""), "active": is_active, - "enabled": is_enabled == "enabled", + "enabled": enabled, "port": service_config.get("port"), "viewable": service_config.get("viewable", False), "view_path": service_config.get("view_path", ""), "use_https": service_config.get("use_https", False), "description": service_config.get("description", ""), + "kind": kind, + "group": classify_service_group(kind, enabled), } @@ -241,6 +287,8 @@ def get_discovered_service_status(name: str) -> dict: "view_path": "", "use_https": False, "description": "", + "kind": "task", + "group": "optional", "error": "systemctl not available", } @@ -266,6 +314,9 @@ def get_discovered_service_status(name: str) -> dict: "view_path": "", "use_https": False, "description": "", + # Discovered wrolpi-* units are always one-off/maintenance tasks. + "kind": "task", + "group": "optional", } diff --git a/controller/main.py b/controller/main.py index 31817e3c..f5cde7e9 100644 --- a/controller/main.py +++ b/controller/main.py @@ -228,8 +228,13 @@ async def dashboard(request: Request): else: services = get_all_services_status() - # Sort services by name for consistent display - services = sorted(services, key=lambda s: s["name"]) + # Sort services so problems surface first, then alphabetically. The + # template partitions on each service's "group" (core vs optional). + _status_order = {"failed": 0, "unknown": 1, "stopped": 2, "running": 3} + services = sorted( + services, + key=lambda s: (_status_order.get(s.get("status"), 1), s["name"]), + ) # Root CA download is only useful after the media drive is mounted (so an # existing CA on the drive is visible) and repair has finished generating diff --git a/controller/templates/index.html b/controller/templates/index.html index de695262..00027622 100644 --- a/controller/templates/index.html +++ b/controller/templates/index.html @@ -241,6 +241,20 @@ background: #1a3a5c; } + /* Service group separators (Core vs Optional & Maintenance) */ + tr.service-group-label td { + background: #0f3460; + text-transform: uppercase; + font-size: 0.75rem; + letter-spacing: 0.05em; + color: #a9b7d0; + padding-top: 0.9rem; + } + + tr.service-group-label:hover { + background: transparent; + } + /* Status indicators */ .status-running { color: #4ecca3; @@ -988,8 +1002,55 @@ + {% macro service_row(service, docker_mode, host) %} + + + {{ service.name }} + {% if service.port %}:{{ service.port }}{% endif %} + + {{ service.status }} + + {% if service.status == 'running' %} + + {% else %} + + {% endif %} + + + {% if service.viewable and service.status == 'running' and service.port %} + {% set is_default_port = (service.use_https and service.port == 443) or (not service.use_https and service.port == 80) %} + View + {% endif %} + + {% if not docker_mode %} + + {% if service.name == 'wrolpi-upgrade' %} + -- + {% else %} + + + {{ 'Enabled' if service.enabled else 'Disabled' }} + + {% endif %} + + {% endif %} + + {% endmacro %} +

Services

+ {% set core = services | rejectattr('group', 'equalto', 'optional') | list %} + {% set optional = services | selectattr('group', 'equalto', 'optional') | list %} + {% set span = 3 if docker_mode else 4 %} @@ -1002,55 +1063,22 @@ - {% for service in services %} - - - - - {% if not docker_mode %} - + {% if services %} + {% if core and optional %} + + {% endif %} + {% for service in core %}{{ service_row(service, docker_mode, host) }}{% endfor %} + {% if optional %} + + {% for service in optional %}{{ service_row(service, docker_mode, host) }}{% endfor %} {% endif %} - {% else %} - - {% endfor %} + {% endif %}
- {{ service.name }} - {% if service.port %}:{{ service.port }}{% endif %} - {{ service.status }} - {% if service.status == 'running' %} - - {% else %} - - {% endif %} - - - {% if service.viewable and service.status == 'running' and service.port %} - {% set is_default_port = (service.use_https and service.port == 443) or (not service.use_https and service.port == 80) %} - View - {% endif %} - - {% if service.name == 'wrolpi-upgrade' %} - -- - {% else %} - - - {{ 'Enabled' if service.enabled else 'Disabled' }} - - {% endif %} -
Core Services
Optional & Maintenance
+ Service status not yet implemented
@@ -2728,10 +2756,10 @@ // Check if we're in docker mode by looking for Boot column const isDockerMode = !document.querySelector('th:last-child')?.textContent?.includes('Boot'); + const colspan = isDockerMode ? 3 : 4; - let html = ''; - for (const service of services) { - html += ` + function serviceRowHtml(service) { + let row = ` ${service.name} ${service.port ? `:${service.port}` : ''} @@ -2740,12 +2768,12 @@ `; if (service.status === 'running') { - html += ``; + row += ``; } else { - html += ``; + row += ``; } - html += ` + row += ` `; if (service.viewable && service.status === 'running' && service.port) { @@ -2754,16 +2782,16 @@ const viewPath = service.view_path || ''; const isDefaultPort = (protocol === 'https' && service.port === 443) || (protocol === 'http' && service.port === 80); const portSuffix = isDefaultPort ? '' : ':' + service.port; - html += `View`; + row += `View`; } - html += ''; + row += ''; if (!isDockerMode) { if (service.name === 'wrolpi-upgrade') { - html += `--`; + row += `--`; } else { - html += ` + row += `