app store: check the host before writing anything
An install that discovers a missing dependency halfway through has already made a directory, possibly started a container and written a row, and then has to unwind — leaving the user with something that neither works nor uninstalls. A 30ms check first is worth most of that. Verified while writing this: nothing in scripts/ installs Docker, and nothing checks for it. setup-dockers.sh invokes `docker compose` with no preflight, so a fresh host without Docker fails partway through setup with a bare "command not found". Recorded in the design doc rather than fixed here — the intended fix is a setup.sh per sidecar, which is also what a sidecar needs once it ships from its own repository. `docker compose version` is the probe, not `docker --version`: the latter passes with a dead daemon, which is the failure people actually hit. "Not installed" and "daemon unreachable" are reported separately because the remedies differ. Checked per MODE, not per entry. A host without Docker can still install Photos by pointing at an Immich somewhere else; refusing the whole entry is the over-strict check that makes people work around the installer instead of using it. Dropped `requires: 'docker'` from the catalogue type. Needing Docker is exactly "this entry can provision", which `modes` already says, so declaring it twice invites the two to disagree. Derived by needsDocker instead, and a test asserts the derivation matches every entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -105,6 +105,32 @@ consequences, both wanted:
|
|||||||
|
|
||||||
`[open]` Podman, for anyone wanting genuinely rootless.
|
`[open]` Podman, for anyone wanting genuinely rootless.
|
||||||
|
|
||||||
|
### Docker is assumed, and nothing guarantees it
|
||||||
|
|
||||||
|
Verified: **nothing in `scripts/` installs Docker, and nothing checks for it.** `setup.sh` calls
|
||||||
|
`setup-dockers.sh`, which invokes `docker compose` with no preflight, so a fresh host without Docker
|
||||||
|
fails partway through setup with a bare "command not found".
|
||||||
|
|
||||||
|
That is the seam where this project's origin shows — it began as one person's own machine, provisioned
|
||||||
|
by his own scripts, where Docker was simply always there.
|
||||||
|
|
||||||
|
The intended fix is **a `setup.sh` per sidecar**, ensuring its own dependencies before its compose file
|
||||||
|
is used. That is also the shape a sidecar needs once it lives in its own repository, so a sidecar package
|
||||||
|
becomes:
|
||||||
|
|
||||||
|
```
|
||||||
|
metadata (catalogue entry) · compose template · setup.sh · schema
|
||||||
|
```
|
||||||
|
|
||||||
|
Until that exists, the app store **detects and reports** rather than guessing or half-installing:
|
||||||
|
`preflight.ts` checks `docker compose version` — which exercises the binary, the daemon connection and
|
||||||
|
the plugin in one call, unlike `docker --version`, which passes with a dead daemon — and distinguishes
|
||||||
|
"not installed" from "daemon unreachable", because the remedies differ.
|
||||||
|
|
||||||
|
The check is **per mode, not per entry**: a host without Docker can still install Photos by pointing at
|
||||||
|
an Immich elsewhere. Refusing the whole entry would be the over-strict check that makes people work
|
||||||
|
around the installer instead of using it.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Install state
|
## Install state
|
||||||
|
|||||||
@@ -61,10 +61,13 @@ export type CatalogueEntry = {
|
|||||||
/** Name of the compose template under `app-store/templates/`. Required iff `modes` includes 'provisioned'. */
|
/** Name of the compose template under `app-store/templates/`. Required iff `modes` includes 'provisioned'. */
|
||||||
composeTemplate?: string;
|
composeTemplate?: string;
|
||||||
/**
|
/**
|
||||||
* Why this cannot be installed on some hosts, if so. Shown instead of the install button rather than
|
* A host requirement that is NOT derivable from `modes`. Shown instead of the install button rather
|
||||||
* failing halfway through — a check the installer can make before it starts.
|
* than failing halfway through.
|
||||||
|
*
|
||||||
|
* Docker deliberately does not appear here: needing it is exactly "this entry can provision", which
|
||||||
|
* `modes` already says. `preflight.needsDocker` derives it, so the two cannot disagree.
|
||||||
*/
|
*/
|
||||||
requires?: 'docker' | 'linux-display';
|
requires?: 'linux-display';
|
||||||
};
|
};
|
||||||
|
|
||||||
export const CATALOGUE: CatalogueEntry[] = [
|
export const CATALOGUE: CatalogueEntry[] = [
|
||||||
@@ -79,7 +82,13 @@ export const CATALOGUE: CatalogueEntry[] = [
|
|||||||
composeTemplate: 'immich',
|
composeTemplate: 'immich',
|
||||||
existingFields: [
|
existingFields: [
|
||||||
{ key: 'url', label: 'Immich URL', type: 'url', required: true, placeholder: 'https://photos.example.com' },
|
{ key: 'url', label: 'Immich URL', type: 'url', required: true, placeholder: 'https://photos.example.com' },
|
||||||
{ key: 'secret', label: 'API key', type: 'secret', required: true, help: 'Immich → Account Settings → API Keys. Create it with all permissions: a scoped key returns 403 per route, which reads as a broken feature.' },
|
{
|
||||||
|
key: 'secret',
|
||||||
|
label: 'API key',
|
||||||
|
type: 'secret',
|
||||||
|
required: true,
|
||||||
|
help: 'Immich → Account Settings → API Keys. Create it with all permissions: a scoped key returns 403 per route, which reads as a broken feature.',
|
||||||
|
},
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -150,8 +159,21 @@ export const CATALOGUE: CatalogueEntry[] = [
|
|||||||
composeTemplate: 'transmission',
|
composeTemplate: 'transmission',
|
||||||
existingFields: [
|
existingFields: [
|
||||||
{ key: 'url', label: 'Transmission URL', type: 'url', required: true, placeholder: 'http://localhost:9091' },
|
{ key: 'url', label: 'Transmission URL', type: 'url', required: true, placeholder: 'http://localhost:9091' },
|
||||||
{ key: 'path', label: 'RPC path', type: 'text', required: false, placeholder: '/transmission/rpc', help: 'Only differs behind a reverse proxy.' },
|
{
|
||||||
{ key: 'username', label: 'RPC username', type: 'text', required: false, help: 'Usually blank — Transmission is normally run with no RPC auth.' },
|
key: 'path',
|
||||||
|
label: 'RPC path',
|
||||||
|
type: 'text',
|
||||||
|
required: false,
|
||||||
|
placeholder: '/transmission/rpc',
|
||||||
|
help: 'Only differs behind a reverse proxy.',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: 'username',
|
||||||
|
label: 'RPC username',
|
||||||
|
type: 'text',
|
||||||
|
required: false,
|
||||||
|
help: 'Usually blank — Transmission is normally run with no RPC auth.',
|
||||||
|
},
|
||||||
{ key: 'secret', label: 'RPC password', type: 'secret', required: false },
|
{ key: 'secret', label: 'RPC password', type: 'secret', required: false },
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
@@ -210,9 +232,7 @@ export const CATALOGUE: CatalogueEntry[] = [
|
|||||||
summary: 'Index and play the library on this machine',
|
summary: 'Index and play the library on this machine',
|
||||||
modes: ['config'],
|
modes: ['config'],
|
||||||
capability: 'music',
|
capability: 'music',
|
||||||
configFields: [
|
configFields: [{ key: 'library', label: 'Music folder', type: 'text', required: true, placeholder: '~/Music' }],
|
||||||
{ key: 'library', label: 'Music folder', type: 'text', required: true, placeholder: '~/Music' },
|
|
||||||
],
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
id: 'wallet',
|
id: 'wallet',
|
||||||
|
|||||||
@@ -0,0 +1,22 @@
|
|||||||
|
import { describe, expect, it } from 'bun:test';
|
||||||
|
import { needsDocker, preflight } from './preflight';
|
||||||
|
import { CATALOGUE, byId } from './catalogue';
|
||||||
|
|
||||||
|
describe('needsDocker is derived, not declared', () => {
|
||||||
|
it('is true for exactly the entries that can provision', () => {
|
||||||
|
for (const e of CATALOGUE) expect(needsDocker(e)).toBe(e.modes.includes('provisioned'));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('preflight is per mode, not per entry', () => {
|
||||||
|
it('allows pointing at an existing instance without Docker', async () => {
|
||||||
|
// The whole point: a host with no Docker can still use Photos against an Immich elsewhere.
|
||||||
|
// Refusing the entry outright is the over-strict check that makes people bypass the installer.
|
||||||
|
const photos = byId('photos')!;
|
||||||
|
expect(await preflight(photos, 'existing')).toEqual({ ok: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('allows a config-only sidecar regardless', async () => {
|
||||||
|
expect(await preflight(byId('email')!, 'config')).toEqual({ ok: true });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,84 @@
|
|||||||
|
import type { CatalogueEntry } from './catalogue';
|
||||||
|
|
||||||
|
// Can this machine install this sidecar at all — asked BEFORE anything is written.
|
||||||
|
//
|
||||||
|
// The point is the ordering. An install that discovers a missing dependency halfway through has already
|
||||||
|
// created a directory, possibly started a container, and written a row; it then has to unwind, and the
|
||||||
|
// user is left with something that neither works nor uninstalls. A check that costs 30ms up front is
|
||||||
|
// worth a great deal of that.
|
||||||
|
//
|
||||||
|
// ── What this deliberately does NOT do ──
|
||||||
|
//
|
||||||
|
// It does not install anything. Today nothing in `scripts/` installs Docker either — `setup.sh` runs
|
||||||
|
// `setup-dockers.sh`, which invokes `docker compose` without ever checking it exists, so a fresh host
|
||||||
|
// without Docker fails partway through setup with a bare "command not found". That is a real gap, and
|
||||||
|
// the intended fix is a per-sidecar `setup.sh` that ensures its own dependencies — which is also the
|
||||||
|
// shape a sidecar needs once it lives in its own repository and ships independently.
|
||||||
|
//
|
||||||
|
// Until that exists, the honest thing is to detect and report rather than to guess or to half-install.
|
||||||
|
|
||||||
|
export type Preflight =
|
||||||
|
| { ok: true }
|
||||||
|
| { ok: false; reason: string; /** What the user has to do about it. */ remedy: string };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Docker is needed to PROVISION, never to point at something already running. Derived from `modes`
|
||||||
|
* rather than declared per entry, so the two cannot drift: an entry that can provision needs Docker, by
|
||||||
|
* definition, and nobody has to remember to tick a second box.
|
||||||
|
*/
|
||||||
|
export const needsDocker = (entry: CatalogueEntry): boolean => entry.modes.includes('provisioned');
|
||||||
|
|
||||||
|
/** `docker` on PATH, the daemon reachable, and the compose plugin present. All three, or it is not usable. */
|
||||||
|
export async function checkDocker(): Promise<Preflight> {
|
||||||
|
try {
|
||||||
|
// `docker compose version` exercises the binary, the daemon connection and the plugin in one call.
|
||||||
|
// `docker --version` would pass with a dead daemon, which is the failure people actually hit.
|
||||||
|
const proc = Bun.spawn(['docker', 'compose', 'version'], { stdout: 'pipe', stderr: 'pipe' });
|
||||||
|
const code = await proc.exited;
|
||||||
|
if (code === 0) return { ok: true };
|
||||||
|
|
||||||
|
const err = (await new Response(proc.stderr).text()).trim();
|
||||||
|
// The daemon being down and the plugin being absent need different remedies, and the message is the
|
||||||
|
// only way to tell them apart — the exit code is 1 for both.
|
||||||
|
if (/permission denied|daemon|cannot connect/i.test(err)) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: 'The Docker daemon is not reachable.',
|
||||||
|
remedy:
|
||||||
|
'Start Docker (`sudo systemctl start docker`), or add your user to the `docker` group and log in again.',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: 'Docker Compose is not available.',
|
||||||
|
remedy: 'Install the Docker Compose plugin (`docker-compose-plugin`).',
|
||||||
|
};
|
||||||
|
} catch {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: 'Docker is not installed.',
|
||||||
|
remedy: 'Install Docker Engine, then try again. https://docs.docker.com/engine/install/',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Everything that must be true before `mode` can be attempted for `entry`.
|
||||||
|
*
|
||||||
|
* Split by mode on purpose: a host with no Docker can still install Photos by pointing at an Immich
|
||||||
|
* somewhere else. Refusing the whole entry would be wrong, and is the sort of over-strict check that
|
||||||
|
* makes people work around the installer instead of using it.
|
||||||
|
*/
|
||||||
|
export async function preflight(entry: CatalogueEntry, mode: string): Promise<Preflight> {
|
||||||
|
if (entry.requires === 'linux-display' && process.platform !== 'linux') {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
reason: `${entry.label} mirrors an Xorg display, which this platform does not have.`,
|
||||||
|
remedy: 'This sidecar only runs on a Linux host with a display.',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
if (mode === 'provisioned' && needsDocker(entry)) return checkDocker();
|
||||||
|
|
||||||
|
return { ok: true };
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user