Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 125 additions & 18 deletions src/commands/compute/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,65 @@ vi.mock('../../lib/errors.js', async (importOriginal) => {
import { Command } from 'commander';
import { registerComputeDeployCommand } from './deploy.js';

/**
* Route the mock by URL rather than by call order.
*
* `compute deploy` now asks /api/metadata what the provider can do before shaping
* the request, so an assertion keyed on `mock.calls[1]` breaks as soon as another
* call is added. Routing by URL survives that.
*/
function routeOssFetch(capabilities?: Record<string, unknown>) {
const caps =
capabilities ?? {
scaleToZero: true,
regions: true,
ingressModes: ['host'],
sourceBuild: 'flyctl',
deployTokenIssuance: true,
};
ossFetchMock.mockImplementation((url: string, init?: { method?: string }) => {
if (url === '/api/metadata') {
return Promise.resolve({
json: async () => ({ compute: { defaultProvider: 'test', providers: { test: caps } } }),
});
}
if (url === '/api/compute/services' && !init?.method) {
return Promise.resolve({ json: async () => [] });
}
return Promise.resolve({
json: async () => ({
id: 'svc-1',
name: 'cache',
status: 'started',
endpointUrl: 'https://cache.fly.dev',
port: 6379,
service: { name: 'cache', status: 'running' },
imageTag: 'insforge-x/cache:abc',
logs: ['Step 1/2 : FROM alpine'],
}),
});
});
}

/** Body of the POST that creates or prepares the service. */
function createCallBody(): Record<string, unknown> {
const call = ossFetchMock.mock.calls.find(
([url, init]) =>
typeof url === 'string' &&
url.startsWith('/api/compute/services') &&
(init as { method?: string } | undefined)?.method === 'POST'
);
if (!call) {
throw new Error('no create/prepare POST was made');
}
return JSON.parse((call[1] as { body: string }).body) as Record<string, unknown>;
}


describe('compute deploy --protocol', () => {
beforeEach(() => {
ossFetchMock.mockReset();
ossFetchMock.mockResolvedValueOnce({ json: async () => [] }); // initial list
ossFetchMock.mockResolvedValueOnce({
json: async () => ({ name: 'cache', status: 'started', endpointUrl: 'https://cache.fly.dev', port: 6379 }),
});
routeOssFetch();
});

it('includes protocol="tcp" in request body when --protocol tcp', async () => {
Expand All @@ -37,8 +89,7 @@ describe('compute deploy --protocol', () => {
'--protocol', 'tcp',
'--port', '6379',
]);
const createCall = ossFetchMock.mock.calls[1];
const body = JSON.parse(createCall[1].body);
const body = createCallBody();
expect(body.protocol).toBe('tcp');
expect(body.port).toBe(6379);
});
Expand All @@ -52,8 +103,7 @@ describe('compute deploy --protocol', () => {
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx', '--name', 'web', '--port', '8080',
]);
const createCall = ossFetchMock.mock.calls[1];
const body = JSON.parse(createCall[1].body);
const body = createCallBody();
expect('protocol' in body).toBe(false);
});

Expand All @@ -74,10 +124,7 @@ describe('compute deploy --protocol', () => {
describe('compute deploy --always-on / --scale-to-zero', () => {
beforeEach(() => {
ossFetchMock.mockReset();
ossFetchMock.mockResolvedValueOnce({ json: async () => [] }); // initial list
ossFetchMock.mockResolvedValueOnce({
json: async () => ({ name: 'api', status: 'running', endpointUrl: 'https://api.fly.dev', port: 8080, scaleToZero: false }),
});
routeOssFetch();
});

it('includes scaleToZero=false in request body when --always-on', async () => {
Expand All @@ -89,8 +136,7 @@ describe('compute deploy --always-on / --scale-to-zero', () => {
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx', '--name', 'api', '--always-on',
]);
const createCall = ossFetchMock.mock.calls[1];
const body = JSON.parse(createCall[1].body);
const body = createCallBody();
expect(body.scaleToZero).toBe(false);
});

Expand All @@ -103,8 +149,7 @@ describe('compute deploy --always-on / --scale-to-zero', () => {
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx', '--name', 'api', '--scale-to-zero',
]);
const createCall = ossFetchMock.mock.calls[1];
const body = JSON.parse(createCall[1].body);
const body = createCallBody();
expect(body.scaleToZero).toBe(true);
});

Expand All @@ -117,8 +162,7 @@ describe('compute deploy --always-on / --scale-to-zero', () => {
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx', '--name', 'api',
]);
const createCall = ossFetchMock.mock.calls[1];
const body = JSON.parse(createCall[1].body);
const body = createCallBody();
expect('scaleToZero' in body).toBe(false);
});

Expand All @@ -135,3 +179,66 @@ describe('compute deploy --always-on / --scale-to-zero', () => {
).rejects.toThrow(/mutually exclusive/);
});
});

