Remove in memory storage of started/stopped containers

Change-type: patch
Signed-off-by: 20k-ultra <3946250+20k-ultra@users.noreply.github.com>

src/compose/app.ts

@@ -8,7 +8,6 @@ import Service from './service';
 
 import * as imageManager from './images';
 import type { Image } from './images';
-import * as applicationManager from './application-manager';
 import {
 	CompositionStep,
 	generateStep,
@@ -358,9 +357,7 @@ export class App {
 			// check that we want to stop it, and that it isn't stopped
 			return (
 				serviceTarget.config.running === false &&
-				// When we issue a stop step, we remove the containerId from this structure.
-				// We check if the container has been removed first, so that we can ensure we're not providing multiple stop steps.
-				applicationManager.containerStarted[serviceCurrent.containerId!] == null
+				serviceCurrent.status !== 'Stopped'
 			);
 		};
 

src/compose/application-manager.ts

@@ -74,9 +74,6 @@ export const router = (() => {
 	return $router;
 })();
 
-// We keep track of the containers we've started, to avoid triggering successive start
-// requests for a container
-export let containerStarted: { [containerId: string]: boolean } = {};
 export let fetchesInProgress = 0;
 export let timeSpentFetching = 0;
 
@@ -94,12 +91,6 @@ export function resetTimeSpentFetching(value: number = 0) {
 const actionExecutors = getExecutors({
 	lockFn: lock,
 	callbacks: {
-		containerStarted: (id: string) => {
-			containerStarted[id] = true;
-		},
-		containerKilled: (id: string) => {
-			delete containerStarted[id];
-		},
 		fetchStart: () => {
 			fetchesInProgress += 1;
 		},
@@ -347,9 +338,6 @@ export async function stopAll({ force = false, skipLock = false } = {}) {
 		services.map(async (s) => {
 			return lock(s.appId, { force, skipLock }, async () => {
 				await serviceManager.kill(s, { removeContainer: false, wait: true });
-				if (s.containerId) {
-					delete containerStarted[s.containerId];
-				}
 			});
 		}),
 	);

src/compose/composition-steps.ts

@@ -128,8 +128,6 @@ type LockingFn = (
 interface CompositionCallbacks {
 	// TODO: Once the entire codebase is typescript, change
 	// this to number
-	containerStarted: (containerId: string | null) => void;
-	containerKilled: (containerId: string | null) => void;
 	fetchStart: () => void;
 	fetchEnd: () => void;
 	fetchTime: (time: number) => void;
@@ -155,7 +153,6 @@ export function getExecutors(app: {
 						removeContainer: false,
 						wait,
 					});
-					app.callbacks.containerKilled(step.current.containerId);
 				},
 			);
 		},
@@ -168,7 +165,6 @@ export function getExecutors(app: {
 				},
 				async () => {
 					await serviceManager.kill(step.current);
-					app.callbacks.containerKilled(step.current.containerId);
 				},
 			);
 		},
@@ -201,15 +197,12 @@ export function getExecutors(app: {
 				},
 				async () => {
 					await serviceManager.kill(step.current, { wait: true });
-					app.callbacks.containerKilled(step.current.containerId);
-					const container = await serviceManager.start(step.target);
-					app.callbacks.containerStarted(container.id);
+					await serviceManager.start(step.target);
 				},
 			);
 		},
 		start: async (step) => {
-			const container = await serviceManager.start(step.target);
-			app.callbacks.containerStarted(container.id);
+			await serviceManager.start(step.target);
 		},
 		updateCommit: async (step) => {
 			await commitStore.upsertCommitForApp(step.appId, step.target);

test/src/compose/app.spec.ts

@@ -1,7 +1,6 @@
 import { expect } from 'chai';
 import * as sinon from 'sinon';
 import App from '../../../src/compose/app';
-import * as applicationManager from '../../../src/compose/application-manager';
 import {
 	CompositionStep,
 	CompositionStepAction,
@@ -127,17 +126,7 @@ describe('compose/app', () => {
 		sinon.stub(log, 'success');
 	});
 
-	beforeEach(() => {
-		// Cleanup application manager
-		// @ts-ignore
-		applicationManager.containerStarted = {};
-	});
-
 	after(() => {
-		// Cleanup application manager once more just in case
-		// @ts-ignore
-		applicationManager.containerStarted = {};
-
 		// Restore stubbed methods
 		sinon.restore();
 	});
@@ -780,11 +769,6 @@ describe('compose/app', () => {
 				],
 			});
 
-			// Mark this container as previously being started
-			// TODO: this is a circular dependency and is an implementation detail that should
-			// not be part of a test. NEEDS refactor
-			applicationManager.containerStarted['run_once'] = true;
-
 			// Now test that another start step is not added on this service
 			const target = createApp({
 				services: [
@@ -798,10 +782,6 @@ describe('compose/app', () => {
 
 			const steps = current.nextStepsForAppUpdate(defaultContext, target);
 			expectNoStep('start', steps);
-
-			// Cleanup application manager
-			// @ts-ignore
-			applicationManager.containerStarted = {};
 		});
 
 		it('should recreate a container if the target configuration changes', async () => {
@@ -929,12 +909,6 @@ describe('compose/app', () => {
 				networks: [defaultNetwork],
 			});
 
-			// We keep track of the containers that we've tried to start so that we
-			// dont spam start requests if the container hasn't started running
-			// TODO: this is a circular dependency and is an implementation detail that should
-			// not be part of a test. NEEDS refactor
-			applicationManager.containerStarted['dep-id'] = true;
-
 			// we should now see a start for the 'main' service...
 			const stepsToTarget = intermediate.nextStepsForAppUpdate(
 				{ ...contextWithImages, ...{ containerIds: { dep: 'dep-id' } } },
@@ -945,10 +919,6 @@ describe('compose/app', () => {
 			expect(startMainStep)
 				.to.have.property('target')
 				.that.deep.includes({ serviceName: 'main' });
-
-			// Reset the state of applicationManager
-			// @ts-ignore
-			applicationManager.containerStarted = {};
 		});
 
 		it('should not create a start step when all that changes is a running state', async () => {