From 7e9e59885ef7409c4b7259bc31df2af1b797ead2 Mon Sep 17 00:00:00 2001 From: Joel Clemence <15887464+JoelClemence@users.noreply.github.com> Date: Tue, 8 Oct 2024 08:14:38 +0100 Subject: [PATCH] Update development disable command to pull service by default (#110) * Currently when service is disabled in development mode if a user does not delete it or pull it manually, the image will remain to be the image built by development mode potentially. * Pulling the image after disabling will mean that the service is reloaded as the one held in ECR * Added the `-P` to the disable command to maintain current behaviour if desired * Reorganied `disable.spec.ts` mocks to use automatic mocks and not manual mock * Add a method which can be overidden in children of AbstractStateModificationCommand so that actions can occur after the hook has successfully run --- README.md | 5 +- .../AbstractStateModificationCommand.ts | 6 +- src/commands/development/disable.ts | 41 +++++++- src/run/docker-compose.ts | 12 +++ src/run/logs/LogNothingLogHandler.ts | 8 ++ test/commands/development/disable.spec.ts | 99 +++++++++++++++---- test/run/docker-compose.spec.ts | 33 +++++++ test/run/logs/LogNothingLogHandler.spec.ts | 62 ++++++++++++ 8 files changed, 242 insertions(+), 24 deletions(-) create mode 100644 src/run/logs/LogNothingLogHandler.ts create mode 100644 test/run/logs/LogNothingLogHandler.spec.ts diff --git a/README.md b/README.md index 645792d..0c47877 100644 --- a/README.md +++ b/README.md @@ -232,11 +232,14 @@ Removes a service from development mode ``` USAGE - $ chs-dev development disable SERVICES... + $ chs-dev development disable SERVICES... [-P] ARGUMENTS SERVICES... names of services to be removed to development mode +FLAGS + -P, --noPull Does not perform a docker compose pull to reset the service to what is stored in ECR + DESCRIPTION Removes a service from development mode ``` diff --git a/src/commands/AbstractStateModificationCommand.ts b/src/commands/AbstractStateModificationCommand.ts index 8a7c44f..63b8e77 100644 --- a/src/commands/AbstractStateModificationCommand.ts +++ b/src/commands/AbstractStateModificationCommand.ts @@ -58,7 +58,9 @@ export default abstract class AbstractStateModificationCommand extends Command { } if (runHook) { - this.config.runHook("generate-runnable-docker-compose", {}); + await this.config.runHook("generate-runnable-docker-compose", {}); + + await this.handlePostHookCall(argv as string[]); } } @@ -69,6 +71,8 @@ export default abstract class AbstractStateModificationCommand extends Command { return this.parse(AbstractStateModificationCommand); } + protected async handlePostHookCall (_: string[]): Promise {} + private get properStateModificationObjectType (): string { return `${this.stateModificationObjectType.substring(0, 1).toUpperCase()}${this.stateModificationObjectType.substring(1)}`; } diff --git a/src/commands/development/disable.ts b/src/commands/development/disable.ts index 9f22ab0..d018b51 100644 --- a/src/commands/development/disable.ts +++ b/src/commands/development/disable.ts @@ -1,6 +1,7 @@ -import { Args, Config } from "@oclif/core"; +import { Args, Config, Flags, ux } from "@oclif/core"; import AbstractStateModificationCommand from "../AbstractStateModificationCommand.js"; import { serviceValidator } from "../../helpers/validator.js"; +import { DockerCompose } from "../../run/docker-compose.js"; export default class Disable extends AbstractStateModificationCommand { @@ -14,16 +15,52 @@ export default class Disable extends AbstractStateModificationCommand { }) }; + static flags = { + noPull: Flags.boolean({ + name: "no-pull", + required: false, + default: false, + char: "P", + aliases: ["noPull"], + description: "Does not perform a docker compose pull to reset the service to what is stored in ECR" + }) + }; + + private readonly dockerCompose: DockerCompose; + constructor (argv: string[], config: Config) { super(argv, config, "service"); this.argumentValidationPredicate = serviceValidator(this.inventory, this.error, true); this.validArgumentHandler = this.handleValidService; + this.dockerCompose = new DockerCompose( + this.chsDevConfig, { + log: this.log + } + ); } private async handleValidService (serviceName: string): Promise { this.stateManager.excludeServiceFromLiveUpdate(serviceName); - this.log(`Service "${serviceName}" is disabled`); } + protected async handlePostHookCall (serviceNames: string[]): Promise { + + const noPull = this.flagValues?.noPull || false; + + if (!noPull) { + for (const serviceName of serviceNames) { + ux.action.start(`pulling service container: ${serviceName}`); + await this.dockerCompose.pull(serviceName); + ux.action.stop(); + + this.log(`Service "${serviceName}" is disabled`); + } + + } + } + + protected parseArgumentsAndFlags (): Promise<{ argv: unknown[]; flags: Record; }> { + return this.parse(Disable); + } } diff --git a/src/run/docker-compose.ts b/src/run/docker-compose.ts index e35576e..ac34f63 100644 --- a/src/run/docker-compose.ts +++ b/src/run/docker-compose.ts @@ -8,6 +8,7 @@ import Config from "../model/Config.js"; import LogEverythingLogHandler from "./logs/LogEverythingLogHandler.js"; import { spawn } from "../helpers/spawn-promise.js"; import { runStatusColouriser, stopStatusColouriser } from "../helpers/colouriser.js"; +import LogNothingLogHandler from "./logs/LogNothingLogHandler.js"; interface Logger { log: (msg: string) => void; @@ -117,6 +118,17 @@ export class DockerCompose { signal); } + pull (serviceName: string, abortSignal?: AbortSignal): Promise { + return this.runDockerCompose( + [ + "pull", + serviceName + ], + new LogNothingLogHandler(this.logFile, this.logger), + abortSignal + ); + } + private createStatusMatchLogHandler (pattern: RegExp, colouriser?: (status: string) => string) { return new PatternMatchingConsoleLogHandler( pattern, this.logFile, this.logger, colouriser diff --git a/src/run/logs/LogNothingLogHandler.ts b/src/run/logs/LogNothingLogHandler.ts new file mode 100644 index 0000000..ed78248 --- /dev/null +++ b/src/run/logs/LogNothingLogHandler.ts @@ -0,0 +1,8 @@ +import { AbstractLogHandler, Logger, LogHandler } from "./logs-handler.js"; + +export class LogNothingLogHandler extends AbstractLogHandler { + protected logToConsole (_: string[]): void { } + +} + +export default LogNothingLogHandler; diff --git a/test/commands/development/disable.spec.ts b/test/commands/development/disable.spec.ts index e90511e..5a0439d 100644 --- a/test/commands/development/disable.spec.ts +++ b/test/commands/development/disable.spec.ts @@ -1,32 +1,34 @@ import { expect, jest } from "@jest/globals"; import { modules, services } from "../../utils/data"; import Disable from "../../../src/commands/development/disable"; +import { Inventory } from "../../../src/state/inventory"; +import { StateManager } from "../../../src/state/state-manager"; import { Config } from "@oclif/core"; +import Module from "../../../src/model/Module"; +import { DockerCompose } from "../../../src/run/docker-compose"; const excludeServiceFromLiveUpdateMock = jest.fn(); let snapshot; -jest.mock("../../../src/state/inventory", () => { - return { - Inventory: function () { - return { - services, - modules - }; - } - }; -}); +const inventoryMock = { + services, + modules: modules as Module[] +}; -jest.mock("../../../src/state/state-manager", () => { - return { - StateManager: function () { - return { - snapshot, - excludeServiceFromLiveUpdate: excludeServiceFromLiveUpdateMock - }; - } - }; -}); +const stateManagerMock = { + snapshot, + excludeServiceFromLiveUpdate: excludeServiceFromLiveUpdateMock +}; + +const dockerComposeMock = { + pull: jest.fn() +}; + +jest.mock("../../../src/state/inventory"); + +jest.mock("../../../src/state/state-manager"); + +jest.mock("../../../src/run/docker-compose"); describe("development disable", () => { const cwdSpy = jest.spyOn(process, "cwd"); @@ -41,6 +43,15 @@ describe("development disable", () => { cwdSpy.mockReturnValue("./project"); + // @ts-expect-error + Inventory.mockReturnValue(inventoryMock); + + // @ts-expect-error + StateManager.mockReturnValue(stateManagerMock); + + // @ts-expect-error + DockerCompose.mockReturnValue(dockerComposeMock); + developmentDisable = new Disable( // @ts-expect-error [], { cacheDir: "./caches", runHook: runHookMock } as Config @@ -60,6 +71,9 @@ describe("development disable", () => { command: "disable", services: "" }, + flags: { + noPull: false + }, argv: [] }); @@ -75,6 +89,9 @@ describe("development disable", () => { args: { services: "" }, + flags: { + noPull: false + }, argv: [ "", "", @@ -95,6 +112,9 @@ describe("development disable", () => { command: "disable", services: serviceName }, + flags: { + noPull: false + }, argv: [ serviceName ] @@ -111,6 +131,9 @@ describe("development disable", () => { args: { services: "service-two" }, + flags: { + noPull: false + }, argv: ["service-two"] }); @@ -123,4 +146,40 @@ describe("development disable", () => { expect(excludeServiceFromLiveUpdateMock).toHaveBeenCalledWith("service-two"); }); + + it("runs docker compose pull", async () => { + // @ts-expect-error + parseMock.mockResolvedValue({ + args: { + services: "service-two" + }, + flags: { + noPull: false + }, + argv: ["service-two"] + }); + + await expect(developmentDisable.run()).resolves.toBeUndefined(); + + expect(dockerComposeMock.pull).toHaveBeenCalledWith( + "service-two" + ); + }); + + it("does not run docker compose pull when noPull true", async () => { + // @ts-expect-error + parseMock.mockResolvedValue({ + args: { + services: "service-two" + }, + flags: { + noPull: true + }, + argv: ["service-two"] + }); + + await expect(developmentDisable.run()).resolves.toBeUndefined(); + + expect(dockerComposeMock.pull).not.toHaveBeenCalled(); + }); }); diff --git a/test/run/docker-compose.spec.ts b/test/run/docker-compose.spec.ts index 5deb75f..bd1d01e 100644 --- a/test/run/docker-compose.spec.ts +++ b/test/run/docker-compose.spec.ts @@ -551,4 +551,37 @@ describe("DockerCompose", () => { ], expect.anything()); }); }); + + describe("pull", () => { + + let dockerCompose; + const mockStdout = jest.fn(); + + const mockSterr = jest.fn(); + const mockOnce = jest.fn(); + + beforeEach(() => { + jest.resetAllMocks(); + dockerCompose = new DockerCompose(config, logger); + + spawnMock.mockResolvedValue(undefined as never); + + }); + + it("pulls the service", async () => { + await dockerCompose.pull( + "my-awesome-service" + ); + + expect(spawnMock).toHaveBeenCalledWith( + "docker", + [ + "compose", + "pull", + "my-awesome-service" + ], + expect.anything() + ); + }); + }); }); diff --git a/test/run/logs/LogNothingLogHandler.spec.ts b/test/run/logs/LogNothingLogHandler.spec.ts new file mode 100644 index 0000000..4647695 --- /dev/null +++ b/test/run/logs/LogNothingLogHandler.spec.ts @@ -0,0 +1,62 @@ +import { expect, jest } from "@jest/globals"; +import fs from "fs"; +import LogNothingLogHandler from "../../../src/run/logs/LogNothingLogHandler"; + +describe("LogNothingLogHandler", () => { + + const writeFileSyncSpy = jest.spyOn(fs, "writeFileSync"); + const logFile = "/home/user/project/something.log"; + + beforeEach(() => { + jest.resetAllMocks(); + + writeFileSyncSpy.mockImplementation((_, __) => {}); + }); + + it("does not log any lines", () => { + const loggerMock = { + log: jest.fn() + }; + + const logNothingLogHandler = new LogNothingLogHandler(logFile, loggerMock); + + logNothingLogHandler.handle("one\ntwo\n\tthree\n"); + + expect(loggerMock.log).not.toHaveBeenCalled(); + }); + + it("does write log entries to file", () => { + const logLines = [ + "Watch configuration for service \"overseas-application\"", + "- Action rebuild for path \"/do\"", + "another log entry" + ]; + const logMessage = logLines.join("\n"); + const loggerMock = { + log: jest.fn() + }; + + const logNothingLogHandler = new LogNothingLogHandler(logFile, loggerMock); + + logNothingLogHandler.handle(logMessage); + + expect(writeFileSyncSpy).toHaveBeenCalledTimes(1); + + const call = writeFileSyncSpy.mock.calls[0]; + + expect(call[0]).toBe(logFile); + expect(call[2]).toEqual({ + flag: "a" + }); + + const contentsWrittenToFile = call[1]; + + for (const expectedLine of logLines) { + // eslint-disable-next-line no-useless-escape + const pattern = new RegExp(`\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\.\\d{3}Z\\s-\\s${expectedLine}`, "g"); + + expect(pattern.test(contentsWrittenToFile.toString())).toBe(true); + } + }); + +});