describe('compute deploy against a single-host provider', () => {
const DOCKER = {
scaleToZero: false,
regions: false,
ingressModes: ['none', 'port', 'host'],
sourceBuild: 'context-upload',
deployTokenIssuance: false,
};

beforeEach(() => {
ossFetchMock.mockReset();
});

// Sending a region to a provider with one host records a choice that never takes
// effect, and nothing on screen says so.
it('omits region when the provider has none', async () => {
routeOssFetch(DOCKER);
const cmd = new Command();
cmd.exitOverride();
registerComputeDeployCommand(cmd.command('compute'));
await cmd.parseAsync([
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx:alpine', '--name', 'web', '--port', '8080',
]);
expect(createCallBody()).not.toHaveProperty('region');
});

it('omits scaleToZero when the provider cannot honour it', async () => {
routeOssFetch(DOCKER);
const cmd = new Command();
cmd.exitOverride();
registerComputeDeployCommand(cmd.command('compute'));
await cmd.parseAsync([
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx:alpine', '--name', 'web', '--port', '8080', '--always-on',
]);
expect(createCallBody()).not.toHaveProperty('scaleToZero');
});

// The gate must not be a blanket removal — a provider with regions still gets one.
it('still sends region to a provider that has regions', async () => {
routeOssFetch();
const cmd = new Command();
cmd.exitOverride();
registerComputeDeployCommand(cmd.command('compute'));
await cmd.parseAsync([
'node', 'lim', 'compute', 'deploy',
'--image', 'nginx:alpine', '--name', 'web', '--port', '8080', '--region', 'lhr',
]);
expect(createCallBody().region).toBe('lhr');
});

it('refuses source mode when the provider cannot build at all', async () => {
routeOssFetch({ ...DOCKER, sourceBuild: 'none' });
const cmd = new Command();
cmd.exitOverride();
registerComputeDeployCommand(cmd.command('compute'));
await expect(
cmd.parseAsync(['node', 'lim', 'compute', 'deploy', '.', '--name', 'web'])
).rejects.toThrow(/cannot build from source/);
});
});
121 changes: 115 additions & 6 deletions src/commands/compute/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
ensureFlyctlAvailable,
flyctlBuildAndPush,
} from '../../lib/flyctl.js';
import { fetchComputeCapabilities } from '../../lib/compute-capabilities.js';
import { packBuildContext } from '../../lib/build-context.js';

// `compute deploy` has two modes:
//
Expand Down Expand Up @@ -44,7 +46,7 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
'shared-1x'
)
.option('--memory <mb>', 'Memory in MB', '512')
.option('--region <region>', 'Fly.io region', 'iad')
.option('--region <region>', 'Region (providers that have more than one)', 'iad')
.option('--env <json>', 'Env vars as JSON object')
.option(
'--env-file <path>',
Expand Down Expand Up @@ -127,16 +129,36 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
envVars = parseEnvFile(resolve(opts.envFile));
}

// Ask the backend what its provider can do before shaping the request. A
// self-hosted Docker daemon has one region and no scale-to-zero, and
// sending those anyway records a choice that never takes effect.
const { provider, capabilities } = await fetchComputeCapabilities();
if (!capabilities.regions && process.argv.includes('--region') && !json) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Detecting an explicitly-supplied region via process.argv.includes('--region') misses the --region=<value> syntax that commander accepts. When a user writes --region=lhr against a single-host provider, no 'Ignoring --region' message is printed yet the region is still silently omitted from the request body (because capabilities.regions is false) — the exact situation the message exists to surface. Make the detection cover the =-form too (e.g. match /^--region(=|$)/ against argv), so the informative message fires regardless of flag spelling.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 136:

