Let node reruns recover from earlier attempts

apps/web/src/features/sandbox/NodeCleanupDialog.tsx

@@ -22,8 +22,9 @@ export interface NodeCleanup {
  * host (deploy/node/node_install.py `uninstall_system`).
  * The installer first confirms with Core, at the node's own address, that the
  * node is removed, which holds from the removal on. The command uses sudo unless the shell is already root. A node enrolled with an earlier address may find it gone; then
- * `--force` skips only that confirmation. Nothing deletes sandboxes, volumes or
- * images; the uninstaller prints what it kept. Like Add node's, the command downloads from the installation's public
+ * `--force` skips only that confirmation. It keeps Docker images and volumes and
+ * deletes a microsandbox store once Core confirmed the removal; the uninstaller
+ * prints what it kept. Like Add node's, the command downloads from the installation's public
  * URL, which the dialog reads (again, if it is not at hand): until it is read, if
  * the read fails (with Try again), or while other machines can't use it, the
  * dialog says so in place of the command. It never opens empty.

apps/web/src/features/sandbox/SandboxManagerView.tsx

@@ -185,8 +185,9 @@ function SandboxManager({ consoleConfig }: { consoleConfig: SandboxConsoleConfig
   </ConfirmDialog>;
   const cleanupDialog = <NodeCleanupDialog cleanup={cleanup?.node ?? null} open={cleanup?.open ?? false} onClose={() => {
     setCleanup((current) => current && { ...current, open: false });
-    // The removed node's button is gone, so focus returns to the page, after the dialog restores its own.
-    window.requestAnimationFrame(() => heading.current?.focus());
+    // After a removal the node's button is gone, so focus returns to the page, after the dialog restores its own.
+    // Clean up a host keeps its button, which the dialog restores focus to.
+    if (cleanup?.node.name !== null) window.requestAnimationFrame(() => heading.current?.focus());
   }} />;
   if (params.id && hostedNodes) {
     const back = () => goBack("nodes");

deploy/node/distribution.py

@@ -119,13 +119,6 @@ def redirect_request(self, request, fp, code, msg, headers, newurl):
         return urllib.request.Request(newurl, headers=headers, method=request.get_method(), unverifiable=True)
 
 
-class NoRedirect(urllib.request.HTTPRedirectHandler):
-    """Bootstrap metadata and credentials stay on the configured console origin."""
-    def redirect_request(self, request, fp, code, message, headers, newurl):
-        raise ArtifactError('Metadata downloads do not follow redirects; check the console URL '
-                                'and the reverse proxy in front of it')
-
-
 def checked_path(path):
     path = Path(path)
     if not path.is_absolute() or path.resolve() != path:
@@ -309,28 +302,11 @@ def runtime_archive(manifest, cache_root, replace=False):
     return target
 
 
-def load_manifest(source_url=None, offline_root=None):
-    """Read the matched public manifest without transmitting installation credentials."""
+def load_manifest(offline_root):
+    """Read the matched public manifest from an offline bundle."""
     def read(name):
-        if offline_root is not None:
-            with (Path(offline_root) / name).open('rb') as stream:
-                data = stream.read(1024 * 1024 + 1)
-        else:
-            if not source_url:
-                raise DistributionError('A Core source URL or offline bundle is required')
-            url = safe_url(source_url.rstrip('/') + '/node-install/' + name)
-            for attempt in range(3):
-                try:
-                    with urllib.request.build_opener(NoRedirect()).open(url, timeout=30) as stream:
-                        data = stream.read(1024 * 1024 + 1)
-                    break
-                except urllib.error.HTTPError as error:
-                    if error.code not in (408, 429, 500, 502, 503, 504) or attempt == 2:
-                        raise DistributionError(f'Distribution metadata unavailable (HTTP {error.code}); check the Core source URL') from None
-                except (urllib.error.URLError, socket.timeout, ConnectionError, http.client.HTTPException):
-                    if attempt == 2:
-                        raise DistributionError('Cannot reach distribution metadata; check network and TLS trust, then retry') from None
-                time.sleep(attempt + 1)
+        with (Path(offline_root) / name).open('rb') as stream:
+            data = stream.read(1024 * 1024 + 1)
         if len(data) > 1024 * 1024:
             raise DistributionError('Distribution metadata exceeds size limit')
         return data
@@ -348,8 +324,7 @@ def read(name):
         if (manifest.get('platform') != 'linux/amd64'
                 or not re.fullmatch(r'[0-9a-f]{40}', manifest.get('source_commit', ''))):
             raise DistributionError('Unsupported distribution platform or revision')
-        # Resolve artifacts through the console, which selects local bytes or a pinned HTTPS release.
-        manifest['artifact_base_url'] = source_url.rstrip('/') + '/node-install/artifacts' if source_url and offline_root is None else ''
+        manifest['artifact_base_url'] = ''
         return manifest
     except (ValueError, TypeError, AttributeError):
         raise DistributionError('Invalid distribution metadata') from None

deploy/node/install_display.py

@@ -1,10 +1,7 @@
 """Shared terminal presentation for the Core and node installers."""
 import os
-import contextlib
 import sys
 import textwrap
-import threading
-import time
 
 
 def color(message, code, stream=None):
@@ -18,36 +15,6 @@ def step(message):
     print("\n" + color("==> " + message + "...", "36"), flush=True)
 
 
-@contextlib.contextmanager
-def busy(message):
-    """Show elapsed time for a quiet operation whose completion percentage is unknown."""
-    stream = sys.stderr
-    if not stream.isatty() or os.environ.get("TERM") == "dumb":
-        yield
-        return
-    stopped = threading.Event()
-    started = time.monotonic()
-
-    def update():
-        frames = "|/-\\"
-        while not stopped.wait(0.5):
-            try:
-                elapsed = int(time.monotonic() - started)
-                print(f"\r{frames[elapsed % 4]} {message} ({elapsed}s)", end="\033[K", file=stream, flush=True)
-            except OSError:
-                return
-
-    worker = threading.Thread(target=update, daemon=True)
-    worker.start()
-    try:
-        yield
-    finally:
-        stopped.set()
-        worker.join()
-        with contextlib.suppress(OSError):
-            print("\r\033[K", end="", file=stream, flush=True)
-
-
 def heading(message):
     print("\n" + color(message, "1"))
 

deploy/node/node_install.py

@@ -365,6 +365,7 @@ def drop_runtime_archives(root):
     """The imported image is the cache; the archives only hold disk."""
     for name in ("images/runtime.tar", "images/runtime.tar.gz"):
         (root / name).unlink(missing_ok=True)
+        distribution.partial_path(root / name).unlink(missing_ok=True)
 
 
 def micro_env(root, args):
@@ -881,14 +882,14 @@ def host_checks():
 
 
 def root_file(path, content, replace=False):
-    """Write a root-owned 0644 file, or accept an identical existing one (or replace it)."""
+    """Write a root-owned 0644 file, or accept an identical existing one (or replace it); True when it wrote."""
     if path.is_symlink() or (path.exists() and not path.is_file()):
         raise InstallError(str(path) + " must be a regular file")
     if path.exists():
         if path.stat().st_uid != os.geteuid():
             raise InstallError(str(path) + " must be owned by root; preserve it and inspect the node")
         if path.read_text() == content:
-            return
+            return False
         if not replace:
             raise InstallError(str(path) + " differs from this installer's version; preserve it and inspect the node")
     if not path.parent.exists():
@@ -900,6 +901,7 @@ def root_file(path, content, replace=False):
             stream.write(content)
         os.chmod(temporary, 0o644)
         os.replace(temporary, path)
+    return True
 
 
 def read_root_json(path):
@@ -1139,12 +1141,24 @@ def node_record(installation_id):
     return record
 
 
+def refresh_service(installation_id, root, provider, proxy):
+    """Write the root-owned unit and proxy settings, reload systemd and restart a running service that changed."""
+    unit, environment = SYSTEM_UNITS / unit_name(installation_id), SYSTEM_RECORDS / (installation_id + ".env")
+    changed = root_file(unit, system_unit(root, provider, environment), replace=True)
+    changed = write_service_environment(environment, proxy) or changed
+    checked(["systemctl", "daemon-reload"], "Cannot reload systemd")
+    if changed:  # A running service reads its unit and environment only when it starts; a stopped one is left to enable.
+        checked(["systemctl", "try-restart", unit.name], "Cannot restart the node service with the new settings. Inspect sudo journalctl -u " + unit.name)
+
+
 def install_system(args, token):
     """Sudo mode: prepare the host, then run the node as a root-owned system service."""
     os.environ["PATH"] = SAFE_PATH
     host_checks()
     # Checks that change nothing run first, so a refusal leaves no trace, not even a lock.
-    record = node_record(args.installation_id)
+    recorded = node_record(args.installation_id)
+    # A record still marked pending belongs to an attempt that never registered; it pins nothing.
+    record = None if recorded and recorded.get("pending") else recorded
     proxy = service_environment(os.environ)
     configuration = None
     if record is None:
@@ -1162,7 +1176,7 @@ def install_system(args, token):
         other_node(args, provider)
         host_capacity(provider, configuration["specification"]["resources"], details)
     with host_lock():
-        if node_record(args.installation_id) != record or (record is None and [i for i in node_records() if i != args.installation_id]):
+        if node_record(args.installation_id) != recorded or (record is None and [i for i in node_records() if i != args.installation_id]):
             raise InstallError("Another node installation changed this host meanwhile; rerun the command." + NOTHING_CHANGED)
         account, account_record = account_plan()
         helper_archive = node_generations.helper_archive(args, sys.modules[__name__])
@@ -1172,18 +1186,18 @@ def install_system(args, token):
         child_docker_config()
         root = SERVICE_HOME / ".oac/nodes" / args.installation_id
         unit = SYSTEM_UNITS / unit_name(args.installation_id)
-        root_file(SYSTEM_RECORDS / (args.installation_id + ".json"),
-                  json_text({"format": 1, "installation_id": args.installation_id, "provider": provider,
-                             "core_url": args.core_url, "node_root": str(root), "unit": str(unit)}))
+        entry = {"format": 1, "installation_id": args.installation_id, "provider": provider,
+                 "core_url": args.core_url, "node_root": str(root), "unit": str(unit)}
+        if record is None:
+            root_file(SYSTEM_RECORDS / (args.installation_id + ".json"), json_text(dict(entry, pending=True)), replace=True)
+        else:  # A registered node's service takes the new proxy even when the node step stops.
+            refresh_service(args.installation_id, root, provider, proxy)
         args.system, args.provider = True, provider
         run_as(account, prepare_service_node, args, token, helper_archive)
-        environment = SYSTEM_RECORDS / (args.installation_id + ".env")
-        root_file(unit, system_unit(root, provider, environment))
-        changed = write_service_environment(environment, proxy)
+        if record is None:
+            root_file(SYSTEM_RECORDS / (args.installation_id + ".json"), json_text(entry), replace=True)
+            refresh_service(args.installation_id, root, provider, proxy)
         install_display.step("Starting the node service")
-        checked(["systemctl", "daemon-reload"], "Cannot reload systemd")
-        if changed:  # A running service reads its environment only when it starts; a stopped one is left to enable.
-            checked(["systemctl", "try-restart", unit.name], "Cannot restart the node service with the new proxy settings. Inspect sudo journalctl -u " + unit.name)
         checked(["systemctl", "enable", "--now", unit.name], "Cannot start the node service; its identity is kept. Inspect sudo journalctl -u " + unit.name)
         checked(["systemctl", "is-active", "--quiet", unit.name], "Node service is unavailable; inspect sudo journalctl -u " + unit.name)
         install_display.step("Waiting for Core connection and provider readiness")
@@ -1216,14 +1230,20 @@ def no_links(path):
         raise InstallError(str(path) + " is reached through a symbolic link; inspect and remove it by hand." + NOTHING_CHANGED)
 
 
+def has_node_state(root):
+    """Whether the node ever registered: only then does Core know it."""
+    return (root / "registered.json").exists() or (root / "state/node/identity.json").exists()
+
+
 def confirm_removed(root, core_url, force):
     """Continue only once Core rejects the node's credential, or when it never registered.
 
     Runs as the node's own user. The Core address comes from the caller: in sudo mode
     the root-owned record, never a file the service user can write."""
     no_links(root)
+    no_links(micro_home(root.name))  # Refused here, before anything is removed, so "Nothing was changed" holds.
     identity_file = root / "state/node/identity.json"
-    if not (root / "registered.json").exists() and not identity_file.exists():
+    if not has_node_state(root):
         return
     if force:
         print("Skipping the Core check (--force).", flush=True)
@@ -1271,14 +1291,17 @@ def release_docker_network(installation_id):
     checked(list(DOCKER) + ["network", "rm", network], "Cannot remove Docker network " + network)
 
 
-def remove_node_files(root, installation_id, delete_store):
+def remove_node_files(root, installation_id, confirmed):
     """Delete the node's state directory and, once Core confirmed the removal, its microsandbox store; keep the Runtime image.
 
+    A node without registered state was never confirmed, so its store stays.
+
     Runs as the node's own user, so a link it planted can never reach another user's files."""
     no_links(root)
     provider = private_json(root / "provider.json") or {}
     image = (provider.get("native") or {}).get("image")
     runtime_home = micro_home(installation_id)
+    delete_store = confirmed and has_node_state(root)
     if root.exists():
         shutil.rmtree(root)
     # The service user can write provider.json; print only what an image ID can be.

deploy/node/node_spec.py

@@ -3,8 +3,6 @@
 import http.client
 import json
 import re
-import shlex
-import sys
 import urllib.error
 import urllib.request
 import uuid
@@ -145,8 +143,8 @@ def fetch(args, token, retained, open_request, allow_enrollment=False, generatio
     except urllib.error.HTTPError as error:
         if error.code == 401 and retained is not None and not allow_enrollment:
             raise SpecificationError("Core no longer accepts this node: it was removed on the Nodes page, or a sandbox "
-                                     "deployment change retired it. Uninstall it with: sudo python3 " + shlex.quote(sys.argv[0]) +
-                                     " --uninstall --installation-id " + shlex.quote(args.installation_id) + ", then add the host with a new command.") from None
+                                     "deployment change retired it. Uninstall it with the command from Clean up a host on the Nodes page, "
+                                     "then add the host with a new command.") from None
         if error.code == 404:
             raise SpecificationError("Core node configuration was not found (HTTP 404); route /api/v1 on the Core origin directly to Core, not to Web") from None
         if error.code == 409:

deploy/node/test_distribution.py

@@ -239,14 +239,12 @@ def test_url_and_path_boundaries(self):
             distribution.obtain_artifact(self.manifest, 'native/bin/node', self.root / 'link')
 
 
-    def test_artifact_downgrades_and_metadata_redirects_are_refused(self):
+    def test_artifact_downgrades_are_refused(self):
         self.status = 302
         self.redirect_target = 'http://127.0.0.1:1'
         with self.assertRaisesRegex(distribution.DistributionError, 'HTTPS'):
             distribution.obtain_artifact(self.manifest, 'native/bin/node', self.root / 'node')
-        with self.assertRaisesRegex(distribution.DistributionError, 'do not follow redirects'):
-            distribution.load_manifest(source_url=f'http://127.0.0.1:{self.server.server_port}')
-        self.assertEqual(len(self.requests), 2)
+        self.assertEqual(len(self.requests), 1)
         self.assertEqual(list(self.root.iterdir()), [])
 
 
@@ -386,11 +384,6 @@ def test_console_is_the_only_artifact_source(self):
             (Path(bundle) / 'manifest.json').write_bytes(manifest)
             (Path(bundle) / 'SHA256SUMS').write_bytes(sums)
             self.assertEqual(distribution.load_manifest(offline_root=bundle)['artifact_base_url'], '')
-        files = {'SHA256SUMS': sums, 'manifest.json': manifest}
-        opener = Mock(open=lambda url, timeout: io.BytesIO(files[url.rsplit('/', 1)[1]]))
-        with patch.object(distribution.urllib.request, 'build_opener', return_value=opener):
-            loaded = distribution.load_manifest(source_url='https://console.example')
-        self.assertEqual(loaded['artifact_base_url'], 'https://console.example/node-install/artifacts')
 
 
 if __name__ == '__main__':