Merge pull request 'fix(deploy): correct service selection and recovery validation' (#117) from fix/cicd-review-recovery into main

Reviewed-on: #117

.gitea/runner/README.md

@@ -157,3 +157,25 @@ run first. Restore the runner config/unit from `.before-<timestamp>` backups,
 reload systemd and restart the runner. Restore the prior workflows from Git.
 Production data and persistent volumes stay where they were. Do not remove run
 state or Compose recovery files until recovery is confirmed.
+
+### Compose configuration recovery
+
+Successful deploys save the complete resolved Compose configuration in
+`~/.local/state/homelab-deploy/compose-configs/`. These files can contain secrets.
+Keep them private and do not commit or upload them.
+The next deploy uses this configuration for its recovery file, including old
+commands, environment, mounts, ports, and removed services. The recovery command
+uses `--remove-orphans` to remove services added by the failed deploy. It does
+not restore volume data or reverse database migrations.
+
+On the first run after this update, the controller can use the Compose file
+from the previous successful run. If that file is absent, it reads the persistent
+checkout and checks its service configuration hashes against existing containers.
+A mismatch stops preflight. Restore the previous configuration before retrying.
+Update the installed controller with `bash .gitea/runner/setup-workstation.sh`
+from the reviewed checkout before using this change.
+
+New namespaces are checked during preflight. Server validation of their resources
+runs after namespace creation and before application resources are applied.
+Plan mode does not create namespaces. A failed deferred check can leave an empty
+namespace; inspect it before removing it.

.gitea/tests/deploy-validation.sh

@@ -92,3 +92,44 @@ if check_referenced_secrets >"$scratch/secrets.log"; then
   exit 1
 fi
 printf '%s\n' 'Deploy validation regressions passed.'
+
+# New declared namespaces defer only their own resources during preflight.
+render_selected_resources() {
+  cat <<'JSON'
+{"apiVersion":"v1","kind":"List","items":[
+ {"apiVersion":"v1","kind":"Namespace","metadata":{"name":"new"}},
+ {"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"new-config","namespace":"new"}},
+ {"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"existing-config","namespace":"default"}}
+]}
+JSON
+}
+kubectl() {
+  case "$1" in
+    get) printf '%s\n' '{"items":[{"metadata":{"name":"default"}}]}' ;;
+    apply) cat >"$scratch/server-input.json" ;;
+    *) return 1 ;;
+  esac
+}
+validate_server_resources true
+jq -e '.items | length == 2 and all(.metadata.name != "new-config")' "$scratch/server-input.json" >/dev/null
+if validate_server_resources false 2>"$scratch/deferred.log"; then
+  echo 'Post-namespace validation accepted a missing namespace' >&2
+  exit 1
+fi
+kubectl() {
+  case "$1" in
+    get) printf '%s\n' '{"items":[{"metadata":{"name":"default"}},{"metadata":{"name":"new"}}]}' ;;
+    apply) cat >"$scratch/server-input.json" ;;
+    *) return 1 ;;
+  esac
+}
+validate_server_resources false
+jq -e '.items | length == 3' "$scratch/server-input.json" >/dev/null
+render_selected_resources() {
+  printf '%s\n' '{"items":[{"kind":"ConfigMap","metadata":{"name":"bad","namespace":"undeclared"}}]}'
+}
+if validate_server_resources true 2>"$scratch/undeclared.log"; then
+  echo 'Preflight accepted an undeclared missing namespace' >&2
+  exit 1
+fi
+printf '%s\n' 'Namespace validation regressions passed.'

.gitea/workflows/compose-release.py

@@ -42,7 +42,37 @@ def prepare(source_file):
     images_file = directory / 'compose-images.json'
     locks = json.loads(images_file.read_text()) if images_file.exists() else previous.get('compose-images', {})
     release = json.loads((directory / 'release.json').read_text())
