feat(BA-5669): Add optional Harbor registry installation in TUI dev installer#10963
feat(BA-5669): Add optional Harbor registry installation in TUI dev installer#10963
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in Harbor registry setup path to the dev-mode TUI installer, plus a ./dev harbor helper to manage the generated Harbor Compose stack.
Changes:
- Added CLI/install variables and service config fields to enable/configure an optional local Harbor registry
- Implemented
DevContext.configure_harbor()to download/extract Harbor offline installer, generateharbor.yml, and run Harbor’s prepare/install flow - Added
./dev harbor <start|stop|restart|status|logs>and updated post-install report with a Harbor tab
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/install/types.py | Adds Harbor-related install args/variables and service config fields |
| src/ai/backend/install/context.py | Implements Harbor download/extract/configure + hooks into install flow |
| src/ai/backend/install/cli.py | Exposes --with-harbor and Harbor configuration CLI options |
| src/ai/backend/install/app.py | Adds a Harbor tab to the post-install guide and wires args into InstallVariable |
| dev | Adds ./dev harbor subcommand wrapper around docker compose in ./harbor |
| changes/10963.feature.md | Changelog entry for the new Harbor option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await self.run_exec( | ||
| ["bash", "install.sh"], | ||
| cwd=harbor_dir, | ||
| ) | ||
| await self.run_exec( | ||
| ["docker", "compose", "down", "--remove-orphans"], | ||
| cwd=harbor_dir, | ||
| ) |
There was a problem hiding this comment.
This sequence can unintentionally stop an already-running Harbor instance when the installer is re-run (or if the user started Harbor in parallel), because docker compose down will tear down the project regardless of who started it. Prefer running Harbor’s config generation step without starting containers (e.g., invoking the bundled prepare script directly if available), or at minimum detect whether Harbor services were running before install.sh and only bring them down if they were started by this configure step.
| # Copy everything into our harbor_dir (overwriting previous contents, | ||
| # but preserving generated docker-compose.yml if idempotent re-run). | ||
| for child in extracted_harbor_dir.iterdir(): | ||
| dest = harbor_dir / child.name | ||
| if child.is_dir(): | ||
| shutil.copytree(child, dest, dirs_exist_ok=True) | ||
| else: | ||
| shutil.copy2(child, dest) |
There was a problem hiding this comment.
Copying extracted files over an existing harbor/ directory with dirs_exist_ok=True doesn’t remove files that no longer exist in the newer Harbor archive, which can leave stale scripts/configs behind and make behavior version-dependent. Consider syncing into a clean directory (e.g., delete/recreate harbor/, or extract into a temp dir and then replace the directory atomically), optionally preserving only explicitly whitelisted generated files if needed.
| self.log_header(f"Downloading Harbor offline installer from {download_uri}") | ||
| await self.run_exec( | ||
| ["curl", "-fL", "--output", str(archive_path), download_uri], | ||
| cwd=base_path, | ||
| ) |
There was a problem hiding this comment.
Downloading and executing an installer archive without any integrity verification is risky (even with TLS). Consider adding an integrity check (e.g., a pinned SHA-256 passed via CLI/config and validated before extraction) and failing fast if the checksum does not match.
| @click.option( | ||
| "--harbor-admin-password", | ||
| type=str, | ||
| default="Harbor12345", | ||
| show_default=True, | ||
| help="Initial admin password for the local Harbor instance.", | ||
| ) |
There was a problem hiding this comment.
Using a well-known default admin password (and showing it in --help) makes it easy to accidentally deploy Harbor with weak credentials. A safer default is to generate a random password when --with-harbor is enabled (and display it once in the install report), or require explicit input; at minimum, avoid advertising the default via show_default=True and add a strong warning when the default is used.
…nstaller Adds a new --with-harbor option to the dev-mode TUI installer that stands up a local Harbor container registry using Harbor's official offline installer. Behavior: - CLI flag --with-harbor (plus --harbor-http-port / --harbor-admin-password / --harbor-download-uri) propagates through CliArgs -> InstallVariable -> ServiceConfig. - DevContext.configure_harbor() downloads the Harbor offline installer archive (if not already cached), extracts it into <base>/harbor, edits the bundled harbor.yml.tmpl in-place via ruamel.yaml round-trip editing (mirroring the tomlkit pattern used for other service configs, so that comments and default values are preserved), and then runs Harbor's own install.sh to generate docker-compose.yml and the per-service config files. Containers are stopped immediately after prepare so that lifecycle is explicitly owned by the user via ./dev. - Harbor is only installed when --with-harbor is passed; configure_harbor is a no-op otherwise and no behaviour changes for existing installs. - ./dev gains a 'harbor' subcommand (start/stop/restart/status/logs) that wraps docker compose against <base>/harbor. It is separate from the tmux-managed service pipeline since Harbor runs as Docker containers. - InstallReport (post-install guide) gains a Harbor tab showing the URL and admin credentials when Harbor is enabled. No halfstack changes: Harbor is explicitly kept out of halfstack since it is not a required Backend.AI component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3a9707b to
f25191f
Compare
Summary
Add a new
--with-harboroption to the TUI installer (dev mode) that stands up a local Harbor container registry using Harbor's official offline installer. Harbor is not added to halfstack — it is managed separately via./dev harbor.How it works
CliArgs/InstallVariablegainwith_harbor,harbor_http_port,harbor_admin_password,harbor_download_uriServiceConfiggainsharbor_enabled,harbor_hostname,harbor_http_port,harbor_admin_passwordDevContext.configure_harbor():<base>/harbor/harbor.yml.tmplviaruamel.yamlround-trip (preserving comments), setshostname,http.port,harbor_admin_password,database.password,data_volume, removeshttpsblockinstall.shto generatedocker-compose.ymland all per-service config files, then immediately runsdocker compose downso that nothing is left running — lifecycle is owned explicitly by the userDevContext.configure()callsconfigure_harbor()whenwith_harbor=True, otherwise the method is a no-op./dev harbor <start|stop|restart|status|logs>subcommand wrapsdocker composeagainst<base>/harborInstallReportpost-install guide shows a Harbor tab with URL and admin credentials when Harbor is enabledDesign notes
./dev harboris separate from the tmux-managed services because Harbor runs as a bunch of Docker containers, not a single foreground process.Test plan
./backendai-install install --with-harboron a clean dev env produces a workingharbor/docker-compose.yml./dev harbor startbrings Harbor uphttp://127.0.0.1:8084with admin / Harbor12345./dev harbor stopbrings it down cleanly--with-harbor, the installer behaves exactly as beforepants lint/checkpass (verified)🤖 Generated with Claude Code