diff --git a/docs/API.md b/docs/API.md index f7c22db36..83804a3f5 100644 --- a/docs/API.md +++ b/docs/API.md @@ -2084,33 +2084,86 @@ The updated `ServiceJob` (advanced `expiresAt`, new entry in `extendPayments`). #### Description -Recreate the service container (no extra charge), keeping the same `expiresAt` and host ports. -Re-checks the environment service gate and access list; rejected if the service has expired. -Optionally pass `userData` to replace the stored env vars, and/or `dockerCmd`/`dockerEntrypoint` -to replace the stored CMD/ENTRYPOINT overrides — each is independent: supply it (even as `[]`) -to replace the stored value, or omit it to reuse what the service already has. This is the -recommended recovery path after a service lands in `Error` because its container died on its own -(the background health check leaves host ports/network/container record reserved specifically so -restart can reuse them), in addition to recovering from an explicit `serviceStop` or any other -terminal failure. +Recreate the service container (no extra charge), keeping the same `expiresAt`, resources and +host ports. Re-checks the environment service gate and access list; rejected if the service has +expired. This is the recommended recovery path after a service lands in `Error` because its +container died on its own (the background health check leaves host ports/network/container +record reserved specifically so restart can reuse them), in addition to recovering from an +explicit `serviceStop` or any other terminal failure. + +**Restart is atomic — either all-old or all-new, never a mix of new params over the stored job:** + +- **REUSE mode** — the request carries **none** of the container params below. The service + restarts on exactly its stored spec (image, `userData`, `dockerCmd`, `dockerEntrypoint`). Use + this to simply bounce a service back to `Running`. +- **RESPEC mode** — the request carries **any** container param. The container is rebuilt + entirely from the request: `image` becomes **required** and exactly one of `tag`/`checksum`/ + `dockerfile` applies (validated exactly like `serviceStart`). `userData`/`dockerCmd`/ + `dockerEntrypoint` are taken **as-sent** — anything omitted here is empty/unset, it is **not** + pulled from the stored job. This is the bug-fix flow: publish a fixed image under a new tag, + then restart re-supplying the full spec (e.g. same `image`, new `tag`). + +Because `image` is mandatory whenever any container param is present, you cannot ride a new +`userData`/`dockerCmd` on top of the stored image — a partial change is rejected (400). Payment, +resources and duration are always preserved; only the container spec can change. A service whose +start payment was **never claimed** (escrow lock failed or was refunded) cannot be restarted — +start a new service instead. #### Request Body +REUSE mode (bounce the service on its stored spec): + +```json +{ + "consumerAddress": "0x...", + "nonce": "123", + "signature": "0x...", + "serviceId": "0x..." +} +``` + +RESPEC mode (restart on a new image spec — `image` required, plus at most one of +`tag`/`checksum`/`dockerfile`): + ```json { "consumerAddress": "0x...", "nonce": "123", "signature": "0x...", "serviceId": "0x...", - "userData": "", - "dockerCmd": [""], - "dockerEntrypoint": [""] + "image": "myrepo/myservice", + "tag": "v2", + "userData": "", + "dockerCmd": [""], + "dockerEntrypoint": [""] } ``` +| name | type | required | description | +| --- | --- | --- | --- | +| serviceId | string | v | the service to restart | +| image | string | RESPEC | base image name (build label when `dockerfile` is set). Required as soon as any container param is present | +| tag | string | | pull by `name:tag`; mutually exclusive with `checksum`/`dockerfile` | +| checksum | string | | pull by digest `sha256:<64 hex>`; mutually exclusive with `tag`/`dockerfile` | +| dockerfile | string | | build from an inline Dockerfile; requires `allowImageBuild` on the environment; mutually exclusive with `tag`/`checksum` | +| additionalDockerFiles | object | | extra `filename → content` files for the build context (only with `dockerfile`) | +| userData | string | | ECIES-encrypted (to the node public key) JSON → the container's env-var map | +| dockerCmd | string[] | | exact container command (Docker exec-form CMD override) | +| dockerEntrypoint | string[] | | container ENTRYPOINT override | + #### Response (200) -The `ServiceJob` with a new `containerId` (same `hostPort` and `expiresAt`). +The `ServiceJob` with a new `containerId` (same `hostPort` and `expiresAt`; the `image`/`tag`/ +`checksum`/`dockerfile`/`containerImage` fields reflect the new spec in RESPEC mode). + +#### Response (400) + +Not found, expired, payment never claimed, or an invalid respec — a container param was sent +without `image`, or more than one of `tag`/`checksum`/`dockerfile` was provided. + +#### Response (403) + +`dockerfile` was supplied but the environment has `allowImageBuild=false`. --- diff --git a/docs/Ocean Node.postman_collection.json b/docs/Ocean Node.postman_collection.json index aab3d465c..c2a91cf70 100644 --- a/docs/Ocean Node.postman_collection.json +++ b/docs/Ocean Node.postman_collection.json @@ -1065,7 +1065,7 @@ ], "body": { "mode": "raw", - "raw": "{\n \"consumerAddress\": \"{{consumerAddress}}\",\n \"nonce\": \"{{nonce}}\",\n \"signature\": \"{{signature}}\",\n \"serviceId\": \"{{serviceId}}\",\n \"userData\": \"\"\n}" + "raw": "{\n \"consumerAddress\": \"{{consumerAddress}}\",\n \"nonce\": \"{{nonce}}\",\n \"signature\": \"{{signature}}\",\n \"serviceId\": \"{{serviceId}}\"\n}" }, "url": { "raw": "{{baseUrl}}/api/services/serviceRestart", @@ -1078,7 +1078,7 @@ "serviceRestart" ] }, - "description": "Recreate the service container (no extra charge), keeping the same hostPort and expiresAt. Optional userData replaces stored env vars." + "description": "Recreate the service container (no extra charge), keeping the same hostPort, resources and expiresAt. Restart is ATOMIC — all-old or all-new. REUSE mode (body above): send no container params to restart on the stored spec. RESPEC mode: send ANY container param and the container is rebuilt entirely from the request — `image` becomes required plus at most one of tag/checksum/dockerfile (validated like serviceStart), and userData/dockerCmd/dockerEntrypoint are taken as-sent (omitted means empty, NOT inherited). Bug-fix flow example: add \\\"image\\\":\\\"myrepo/myservice\\\",\\\"tag\\\":\\\"v2\\\" to the body. A container param without `image` returns 400; a dockerfile respec where allowImageBuild=false returns 403." } }, { diff --git a/docs/services.md b/docs/services.md index 97540bf38..fb62b9c79 100644 --- a/docs/services.md +++ b/docs/services.md @@ -30,7 +30,7 @@ and `signature` as query parameters (or an auth-token `Authorization` header). | `SERVICE_GET_STATUS` | `/api/services/serviceStatus` | GET | Read job status / endpoints — authenticated, owner-scoped (see notice below); poll this to follow a starting service | | `SERVICE_LIST` | `/api/services/serviceList` | GET | Node-wide service listing — authenticated, **not** owner-scoped. Default: only services currently holding a resource reservation; `status=` filters to one specific status, `includeAllStatuses=true` returns everything, `fromTimestamp` keeps services created at/after that moment. Output is listing-sanitized (no `userData`, no `dockerCmd`/`dockerEntrypoint`, no Dockerfile) | | `SERVICE_EXTEND` | `/api/services/serviceExtend` | POST | Pay to push the expiry further out | -| `SERVICE_RESTART` | `/api/services/serviceRestart` | POST | Recreate the container (no extra charge); asynchronous like start — returns once the job is `Restarting`, poll `serviceStatus` | +| `SERVICE_RESTART` | `/api/services/serviceRestart` | POST | Recreate the container (no extra charge); asynchronous like start — returns once the job is `Restarting`, poll `serviceStatus`. Optionally restart on a **new image spec** (bug-fix flow) — see below | | `SERVICE_STOP` | `/api/services/serviceStop` | POST | Tear down the container; the paid resource reservation (cpu/ram/gpu + host ports) is kept until `expiresAt`, so the service can be restarted anytime on the same endpoints | | `SERVICE_GET_TEMPLATES` | `/api/services/serviceTemplates` | GET | List operator-published service templates | | `SERVICE_GET_STREAMABLE_LOGS` | `/api/services/serviceStreamableLogs` | GET | Stream the container's live stdout/stderr logs — authenticated, owner-scoped; available while `Running` or `Error`; optional `since` to skip history | @@ -71,6 +71,24 @@ lock failed outright (e.g. insufficient funds) or was refunded before being clai cannot be restarted: it was never paid for, so restarting it would run the service for free. Start a new service instead. +**Restart can change the image — atomically.** A restart is either *all-old* or *all-new*; +it never mixes new request params over the stored job: + +- **REUSE mode** — send no container params and the service restarts on exactly its stored + spec (image, `userData`, `dockerCmd`, `dockerEntrypoint`). +- **RESPEC mode** — send *any* container param and the container is rebuilt entirely from the + request. `image` is then **required** and exactly one of `tag`/`checksum`/`dockerfile` applies + (validated like `serviceStart`); `userData`/`dockerCmd`/`dockerEntrypoint` are taken as-sent, + so anything omitted is empty — never inherited from the old job. + +This is the fix-and-redeploy flow: you started a service on your own image, found a bug, pushed +a corrected image under a new tag, and now restart on it — same `image`, new `tag` — without +losing the paid window, resources or host ports. Requiring `image` whenever any container param +is present is what makes a partial change impossible: a lone new `dockerCmd`/`userData` (which +would silently run on top of the old image) is rejected with `400`, and a `dockerfile` respec on +an environment with `allowImageBuild=false` is rejected with `403`. Payment, resources and +duration are always preserved — only the container spec changes. + **The reservation lasts the whole paid window — only `Expired` releases it.** The consumer paid for the resources for a time interval and may use them as they please within it: running the service, stopping it, restarting it. An explicit `SERVICE_STOP` therefore tears diff --git a/src/@types/commands.ts b/src/@types/commands.ts index 21ae04436..4bc087c39 100644 --- a/src/@types/commands.ts +++ b/src/@types/commands.ts @@ -469,14 +469,34 @@ export interface GetServicesCommand extends Command { updatedSince?: string } +// Restart is ATOMIC — all-old or all-new, never a mix of new params over the stored job: +// +// • REUSE mode — the request carries NONE of the container params below. The service +// restarts on exactly its stored spec (image, userData, dockerCmd, dockerEntrypoint). +// • RESPEC mode — the request carries ANY container param. The container is rebuilt +// entirely from the request: `image` becomes REQUIRED and exactly one of +// tag/checksum/dockerfile applies (validated like SERVICE_START). userData/dockerCmd/ +// dockerEntrypoint are taken as-sent; any omitted here is empty/unset — it is NOT +// pulled from the stored job. This is what makes mixing impossible: a request that +// changes only userData still has to re-supply the full image spec. +// +// Requiring `image` whenever any container param is present is the discriminator between +// the two modes, so there is no way to ride a new userData/cmd on top of the old image. export interface ServiceRestartCommand extends Command { consumerAddress: string nonce: string signature: string serviceId: string - userData?: string // optional ECIES-encrypted userData. If provided it REPLACES the stored userData (send the complete set). If omitted, the stored userData is reused — no re-supply needed. - dockerCmd?: string[] // optional. If provided (even []) it REPLACES the stored CMD override. If omitted, the stored dockerCmd is reused. - dockerEntrypoint?: string[] // optional. If provided (even []) it REPLACES the stored ENTRYPOINT override. If omitted, the stored dockerEntrypoint is reused. + // Image spec (RESPEC mode). `image` is required as soon as any container param is sent; + // provide at most one of tag/checksum/dockerfile. + image?: string // base image name (or build label when dockerfile is set) + tag?: string // pull by name:tag + checksum?: string // pull by digest: "sha256:<64 hex>" + dockerfile?: string // build from inline Dockerfile; requires allowImageBuild on the env + additionalDockerFiles?: Record // extra files in the build context + userData?: string // ECIES-encrypted (to the node's public key) JSON → container env-var map + dockerCmd?: string[] // exact container command (Docker exec-form CMD override; no shell) + dockerEntrypoint?: string[] // container ENTRYPOINT override } export interface ServiceExtendCommand extends Command { diff --git a/src/components/c2d/compute_engine_base.ts b/src/components/c2d/compute_engine_base.ts index 0c09dc84d..d6cf534f4 100644 --- a/src/components/c2d/compute_engine_base.ts +++ b/src/components/c2d/compute_engine_base.ts @@ -129,10 +129,17 @@ export abstract class C2DEngine { return await fn() } + // Restart is atomic: when an image spec is supplied (RESPEC mode) the whole container is + // rebuilt from these params; when it is absent (REUSE mode) the stored spec is reused. // eslint-disable-next-line @typescript-eslint/no-unused-vars, require-await public async restartService( serviceId: string, owner: string, + newImage?: string, + newTag?: string, + newChecksum?: string, + newDockerfile?: string, + newAdditionalDockerFiles?: Record, newUserData?: string, newDockerCmd?: string[], newDockerEntrypoint?: string[] diff --git a/src/components/c2d/compute_engine_docker.ts b/src/components/c2d/compute_engine_docker.ts index b930751e8..49e7d4a2e 100755 --- a/src/components/c2d/compute_engine_docker.ts +++ b/src/components/c2d/compute_engine_docker.ts @@ -4088,6 +4088,11 @@ export class C2DEngineDocker extends C2DEngine { public override async restartService( serviceId: string, owner: string, + newImage?: string, + newTag?: string, + newChecksum?: string, + newDockerfile?: string, + newAdditionalDockerFiles?: Record, newUserData?: string, newDockerCmd?: string[], newDockerEntrypoint?: string[] @@ -4147,6 +4152,11 @@ export class C2DEngineDocker extends C2DEngine { // Tracked like loop-launched starts so engine stop() drains an in-flight restart too. const op = this.doRestartService( job, + newImage, + newTag, + newChecksum, + newDockerfile, + newAdditionalDockerFiles, newUserData, newDockerCmd, newDockerEntrypoint @@ -4169,6 +4179,11 @@ export class C2DEngineDocker extends C2DEngine { // already validated and persisted as Restarting (see restartService above). private async doRestartService( job: ServiceJob, + newImage?: string, + newTag?: string, + newChecksum?: string, + newDockerfile?: string, + newAdditionalDockerFiles?: Record, newUserData?: string, newDockerCmd?: string[], newDockerEntrypoint?: string[] @@ -4213,18 +4228,45 @@ export class C2DEngineDocker extends C2DEngine { try { const sod = this.getC2DConfig().connection?.serviceOnDemand - // 2. Pull or rebuild image based on how the service was originally started - // (the original Dockerfile + build files are stored on the job). - if (job.dockerfile) { + // Atomic restart. An image spec on the request (RESPEC mode) rebuilds the whole + // container from these params; without one (REUSE mode) EVERY container param falls + // back to the stored job as a group — the two are never mixed. `newImage` presence is + // the discriminator (the handler makes `image` mandatory whenever any container param + // is sent), so a new userData/cmd can never ride on top of the stored image. + const respec = newImage !== undefined + const effImage = respec ? newImage : job.image + const effTag = respec ? newTag : job.tag + const effChecksum = respec ? newChecksum : job.checksum + const effDockerfile = respec ? newDockerfile : job.dockerfile + const effAdditionalDockerFiles = respec + ? newAdditionalDockerFiles + : job.additionalDockerFiles + const effContainerImage = respec + ? resolveServiceImage(effImage, effTag, effChecksum, effDockerfile, serviceId) + : job.containerImage + const effUserData = respec ? newUserData : job.userData + const effDockerCmd = respec ? newDockerCmd : job.dockerCmd + const effDockerEntrypoint = respec ? newDockerEntrypoint : job.dockerEntrypoint + + // 2. Pull or rebuild the effective image. In REUSE mode this is the original spec; in + // RESPEC mode it is whatever the caller re-supplied (e.g. a freshly-pushed tag). + if (effDockerfile) { + // Authoritative build gate (the handler fast-fails too). A REUSE-mode dockerfile + // service was already allowed at start, but re-checking is cheap and correct if the + // daemon config changed in the meantime. + if (!sod?.allowImageBuild) + throw new Error( + 'Dockerfile-based services are not allowed on this environment (allowImageBuild=false)' + ) job.status = ServiceStatusNumber.BuildImage job.statusText = ServiceStatusText[ServiceStatusNumber.BuildImage] await this.db.updateServiceJob(job) const ram = job.resources.find((r) => r.id === 'ram')?.amount const cpu = job.resources.find((r) => r.id === 'cpu')?.amount await this.buildImageFromSpec( - job.containerImage, - job.dockerfile, - job.additionalDockerFiles ?? {}, + effContainerImage, + effDockerfile, + effAdditionalDockerFiles ?? {}, sod?.maxDurationSeconds ?? job.duration, ram, cpu @@ -4233,23 +4275,16 @@ export class C2DEngineDocker extends C2DEngine { job.status = ServiceStatusNumber.PullImage job.statusText = ServiceStatusText[ServiceStatusNumber.PullImage] await this.db.updateServiceJob(job) - await this.pullImageRef(job.containerImage) + await this.pullImageRef(effContainerImage) } - // 3. Effective overrides: each REPLACES the stored value when supplied (even with an - // empty value), and falls back to the stored one when omitted (undefined). - const effectiveUserData = newUserData ?? job.userData - const effectiveDockerCmd = newDockerCmd !== undefined ? newDockerCmd : job.dockerCmd - const effectiveDockerEntrypoint = - newDockerEntrypoint !== undefined ? newDockerEntrypoint : job.dockerEntrypoint - const decryptedUserData = await decryptUserData(effectiveUserData, this.keyManager) + // 3. Decrypt the effective userData (empty in RESPEC mode when the caller omitted it). + const decryptedUserData = await decryptUserData(effUserData, this.keyManager) // 4. Rebuild env (from userData) + command/entrypoint const env = userDataToEnv(decryptedUserData) - const cmd = effectiveDockerCmd?.length ? effectiveDockerCmd : undefined - const entrypoint = effectiveDockerEntrypoint?.length - ? effectiveDockerEntrypoint - : undefined + const cmd = effDockerCmd?.length ? effDockerCmd : undefined + const entrypoint = effDockerEntrypoint?.length ? effDockerEntrypoint : undefined // 5. Rebuild port bindings — reuse already-allocated host ports const PortBindings: Record> = {} @@ -4273,7 +4308,7 @@ export class C2DEngineDocker extends C2DEngine { // 8. Create and start new container container = await this.docker.createContainer({ - Image: job.containerImage, + Image: effContainerImage, Cmd: cmd, Entrypoint: entrypoint, Env: Object.entries(env).map(([k, v]) => `${k}=${v}`), @@ -4296,12 +4331,20 @@ export class C2DEngineDocker extends C2DEngine { await container.start() CORE_LOGGER.debug(`restart ${serviceId}: container ${container.id} started`) - // 9. Update record — same expiresAt, same payment, new container/network. + // 9. Update record — same expiresAt, same payment, new container/network. In RESPEC + // mode the persisted image spec is overwritten too, so the row reflects what is + // actually running and a later restart's orphan-recovery sees the current image. + job.image = effImage + job.tag = effTag + job.checksum = effChecksum + job.dockerfile = effDockerfile + job.additionalDockerFiles = effAdditionalDockerFiles + job.containerImage = effContainerImage job.containerId = container.id job.networkId = network.id - job.userData = effectiveUserData - job.dockerCmd = effectiveDockerCmd - job.dockerEntrypoint = effectiveDockerEntrypoint + job.userData = effUserData + job.dockerCmd = effDockerCmd + job.dockerEntrypoint = effDockerEntrypoint job.status = ServiceStatusNumber.Running job.statusText = ServiceStatusText[ServiceStatusNumber.Running] await this.db.updateServiceJob(job) diff --git a/src/components/core/service/restartService.ts b/src/components/core/service/restartService.ts index ed5bcd9c3..c3bd494b8 100644 --- a/src/components/core/service/restartService.ts +++ b/src/components/core/service/restartService.ts @@ -16,7 +16,40 @@ import { decryptUserData, findServiceJobAndEngine, toPublicServiceJob } from './ export class ServiceRestartHandler extends CommandHandler { validate(command: ServiceRestartCommand): ValidateParams { - return validateCommandParameters(command, ['consumerAddress', 'serviceId']) + const commandValidation = validateCommandParameters(command, [ + 'consumerAddress', + 'serviceId' + ]) + if (!commandValidation.valid) return commandValidation + // Any container param present ⇒ RESPEC mode (the container is rebuilt entirely from + // this request). In that mode `image` is mandatory — this is the discriminator that + // makes a partial change impossible: you cannot send a new userData/dockerCmd on top + // of the stored image, you must re-supply the whole spec. When no container param is + // present the service restarts on its stored spec (REUSE mode) and no image is needed. + const respec = + command.image !== undefined || + command.tag !== undefined || + command.checksum !== undefined || + command.dockerfile !== undefined || + command.additionalDockerFiles !== undefined || + command.userData !== undefined || + command.dockerCmd !== undefined || + command.dockerEntrypoint !== undefined + if (respec) { + if (!command.image) + return buildInvalidRequestMessage( + 'Restarting with new parameters requires "image": send the full container spec, ' + + 'not a partial change (restart is all-old or all-new)' + ) + const imageModes = [command.tag, command.checksum, command.dockerfile].filter( + Boolean + ).length + if (imageModes > 1) + return buildInvalidRequestMessage( + 'Provide at most one of "tag", "checksum", "dockerfile"' + ) + } + return commandValidation } async handle(task: ServiceRestartCommand): Promise { @@ -93,8 +126,25 @@ export class ServiceRestartHandler extends CommandHandler { buildInvalidRequestMessage('Cannot restart an expired service') ) - // If newUserData is provided it REPLACES the stored userData (must be the complete set). - // Decrypt it as a validity check before touching the container. + // RESPEC mode with a Dockerfile: fast-fail if this daemon forbids image builds, so the + // caller gets an immediate 403 instead of an async BuildImage failure. The engine's + // doRestartService re-checks this as the authoritative backstop. + if (task.dockerfile) { + const sod = engine.getC2DConfig().connection?.serviceOnDemand + if (!sod?.allowImageBuild) + return { + stream: null, + status: { + httpStatus: 403, + error: + 'Dockerfile-based services are not allowed on this environment (allowImageBuild=false)' + } + } + } + + // In RESPEC mode userData (if sent) becomes the new container env — decrypt it as a + // validity check before touching the container. In REUSE mode task.userData is absent + // and the stored userData is reused untouched. if (task.userData) { try { await decryptUserData(task.userData, node.getKeyManager()) @@ -116,6 +166,11 @@ export class ServiceRestartHandler extends CommandHandler { const restarted = await engine.restartService( task.serviceId, task.consumerAddress, + task.image, + task.tag, + task.checksum, + task.dockerfile, + task.additionalDockerFiles, task.userData, task.dockerCmd, task.dockerEntrypoint diff --git a/src/components/httpRoutes/compute.ts b/src/components/httpRoutes/compute.ts index d8e68a780..3ca98d085 100644 --- a/src/components/httpRoutes/compute.ts +++ b/src/components/httpRoutes/compute.ts @@ -453,6 +453,12 @@ computeRoutes.post(`${SERVICES_API_BASE_PATH}/serviceRestart`, async (req, res) nonce: (req.body.nonce as string) || null, signature: (req.body.signature as string) || null, serviceId: (req.body.serviceId as string) || null, + image: (req.body.image as string) || undefined, + tag: (req.body.tag as string) || undefined, + checksum: (req.body.checksum as string) || undefined, + dockerfile: (req.body.dockerfile as string) || undefined, + additionalDockerFiles: + (req.body.additionalDockerFiles as Record) || undefined, userData: (req.body.userData as string) || undefined, dockerCmd: (req.body.dockerCmd as string[]) || undefined, dockerEntrypoint: (req.body.dockerEntrypoint as string[]) || undefined, diff --git a/src/test/integration/services.test.ts b/src/test/integration/services.test.ts index 2157ad38b..585e477e5 100644 --- a/src/test/integration/services.test.ts +++ b/src/test/integration/services.test.ts @@ -952,6 +952,69 @@ describe('********** Service on Demand', () => { ) }) + it('(l4) SERVICE_RESTART RESPEC → new image tag applied, same hostPort + expiresAt', async function () { + this.timeout(DEFAULT_TEST_TIMEOUT * 4) + const before = await getServiceJob(serviceId) + const oldContainerId = before.containerId + expect(before.tag).to.equal('alpine') // service was started on :alpine + + const { + consumerAddress: addr, + nonce, + signature + } = await signFor(consumerAccount, PROTOCOL_COMMANDS.SERVICE_RESTART) + // The bug-fix flow: same image name, a freshly-published tag. Restart is atomic, so + // the FULL spec is re-supplied — `image` is mandatory whenever the image changes. + const task: ServiceRestartCommand = { + command: PROTOCOL_COMMANDS.SERVICE_RESTART, + consumerAddress: addr, + nonce, + signature, + serviceId, + image: 'nginxinc/nginx-unprivileged', + tag: 'alpine-slim' + } + const resp = await new ServiceRestartHandler(oceanNode).handle(task) + assert( + resp.status.httpStatus === 200, + `expected 200, got ${resp.status.httpStatus}: ${resp.status?.error ?? ''}` + ) + const running = await pollServiceStatus(serviceId, ServiceStatusNumber.Running) + expect(running.containerId).to.not.equal(oldContainerId) + // the new image spec is persisted on the job (a later restart must see the new tag) + expect(running.tag).to.equal('alpine-slim') + expect(running.containerImage).to.equal('nginxinc/nginx-unprivileged:alpine-slim') + // identity preserved: same paid window + same forwarded host port + expect(running.endpoints[0].hostPort).to.equal(hostPort) + expect(running.expiresAt).to.equal(expiresAt) + + const res = await httpGetWithRetry(endpointUrl) + assert( + res.status === 200, + `expected nginx HTTP 200 after image-swap restart, got ${res.status}` + ) + }) + + it('(l5) SERVICE_RESTART with a container param but no image → 400 (no partial change)', async function () { + this.timeout(DEFAULT_TEST_TIMEOUT) + const { + consumerAddress: addr, + nonce, + signature + } = await signFor(consumerAccount, PROTOCOL_COMMANDS.SERVICE_RESTART) + // A lone `tag` is a partial change (mixing a new tag onto the stored image) — rejected. + const task = { + command: PROTOCOL_COMMANDS.SERVICE_RESTART, + consumerAddress: addr, + nonce, + signature, + serviceId, + tag: 'alpine' + } as ServiceRestartCommand + const resp = await new ServiceRestartHandler(oceanNode).handle(task) + expect(resp.status.httpStatus).to.equal(400) + }) + it('(m) SERVICE_STOP → Stopped, container + network removed', async function () { this.timeout(DEFAULT_TEST_TIMEOUT * 2) const before = await getServiceJob(serviceId) diff --git a/src/test/unit/compute.test.ts b/src/test/unit/compute.test.ts index 021e2f135..2cc4f6e5b 100644 --- a/src/test/unit/compute.test.ts +++ b/src/test/unit/compute.test.ts @@ -2639,7 +2639,47 @@ describe('service start/restart Docker cleanup on failure', function () { } } - it('restartService overrides dockerCmd/dockerEntrypoint when new ones are supplied', async function () { + // engine.restartService signature: + // (serviceId, owner, image, tag, checksum, dockerfile, additionalDockerFiles, + // userData, dockerCmd, dockerEntrypoint) + it('restartService (RESPEC) applies a new image tag and persists the whole spec', async function () { + const existingJob = makeRunningJobWithCmd() // started on nginx:latest + engine.db.getServiceJob = sinon.stub().resolves([existingJob]) + const container = makeContainer(false) + engine.docker = { + createNetwork: sinon.stub().resolves(network), + createContainer: sinon.stub().resolves(container), + getNetwork: sinon.stub().returns(network) + } + + // RESPEC: same image name, freshly-published tag; no cmd/entrypoint re-supplied ⇒ they + // are dropped (empty), since RESPEC takes the container spec entirely from the request. + await engine.restartService('svc-cmd', '0xowner', 'nginx', '1.27-alpine') + await Promise.allSettled([...engine.serviceOpPromises]) + + // the pull + the new container use the recomputed reference + expect(engine.pullImageRef.calledWith('nginx:1.27-alpine')).to.equal(true) + const createArgs = engine.docker.createContainer.firstCall.args[0] + expect(createArgs.Image).to.equal('nginx:1.27-alpine') + // omitted cmd/entrypoint are NOT carried over from the stored job in RESPEC mode + expect(createArgs.Cmd).to.equal(undefined) + expect(createArgs.Entrypoint).to.equal(undefined) + // the new image spec is persisted on the job + expect(existingJob.tag).to.equal('1.27-alpine') + expect(existingJob.containerImage).to.equal('nginx:1.27-alpine') + expect(existingJob.dockerCmd).to.equal(undefined) + expect( + engine.db.updateServiceJob.calledWith( + sinon.match({ + status: ServiceStatusNumber.Running, + tag: '1.27-alpine', + containerImage: 'nginx:1.27-alpine' + }) + ) + ).to.equal(true) + }) + + it('restartService (RESPEC) overrides dockerCmd/dockerEntrypoint alongside the image', async function () { const existingJob = makeRunningJobWithCmd() engine.db.getServiceJob = sinon.stub().resolves([existingJob]) const container = makeContainer(false) @@ -2652,7 +2692,12 @@ describe('service start/restart Docker cleanup on failure', function () { await engine.restartService( 'svc-cmd', '0xowner', - undefined, + 'nginx', // image (RESPEC — required to change cmd/entrypoint) + 'latest', // tag + undefined, // checksum + undefined, // dockerfile + undefined, // additionalDockerFiles + undefined, // userData ['new', 'cmd'], ['/new-entrypoint'] ) @@ -2675,7 +2720,7 @@ describe('service start/restart Docker cleanup on failure', function () { ).to.equal(true) }) - it('restartService reuses the stored dockerCmd/dockerEntrypoint when none are supplied', async function () { + it('restartService (REUSE) reuses the whole stored spec when no container params are supplied', async function () { const existingJob = makeRunningJobWithCmd() engine.db.getServiceJob = sinon.stub().resolves([existingJob]) const container = makeContainer(false) @@ -2685,10 +2730,12 @@ describe('service start/restart Docker cleanup on failure', function () { getNetwork: sinon.stub().returns(network) } - await engine.restartService('svc-cmd', '0xowner', undefined) + // no image ⇒ REUSE mode: image, cmd and entrypoint all come from the stored job + await engine.restartService('svc-cmd', '0xowner') await Promise.allSettled([...engine.serviceOpPromises]) const createArgs = engine.docker.createContainer.firstCall.args[0] + expect(createArgs.Image).to.equal('nginx:latest') // stored image reused expect(createArgs.Cmd).to.deep.equal(['old', 'cmd']) expect(createArgs.Entrypoint).to.deep.equal(['/old-entrypoint']) expect(existingJob.dockerCmd).to.deep.equal(['old', 'cmd']) @@ -2704,7 +2751,7 @@ describe('service start/restart Docker cleanup on failure', function () { ).to.equal(true) }) - it('restartService clears dockerCmd/dockerEntrypoint when explicitly given an empty array', async function () { + it('restartService (RESPEC) clears dockerCmd/dockerEntrypoint when explicitly given an empty array', async function () { const existingJob = makeRunningJobWithCmd() engine.db.getServiceJob = sinon.stub().resolves([existingJob]) const container = makeContainer(false) @@ -2714,7 +2761,19 @@ describe('service start/restart Docker cleanup on failure', function () { getNetwork: sinon.stub().returns(network) } - await engine.restartService('svc-cmd', '0xowner', undefined, [], []) + // RESPEC (image supplied) with empty cmd/entrypoint arrays ⇒ explicitly cleared + await engine.restartService( + 'svc-cmd', + '0xowner', + 'nginx', + 'latest', + undefined, + undefined, + undefined, + undefined, + [], + [] + ) await Promise.allSettled([...engine.serviceOpPromises]) const createArgs = engine.docker.createContainer.firstCall.args[0] diff --git a/src/test/unit/service/serviceHandlers.test.ts b/src/test/unit/service/serviceHandlers.test.ts index 0866c9b51..7a797a9d3 100644 --- a/src/test/unit/service/serviceHandlers.test.ts +++ b/src/test/unit/service/serviceHandlers.test.ts @@ -312,27 +312,107 @@ describe('Service handlers', () => { expect(jobs[0]).to.not.have.property('userData') }) - it('forwards dockerCmd and dockerEntrypoint overrides to engine.restartService', async () => { + // engine.restartService arg order: + // (serviceId, owner, image, tag, checksum, dockerfile, additionalDockerFiles, + // userData, dockerCmd, dockerEntrypoint) + it('RESPEC: forwards the full image spec (image+tag) + cmd/entrypoint to engine.restartService', async () => { const { node, engine } = buildFakes({ serviceJobInDb: makeJob() }) - await new ServiceRestartHandler(node).handle({ + const res = await new ServiceRestartHandler(node).handle({ ...baseTask, + image: 'myrepo/app', + tag: 'v2', dockerCmd: ['python', 'new_script.py'], dockerEntrypoint: ['/bin/new-entrypoint'] } as any) + expect(res.status.httpStatus).to.equal(200) expect(engine.restartService.calledOnce).to.equal(true) const callArgs = engine.restartService.firstCall.args expect(callArgs[0]).to.equal('svc-1') expect(callArgs[1]).to.equal(OWNER) - expect(callArgs[3]).to.deep.equal(['python', 'new_script.py']) - expect(callArgs[4]).to.deep.equal(['/bin/new-entrypoint']) + expect(callArgs[2]).to.equal('myrepo/app') // image + expect(callArgs[3]).to.equal('v2') // tag + expect(callArgs[8]).to.deep.equal(['python', 'new_script.py']) // dockerCmd + expect(callArgs[9]).to.deep.equal(['/bin/new-entrypoint']) // dockerEntrypoint }) - it('forwards undefined dockerCmd/dockerEntrypoint when not supplied, so the engine reuses stored values', async () => { + it('REUSE: no container params → every spec field forwarded as undefined so the engine reuses the stored spec', async () => { const { node, engine } = buildFakes({ serviceJobInDb: makeJob() }) await new ServiceRestartHandler(node).handle({ ...baseTask } as any) const callArgs = engine.restartService.firstCall.args - expect(callArgs[3]).to.equal(undefined) - expect(callArgs[4]).to.equal(undefined) + expect(callArgs[2]).to.equal(undefined) // image + expect(callArgs[3]).to.equal(undefined) // tag + expect(callArgs[7]).to.equal(undefined) // userData + expect(callArgs[8]).to.equal(undefined) // dockerCmd + expect(callArgs[9]).to.equal(undefined) // dockerEntrypoint + }) + + it('400 RESPEC without image — a lone dockerCmd is a partial change, which is rejected', async () => { + const { node, engine } = buildFakes({ serviceJobInDb: makeJob() }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + dockerCmd: ['python', 'x.py'] + } as any) + expect(res.status.httpStatus).to.equal(400) + expect(engine.restartService.called).to.equal(false) + }) + + it('400 RESPEC without image — a lone tag is rejected', async () => { + const { node } = buildFakes({ serviceJobInDb: makeJob() }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + tag: 'v2' + } as any) + expect(res.status.httpStatus).to.equal(400) + }) + + it('400 RESPEC without image — a lone userData is rejected', async () => { + const { node } = buildFakes({ serviceJobInDb: makeJob() }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + userData: 'ENCRYPTED' + } as any) + expect(res.status.httpStatus).to.equal(400) + }) + + it('400 RESPEC with more than one image mode (tag + checksum)', async () => { + const { node } = buildFakes({ serviceJobInDb: makeJob() }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + image: 'myrepo/app', + tag: 'v2', + checksum: 'sha256:deadbeef' + } as any) + expect(res.status.httpStatus).to.equal(400) + }) + + it('403 RESPEC dockerfile when the environment forbids image builds', async () => { + const { node, engine } = buildFakes({ serviceJobInDb: makeJob() }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + image: 'myrepo/app', + dockerfile: 'FROM nginx' + } as any) + expect(res.status.httpStatus).to.equal(403) + expect(engine.restartService.called).to.equal(false) + }) + + it('200 RESPEC dockerfile when allowImageBuild is enabled — forwards the dockerfile', async () => { + const { node, engine } = buildFakes({ serviceJobInDb: makeJob() }) + engine.getC2DConfig.returns({ + hash: 'hash-1', + connection: { + serviceOnDemand: { maxDurationSeconds: 86400, allowImageBuild: true } + } + }) + const res = await new ServiceRestartHandler(node).handle({ + ...baseTask, + image: 'myrepo/app', + dockerfile: 'FROM nginx' + } as any) + expect(res.status.httpStatus).to.equal(200) + expect(engine.restartService.calledOnce).to.equal(true) + const callArgs = engine.restartService.firstCall.args + expect(callArgs[5]).to.equal('FROM nginx') // dockerfile }) })