-    before = json.loads(json.dumps(config))
+    state = Path(os.environ.get('HOMELAB_STATE', Path.home() / '.local/state/homelab-deploy'))
+    baseline = state / 'compose-configs' / f'{relative.parent.name}.json'
+    if not baseline.exists() and re.fullmatch(r'[0-9]+-[0-9]+', previous.get('run_id', '')):
+        baseline = state / 'runs' / previous['run_id'] / 'compose' / baseline.name
+    bootstrap = not baseline.exists()
+    if not bootstrap:
+        before = json.loads(baseline.read_text())
+    else:
+        # Bootstrap from the persistent configuration, never from the new source.
+        persistent_file = config_repo / relative
+        if persistent_file.exists():
+            before = json.loads(
+                output(
+                    'docker',
+                    'compose',
+                    '--project-directory',
+                    str(project_dir),
+                    '-f',
+                    str(persistent_file),
+                    'config',
+                    '--format',
+                    'json',
+                    cwd=config_repo,
+                )
+            )
+        elif output('docker', 'ps', '-aq', '--filter', f'label=com.docker.compose.project={project}'):
+            raise ValueError(f'{project}: no previous Compose configuration; restore it before deploy')
+        else:
+            before = {'name': project, 'services': {}}
+    if before['name'] != project:
+        raise ValueError('Compose project name changed; manual migration is required')
     for service, settings in config['services'].items():
         reference = settings.get('image')
         nextcloud_aio_master = project == 'nextcloud' and service == 'nextcloud-aio-mastercontainer'
@@ -63,6 +93,12 @@ def prepare(source_file):
             pinned = resolve(reference)
         settings['image'] = pinned
         locks[reference] = pinned
+    for service, settings in before['services'].items():
+        reference = settings['image']
+        image_repo = reference.split('@')[0].rsplit('/', 1)
+        image_repo[-1] = image_repo[-1].split(':')[0]
+        image_repo = '/'.join(image_repo)
+        nextcloud_aio_master = project == 'nextcloud' and service == 'nextcloud-aio-mastercontainer'
         # Capture what is running, not the current value of its mutable tag.
         ids = output(
             'docker',
@@ -74,6 +110,31 @@ def prepare(source_file):
             f'label=com.docker.compose.service={service}',
         ).splitlines()
         actual = set()
+        if bootstrap and ids:
+            expected_hash = output(
+                'docker',
+                'compose',
+                '--project-directory',
+                str(project_dir),
+                '-f',
+                str(persistent_file),
+                'config',
+                '--hash',
+                service,
+                cwd=config_repo,
+            ).split()[-1]
+            for container in ids:
+                running_hash = output(
+                    'docker',
+                    'inspect',
+                    container,
+                    '--format',
+                    '{{ index .Config.Labels "com.docker.compose.config-hash" }}',
+                )
+                if running_hash != expected_hash:
+                    raise ValueError(
+                        f'{project}/{service}: persistent config differs from running config; restore the previous config'
+                    )
         for container in ids:
             image_id = output('docker', 'inspect', container, '--format', '{{.Image}}')
             digests = json.loads(output('docker', 'image', 'inspect', image_id, '--format', '{{json .RepoDigests}}'))
@@ -95,7 +156,7 @@ def prepare(source_file):
     images_file.write_text(json.dumps(locks, indent=2) + '\n')
     print(f'Compose {project}: images pinned; local paths preserved')
     print(
-        f'Recovery: docker compose --project-directory {project_dir} -p {project} -f {directory}/compose-before/{relative.parent.name}.json up -d --pull never'
+        f'Recovery: docker compose --project-directory {project_dir} -p {project} -f {directory}/compose-before/{relative.parent.name}.json up -d --pull never --remove-orphans'
     )
 
 

.gitea/workflows/deploy-controller.py

@@ -165,6 +165,10 @@ def finish_success(directory, plan):
         if previous.exists()
         else {}
     )
+    configs = STATE / 'compose-configs'
+    configs.mkdir(mode=0o700, exist_ok=True)
+    for config in (directory / 'compose').glob('*.json'):
+        atomic_json(configs / config.name, json.loads(config.read_text()))
     atomic_json(STATE / 'last-success.json', plan)
     status = json.loads((directory / 'status.json').read_text())
     status['state'] = 'success'

