diff --git a/src/compose/composition-steps.ts b/src/compose/composition-steps.ts index ded3b966..111c0fae 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -14,9 +14,6 @@ import type { DeviceLegacyReport } from '../types/state'; interface CompositionStepArgs { stop: { current: Service; - options?: { - wait?: boolean; - }; }; kill: { current: Service; @@ -119,10 +116,7 @@ export function getExecutors(app: { callbacks: CompositionCallbacks }) { stop: async (step) => { // Should always be preceded by a takeLock step, // so the call is executed assuming that the lock is taken. - await serviceManager.kill(step.current, { - removeContainer: false, - wait: step.options?.wait || false, - }); + await serviceManager.stop(step.current); }, kill: async (step) => { // Should always be preceded by a takeLock step, diff --git a/src/compose/service-manager.ts b/src/compose/service-manager.ts index cb782ecf..6e1806df 100644 --- a/src/compose/service-manager.ts +++ b/src/compose/service-manager.ts @@ -1,6 +1,5 @@ import type Dockerode from 'dockerode'; import { EventEmitter } from 'events'; -import { isLeft } from 'fp-ts/lib/Either'; import JSONStream from 'JSONStream'; import _ from 'lodash'; import { promises as fs } from 'fs'; @@ -10,7 +9,6 @@ import * as config from '../config'; import { docker } from '../lib/docker-utils'; import * as logger from '../logger'; -import { PermissiveNumber } from '../config/types'; import * as constants from '../lib/constants'; import type { StatusCodeError } from '../lib/errors'; import { @@ -38,7 +36,6 @@ type ServiceManagerEventEmitter = StrictEventEmitter< const events: ServiceManagerEventEmitter = new EventEmitter(); interface KillOpts { - removeContainer?: boolean; wait?: boolean; } @@ -201,6 +198,15 @@ export async function killAllLegacy(): Promise { } } +export function stop(service: Service) { + if (service.containerId == null) { + throw new InternalInconsistencyError( + `Attempt to stop container without containerId! Service :${service}`, + ); + } + return stopContainer(service.containerId, service); +} + export function kill(service: Service, opts: KillOpts = {}) { if (service.containerId == null) { throw new InternalInconsistencyError( @@ -536,10 +542,43 @@ function reportNewStatus( ); } +async function stopContainer( + containerId: string, + service: Partial = {}, +) { + logger.logSystemEvent(LogTypes.stopService, { service }); + if (service.imageId != null) { + reportNewStatus(containerId, service, 'Stopping'); + } + + try { + await docker.getContainer(containerId).stop(); + delete containerHasDied[containerId]; + logger.logSystemEvent(LogTypes.stopServiceSuccess, { service }); + } catch (e) { + if (isStatusError(e) && e.statusCode === 304) { + delete containerHasDied[containerId]; + logger.logSystemEvent(LogTypes.stopServiceSuccess, { service }); + } else if (isStatusError(e) && e.statusCode === 404) { + logger.logSystemEvent(LogTypes.stopService404Error, { service }); + delete containerHasDied[containerId]; + } else { + logger.logSystemEvent(LogTypes.stopServiceError, { + service, + error: e, + }); + } + } finally { + if (service.imageId != null) { + reportChange(containerId); + } + } +} + async function killContainer( containerId: string, service: Partial = {}, - { removeContainer = true, wait = false }: KillOpts = {}, + { wait = false }: KillOpts = {}, ): Promise { // To maintain compatibility of the `wait` flag, this function is not // async, but it feels like whether or not the promise should be waited on @@ -553,29 +592,9 @@ async function killContainer( const containerObj = docker.getContainer(containerId); const killPromise = containerObj - .stop() - .then(() => { - if (removeContainer) { - return containerObj.remove({ v: true }); - } - }) + .remove({ v: true, force: true }) .catch((e) => { - // Get the statusCode from the original cause and make sure it's - // definitely an int for comparison reasons - const maybeStatusCode = PermissiveNumber.decode(e.statusCode); - if (isLeft(maybeStatusCode)) { - throw new Error(`Could not parse status code from docker error: ${e}`); - } - const statusCode = maybeStatusCode.right; - - // 304 means the container was already stopped, so we can just remove it - if (statusCode === 304) { - logger.logSystemEvent(LogTypes.stopServiceNoop, { service }); - // Why do we attempt to remove the container again? - if (removeContainer) { - return containerObj.remove({ v: true }); - } - } else if (statusCode === 404) { + if (isStatusError(e) && e.statusCode === 304) { // 404 means the container doesn't exist, precisely what we want! logger.logSystemEvent(LogTypes.stopRemoveServiceNoop, { service, diff --git a/src/lib/log-types.ts b/src/lib/log-types.ts index 8ecf0a9c..78814a1f 100644 --- a/src/lib/log-types.ts +++ b/src/lib/log-types.ts @@ -19,6 +19,10 @@ export const stopRemoveServiceNoop: LogType = { eventName: 'Service already stopped and container removed', humanName: 'Service is already stopped and the container removed', }; +export const stopService404Error: LogType = { + eventName: 'Service stop error', + humanName: 'Service container not found', +}; export const stopServiceError: LogType = { eventName: 'Service stop error', humanName: 'Failed to kill service',