<comment>Detecting an explicitly-supplied region via `process.argv.includes('--region')` misses the `--region=<value>` syntax that commander accepts. When a user writes `--region=lhr` against a single-host provider, no 'Ignoring --region' message is printed yet the region is still silently omitted from the request body (because `capabilities.regions` is false) — the exact situation the message exists to surface. Make the detection cover the `=`-form too (e.g. match `/^--region(=|$)/` against argv), so the informative message fires regardless of flag spelling.</comment>

<file context>
@@ -127,16 +129,36 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+        // self-hosted Docker daemon has one region and no scale-to-zero, and
+        // sending those anyway records a choice that never takes effect.
+        const { provider, capabilities } = await fetchComputeCapabilities();
+        if (!capabilities.regions && process.argv.includes('--region') && !json) {
+          outputInfo(
+            `Ignoring --region: the ${provider ?? 'configured'} provider runs on a single host.`
</file context>

outputInfo(
`Ignoring --region: the ${provider ?? 'configured'} provider runs on a single host.`
);
}
if (!capabilities.scaleToZero && scaleToZero === false && !json) {
outputInfo(
`Ignoring --always-on: the ${provider ?? 'configured'} provider has no ` +
'scale-to-zero, so services already run continuously.'
);
}

const baseBody: Record<string, unknown> = {
name: opts.name,
port,
cpu: opts.cpu,
memory,
region: opts.region,
// Omitted where it means nothing, so the stored row does not claim a
// region the provider never honoured.
...(capabilities.regions ? { region: opts.region } : {}),
};
if (envVars) baseBody.envVars = envVars;
if (opts.protocol === 'tcp') baseBody.protocol = 'tcp';
if (scaleToZero !== undefined) baseBody.scaleToZero = scaleToZero;
if (scaleToZero !== undefined && capabilities.scaleToZero) {
baseBody.scaleToZero = scaleToZero;
}

// ─── Image mode ─────────────────────────────────────────────────
if (!dir) {
Expand Down Expand Up @@ -188,8 +210,16 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
return;
}

// ─── Source mode (Path A) ───────────────────────────────────────
// ─── Source mode ────────────────────────────────────────────────
const absDir = resolve(dir);
// Provider capability first: when it cannot build at all, whether a
// Dockerfile exists is beside the point.
if (capabilities.sourceBuild === 'none') {
throw new CLIError(
`The ${provider ?? 'configured'} compute provider cannot build from source.\n` +
` Build the image yourself and deploy it with --image <url>.`
);
}
const dockerfilePath = join(absDir, 'Dockerfile');
if (!existsSync(dockerfilePath)) {
throw new CLIError(
Expand All @@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
` • Use --image <url> to deploy a pre-built image instead`
);
}
ensureFlyctlAvailable();

if (!json) outputInfo(`Detected Dockerfile at ${dockerfilePath}`);

// ─── Source mode via context upload (self-hosted Docker) ─────────
//
// The backend owns the build here because it has the daemon: upload the
// context and it builds, tags and deploys in one call. So there is no
// deploy token to mint, no flyctl on PATH, and no follow-up PATCH.
if (capabilities.sourceBuild === 'context-upload') {
const listRes = await ossFetch('/api/compute/services');
const found = ((await listRes.json()) as Array<{ id: string; name: string }>).find(
(s) => s.name === opts.name
);

let serviceId: string;
if (found) {
serviceId = found.id;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Redeploying an existing Docker service ignores deploy settings such as --port, --memory, and --env, since this branch uploads only a tar after finding it. Update the existing service configuration before/with the build so source redeploys retain compute deploy semantics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 247:

<comment>Redeploying an existing Docker service ignores deploy settings such as `--port`, `--memory`, and `--env`, since this branch uploads only a tar after finding it. Update the existing service configuration before/with the build so source redeploys retain `compute deploy` semantics.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+
+          let serviceId: string;
+          if (found) {
+            serviceId = found.id;
+            if (!json) outputInfo(`Found existing service "${opts.name}", rebuilding...`);
+          } else {
</file context>

if (!json) outputInfo(`Found existing service "${opts.name}", rebuilding...`);
Comment on lines +246 to +248

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Existing service settings stay stale

When source-redeploying an existing Docker-backed service with changed port, CPU, memory, environment, or protocol options, this branch retains only the service ID and the subsequent request sends only the tar archive. The new image is deployed while the service silently keeps its previous configuration, which can leave the container unreachable or incorrectly configured.

Knowledge Base Used: Compute & Deployments

} else {
if (!json) outputInfo(`Creating service "${opts.name}"...`);
const prepareRes = await ossFetch('/api/compute/services/deploy', {
method: 'POST',
body: JSON.stringify(baseBody),
});
serviceId = ((await prepareRes.json()) as { id: string }).id;
}

if (!json) outputInfo('Packing build context...');
const { tar, fileCount } = await packBuildContext(absDir);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A filesystem/archiving failure after creating a new service skips rollback and leaves the prepared service behind. Include context packing in the rollback-protected operation so any source-build failure deletes a service created by this command.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 259:

<comment>A filesystem/archiving failure after creating a new service skips rollback and leaves the prepared service behind. Include context packing in the rollback-protected operation so any source-build failure deletes a service created by this command.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+          }
+
+          if (!json) outputInfo('Packing build context...');
+          const { tar, fileCount } = await packBuildContext(absDir);
+          if (!json) {
+            const mb = (tar.length / 1024 / 1024).toFixed(1);
</file context>

if (!json) {
const mb = (tar.length / 1024 / 1024).toFixed(1);
outputInfo(`Uploading ${fileCount} file(s), ${mb} MB...`);
}

let buildRes;
try {
buildRes = await ossFetch(
`/api/compute/services/${encodeURIComponent(serviceId)}/build`,
{
method: 'POST',
headers: { 'Content-Type': 'application/x-tar' },
body: tar,
}
);
} catch (buildErr) {
// Same rollback rule as the flyctl path: clean up a service this
// command created, leave an existing one running.
if (!found) {
await ossFetch(`/api/compute/services/${encodeURIComponent(serviceId)}`, {
method: 'DELETE',
}).catch(() => undefined);
if (!json) outputInfo(`Rolled back service "${opts.name}" after build failure.`);
}
throw buildErr;
}

const built = (await buildRes.json()) as {
service?: Record<string, unknown>;
imageTag: string;
logs?: string[];
};
if (json) {
outputJson(built);
} else {
for (const line of built.logs ?? []) {
console.log(` ${String(line).trimEnd()}`);
}
const svc = built.service ?? {};
const status = String(svc.status ?? 'running');
outputSuccess(`Service "${String(svc.name ?? opts.name)}" deployed [${status}]`);
console.log(` Image: ${built.imageTag}`);
if (svc.endpointUrl) {
console.log(` Endpoint: ${String(svc.endpointUrl)}`);
} else {
console.log(` No public endpoint — reachable on the project's internal network.`);
}
}
await reportCliUsage('cli.compute.deploy', true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new context-upload source-build path ends with reportCliUsage(...) but never calls trackCommandUsage('compute', 'deploy', true), unlike every other success path of this command: image mode (line 191), the flyctl remote-build path (line 419), and the shared error handler (line 442). src/lib/command-telemetry.ts documents that every command should emit trackCommandUsage exactly once per invocation, so a self-hosted Docker deploy silently produces no cli_compute_deploy_invoked telemetry while the other two modes do. Add the same trackCommandUsage call before reportCliUsage in this branch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 308:

<comment>The new context-upload source-build path ends with `reportCliUsage(...)` but never calls `trackCommandUsage('compute', 'deploy', true)`, unlike every other success path of this command: image mode (line 191), the flyctl remote-build path (line 419), and the shared error handler (line 442). `src/lib/command-telemetry.ts` documents that every command should emit `trackCommandUsage` exactly once per invocation, so a self-hosted Docker deploy silently produces no `cli_compute_deploy_invoked` telemetry while the other two modes do. Add the same `trackCommandUsage` call before `reportCliUsage` in this branch.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+              console.log(`  No public endpoint — reachable on the project's internal network.`);
+            }
+          }
+          await reportCliUsage('cli.compute.deploy', true);
+          return;
+        }
</file context>

return;
}

// ─── Source mode via flyctl remote build (Fly.io) ────────────────
ensureFlyctlAvailable();

// 1. Resolve service: list → find by name → /deploy if missing
const listRes = await ossFetch('/api/compute/services');
const existing = ((await listRes.json()) as Array<{
Expand Down
Loading
Loading