.gitea/workflows/deploy-lib.sh

@@ -571,6 +571,41 @@ skip_uninstalled_vmagent_crd() {
   return 1
 }
 
+# Render one complete resource list so new namespaces can be identified across
+# files and Kustomize apps. A missing undeclared namespace remains an error.
+render_selected_resources() {
+  local m k
+  {
+    for m in "${K8S_MANIFESTS[@]}"; do
+      if skip_uninstalled_vmagent_crd "$m" >/dev/null; then continue; fi
+      kubectl create --dry-run=client --validate=false -f "$m" -o json || return 1
+    done
+    for k in "${KUSTOMIZE_APPS[@]}"; do
+      kubectl kustomize "$k" | kubectl create --dry-run=client --validate=false -f - -o json || return 1
+    done
+  } | jq -s '{apiVersion: "v1", kind: "List", items: [ .[] | if .kind == "List" then .items[] else . end ]}'
+}
+
+validate_server_resources() {
+  local defer_new="$1" resources existing filtered
+  resources="$(render_selected_resources)" || return 1
+  existing="$(kubectl get namespaces -o json)" || return 1
+  filtered="$(jq --argjson existing "$existing" --argjson defer "$defer_new" '
+    [.items[] | select(.kind == "Namespace") | .metadata.name] as $declared
+    | [$existing.items[].metadata.name] as $present
+    | .items |= map(
+        (.metadata.namespace // "default") as $ns
+        | if .kind == "Namespace" or ($present | index($ns)) != null then .
+          elif ($declared | index($ns)) == null then error("Undeclared missing namespace: " + $ns)
+          elif $defer then empty
+          else error("Namespace still missing after namespace apply: " + $ns)
+          end)
+  ' <<<"$resources")" || return 1
+  if [ "$(jq '.items | length' <<<"$filtered")" -gt 0 ]; then
+    kubectl apply --dry-run=server -f - <<<"$filtered" >/dev/null
+  fi
+}
+
 stage_validate() {
   check_prune_mode || return 1
   cd "$REPO"
@@ -597,15 +632,7 @@ stage_validate() {
     kubectl apply -k "$k" --dry-run=client >/dev/null
   done
   log "Validate k8s manifests (kubectl dry-run=server)"
-  for m in ${K8S_MANIFESTS[@]+"${K8S_MANIFESTS[@]}"}; do
-    if skip_uninstalled_vmagent_crd "$m"; then
-      continue
-    fi
-    kubectl apply --dry-run=server -f "$m" >/dev/null
-  done
-  for k in ${KUSTOMIZE_APPS[@]+"${KUSTOMIZE_APPS[@]}"}; do
-    kubectl apply -k "$k" --dry-run=server >/dev/null
-  done
+  validate_server_resources true
   log "Checking referenced Secrets exist"
   echo "  (deploy never applies *secret*.yaml; create missing ones manually)"
   check_referenced_secrets
@@ -657,6 +684,14 @@ stage_apply_k8s() {
       record_apply kubectl "${m#"$REPO"/}" success
     done
   fi
+  # Kustomize may declare namespaces inside its rendered resources too.
+  local namespace_resources
+  namespace_resources="$(render_selected_resources | jq '.items |= map(select(.kind == "Namespace"))')" || return 1
+  if [ "$(jq '.items | length' <<<"$namespace_resources")" -gt 0 ]; then
+    kubectl apply -f - <<<"$namespace_resources" || return 1
+  fi
+  # Complete the deferred server checks before Helm or application resources change.
+  validate_server_resources false || return 1
   if selected_service k8s prometheus-stack && [ -f "$REPO/prometheus-stack/k8s/active" ]; then
     if [ ! -f "$CONFIG_REPO/prometheus-stack/k8s/grafana-values.yaml" ]; then
       echo "ERROR: prometheus-stack/k8s/grafana-values.yaml (gitignored) missing on workstation, restore it first."

.gitea/workflows/deploy-plan.py

@@ -83,20 +83,20 @@ def make_plan(repo, config_repo, release, previous, mode, live_helm):
         removed = []
     else:
         paths = output('git', '-C', str(repo), 'diff', '--name-only', previous['sha'], release['sha']).splitlines()
-        changed = {path.split('/')[0] for path in paths}
+        changed = {service for service in all_services for path in paths if path.startswith(service + '/')}
         if any(path.startswith('.gitea/') for path in paths):
             changed |= all_services
         changed |= {s for s in all_services if previous.get('local_inputs', {}).get(s) != local_inputs[s]}
         for file in tracked(repo):
-            service = file.split('/')[0]
-            if service not in all_services or not file.endswith(('.yaml', '.yml')):
+            owners = {service for service in all_services if file.startswith(service + '/')}
+            if not owners or not file.endswith(('.yaml', '.yml')):
                 continue
             text = (repo / file).read_text()
             if any(
                 image in text and previous.get('images', {}).get(image) != digest
                 for image, digest in release['images'].items()
             ):
-                changed.add(service)
+                changed |= owners
         removed = sorted(
             set(previous.get('active', {}).get('k8s', []) + previous.get('active', {}).get('compose', []))
             - all_services

tests/test_cicd.py

@@ -128,6 +128,23 @@ class SelectionTests(unittest.TestCase):
         self.assertEqual(result['selected']['k8s'], ['one'])
         self.assertEqual(result['helm'], [])
 
+    def test_nested_service_change_and_owned_image_are_selected(self):
+        directory = self.repo / 'vpn/xui/k8s'
+        directory.mkdir(parents=True)
+        (directory / 'active').touch()
+        image = next(iter(release()['images']))
+        (directory / 'app.yaml').write_text('image: ' + image + ':main\n')
+        baseline_sha = self.commit()
+        baseline = planner.make_plan(self.repo, self.repo, release(baseline_sha), None, 'full', [])
+        (directory / 'app.yaml').write_text('image: ' + image + ':prod\n')
+        result = planner.make_plan(self.repo, self.repo, release(self.commit()), baseline, 'changed', [])
+        self.assertEqual(result['selected']['k8s'], ['vpn/xui'])
+        baseline = result
+        updated = release(result['sha'])
+        updated['images'][image] = 'sha256:' + 'e' * 64
+        result = planner.make_plan(self.repo, self.repo, updated, baseline, 'changed', [])
+        self.assertEqual(result['selected']['k8s'], ['vpn/xui'])
+
     def test_failed_intermediate_deploy_does_not_lose_changes(self):
         (self.repo / 'one/k8s/app.yaml').write_text('kind: StatefulSet\n')
         self.commit()  # This commit failed deploy: baseline must remain initial.
@@ -162,7 +179,8 @@ class ComposeConfigurationTests(unittest.TestCase):
             source = run / 'source'
             config_repo = root / 'persistent'
             (source / 'headscale').mkdir(parents=True)
-            config_repo.mkdir()
+            (config_repo / 'headscale').mkdir(parents=True)
+            (config_repo / 'headscale/compose.yaml').touch()
             (run / 'release.json').write_text(json.dumps(release()))
             old = 'busybox@sha256:' + 'd' * 64
             new = 'busybox@sha256:' + 'e' * 64
@@ -180,19 +198,41 @@ class ComposeConfigurationTests(unittest.TestCase):
                 'volumes': {'data': {'name': 'headscale_data'}},
             }
 
+            previous_config = json.loads(json.dumps(config))
+            previous_config['services']['app']['command'] = ['old-command']
+            previous_config['services']['app']['environment'] = {'VALUE': 'old'}
+            previous_config['services']['removed'] = {'image': 'busybox:latest'}
+            config['services']['app']['command'] = ['new-command']
+            config['services']['app']['environment'] = {'VALUE': 'new'}
+            config['services']['added'] = {'image': 'busybox:latest'}
+
             def fake_output(*args, **kwargs):
                 if args[:2] == ('docker', 'compose'):
                     self.assertEqual(kwargs['cwd'], config_repo)
                     self.assertIn(str(config_repo / 'headscale'), args)
-                    return json.dumps(config)
+                    if '--hash' in args:
+                        return 'app matching-hash'
+                    return json.dumps(
+                        previous_config if str(config_repo / 'headscale/compose.yaml') in args else config
+                    )
                 if args[:2] == ('docker', 'ps'):
                     return 'container'
                 if args[:2] == ('docker', 'inspect'):
+                    if 'com.docker.compose.config-hash' in args[-1]:
+                        return 'matching-hash'
                     return 'sha256:' + 'f' * 64
                 return json.dumps([old])
 
             with (
-                patch.dict(os.environ, {'CONFIG_REPO': str(config_repo), 'REPO': str(source), 'RUN_DIR': str(run)}),
+                patch.dict(
+                    os.environ,
+                    {
+                        'CONFIG_REPO': str(config_repo),
+                        'REPO': str(source),
+                        'RUN_DIR': str(run),
+                        'HOMELAB_STATE': str(root / 'state'),
+                    },
+                ),
                 patch.object(compose_module, 'output', side_effect=fake_output),
                 patch.object(compose_module, 'resolve', return_value=new),
             ):
@@ -204,6 +244,57 @@ class ComposeConfigurationTests(unittest.TestCase):
             self.assertEqual(pinned['services']['app']['volumes'], config['services']['app']['volumes'])
             self.assertEqual(pinned['services']['app']['image'], new)
             self.assertEqual(before['services']['app']['image'], old)
+            self.assertEqual(before['services']['app']['command'], ['old-command'])
+            self.assertEqual(before['services']['app']['environment'], {'VALUE': 'old'})
+            self.assertIn('removed', before['services'])
+            self.assertNotIn('added', before['services'])
+
+            def mismatched_output(*args, **kwargs):
+                if args[:2] == ('docker', 'inspect') and 'com.docker.compose.config-hash' in args[-1]:
+                    return 'different-hash'
+                return fake_output(*args, **kwargs)
+
+            with (
+                patch.dict(
+                    os.environ,
+                    {
+                        'CONFIG_REPO': str(config_repo),
+                        'REPO': str(source),
+                        'RUN_DIR': str(run),
+                        'HOMELAB_STATE': str(root / 'state'),
+                    },
+                ),
+                patch.object(compose_module, 'output', side_effect=mismatched_output),
+                patch.object(compose_module, 'resolve', return_value=new),
+                self.assertRaisesRegex(ValueError, 'differs from running config'),
+            ):
+                compose_module.prepare(source / 'headscale/compose.yaml')
+            state = root / 'state'
+            with patch.object(controller, 'STATE', state):
+                state.mkdir()
+                (run / 'status.json').write_text('{"state": "running", "stages": {}}')
+                with patch.object(controller, 'retain_completed'):
+                    controller.finish_success(run, {})
+            self.assertEqual(json.loads((state / 'compose-configs/headscale.json').read_text()), pinned)
+            # A stale persistent checkout must not replace the successful baseline.
+            with (
+                patch.dict(
+                    os.environ,
+                    {
+                        'CONFIG_REPO': str(config_repo),
+                        'REPO': str(source),
+                        'RUN_DIR': str(run),
+                        'HOMELAB_STATE': str(state),
+                    },
+                ),
+                patch.object(compose_module, 'output', side_effect=fake_output),
+                patch.object(compose_module, 'resolve', return_value=new),
+            ):
+                compose_module.prepare(source / 'headscale/compose.yaml')
+            before = json.loads((run / 'compose-before/headscale.json').read_text())
+            self.assertEqual(before['services']['app']['command'], ['new-command'])
+            self.assertIn('added', before['services'])
+            self.assertNotIn('removed', before['services'])
             self.assertEqual((run / 'compose/headscale.json').stat().st_mode & 0o777, 0o600)
 
     def test_registry_index_and_single_image_descriptors(self):