perf(action): Eliminate unnecessary caching

In testing workflow, unnecessary Docker images were saved because GitHub
Actions pre-caches a standard set of Docker images when Docker runs as
root; those images aren't accessible when using rootless-docker. Filter
Docker images so that only the empty Docker test image is saved.

src/docker.test.ts

@@ -46,27 +46,51 @@ describe("Docker images", (): void => {
 
   const mockedLoadDockerImages = async (cacheHit: boolean): Promise<void> => {
     cache.restoreCache.mockResolvedValueOnce(cacheHit ? KEY : undefined);
+    util.execBashCommand.mockResolvedValueOnce(
+      cacheHit ? "" : "test-docker-image:v1\ntest-docker-image:v2"
+    );
     await docker.loadDockerImages();
 
     expect(core.getInput).lastCalledWith("key", { required: true });
     expect(cache.restoreCache).lastCalledWith([docker.DOCKER_IMAGES_PATH], KEY);
+    if (cacheHit)
+      expect(util.execBashCommand).lastCalledWith(
+        `docker load --input ${docker.DOCKER_IMAGES_PATH}`
+      );
+    if (!cacheHit) {
+      expect(util.execBashCommand).lastCalledWith(
+        `docker image list --format "{{ .Repository }}:{{ .Tag }}"`
+      );
+      expect(core.saveState).lastCalledWith(
+        docker.DOCKER_IMAGES_LIST,
+        expect.anything()
+      );
+    }
   };
 
   const mockedSaveDockerImages = async (
     cacheHit: boolean,
-    readOnly: boolean = false
+    readOnly: boolean = false,
+    emptyImageList: boolean = false
   ): Promise<void> => {
     core.getState.mockReturnValueOnce(cacheHit.toString());
     if (!cacheHit) {
       core.getInput.mockReturnValueOnce(readOnly.toString());
     }
+    if (!readOnly && !emptyImageList) {
+      core.getState.mockReturnValueOnce("");
+      util.execBashCommand.mockResolvedValueOnce("test\ntest");
+    } else if (!readOnly) {
+      core.getState.mockReturnValueOnce("");
+      util.execBashCommand.mockResolvedValueOnce("");
+    }
     await docker.saveDockerImages();
 
     expect(core.getInput).nthCalledWith(1, "key", { required: true });
     if (!cacheHit) {
       expect(core.getInput).lastCalledWith("read-only");
     }
-    expect(core.getState).lastCalledWith(docker.CACHE_HIT);
+    expect(core.getState).nthCalledWith(1, docker.CACHE_HIT);
   };
 
   const assertCacheHitSave = (): void => {
@@ -81,6 +105,10 @@ describe("Docker images", (): void => {
     expect(docker.CACHE_HIT).toBe("cache-hit");
   });
 
+  test("exports DOCKER_IMAGES_LIST", (): void => {
+    expect(docker.DOCKER_IMAGES_LIST).toBe("docker-images-list");
+  });
+
   test("exports DOCKER_IMAGES_PATH", (): void => {
     expect(docker.DOCKER_IMAGES_PATH).toBe("~/.docker-images.tar");
   });
@@ -109,18 +137,23 @@ describe("Docker images", (): void => {
   test("aren't loaded on cache miss", async (): Promise<void> => {
     await mockedLoadDockerImages(false);
 
-    expect(core.saveState).lastCalledWith(docker.CACHE_HIT, false);
+    expect(core.saveState).nthCalledWith(1, docker.CACHE_HIT, false);
+    expect(core.saveState).lastCalledWith(
+      docker.DOCKER_IMAGES_LIST,
+      expect.anything()
+    );
     expect(core.setOutput).lastCalledWith(docker.CACHE_HIT, false);
-    expect(util.execBashCommand).not.toHaveBeenCalled();
+    expect(util.execBashCommand).lastCalledWith(
+      `docker image list --format "{{ .Repository }}:{{ .Tag }}"`
+    );
   });
 
   test("are saved on cache miss", async (): Promise<void> => {
     await mockedSaveDockerImages(false);
 
-    expect(util.execBashCommand).lastCalledWith(
-      'docker image list --format "{{ .Repository }}:{{ .Tag }}" | ' +
-        '2>&1 xargs --delimiter="\n" --no-run-if-empty --verbose --exit ' +
-        `docker save --output ${docker.DOCKER_IMAGES_PATH}`
+    expect(util.execBashCommand).nthCalledWith(
+      1,
+      'docker image list --format "{{ .Repository }}:{{ .Tag }}"'
     );
     expect(cache.saveCache).lastCalledWith([docker.DOCKER_IMAGES_PATH], KEY);
 
@@ -158,4 +191,9 @@ describe("Docker images", (): void => {
 
     assertCacheHitSave();
   });
+
+  test("aren't saved when new docker image list is empty", async (): Promise<void> => {
+    await mockedSaveDockerImages(false, false, true);
+    expect(cache.saveCache).not.toHaveBeenCalled();
+  });
 });

src/docker.ts

@@ -4,6 +4,7 @@ import { getInput, getState, info, saveState, setOutput } from "@actions/core";
 import { execBashCommand } from "./util.js";
 
 const CACHE_HIT = "cache-hit";
+const DOCKER_IMAGES_LIST = "docker-images-list";
 const DOCKER_IMAGES_PATH = "~/.docker-images.tar";
 
 const loadDockerImages = async (): Promise<void> => {
@@ -14,6 +15,11 @@ const loadDockerImages = async (): Promise<void> => {
   setOutput(CACHE_HIT, cacheHit);
   if (cacheHit) {
     await execBashCommand(`docker load --input ${DOCKER_IMAGES_PATH}`);
+  } else {
+    const dockerImagesList = await execBashCommand(
+      'docker image list --format "{{ .Repository }}:{{ .Tag }}"'
+    );
+    saveState(DOCKER_IMAGES_LIST, dockerImagesList);
   }
 };
 
@@ -26,13 +32,31 @@ const saveDockerImages = async (): Promise<void> => {
       `Cache miss occurred on the primary key ${key}. Not saving cache as read-only option was selected.`
     );
   } else {
-    await execBashCommand(
-      'docker image list --format "{{ .Repository }}:{{ .Tag }}" | ' +
-        '2>&1 xargs --delimiter="\n" --no-run-if-empty --verbose --exit ' +
-        `docker save --output ${DOCKER_IMAGES_PATH}`
+    const preexistingDockerImages = getState(DOCKER_IMAGES_LIST).split("\n");
+    const dockerImages = await execBashCommand(
+      'docker image list --format "{{ .Repository }}:{{ .Tag }}"'
     );
-    await saveCache([DOCKER_IMAGES_PATH], key);
+    const dockerImagesList = dockerImages.split("\n");
+    const newDockerImages = dockerImagesList.filter(
+      (image) => !preexistingDockerImages.includes(image)
+    );
+    if (newDockerImages.length === 0) {
+      info("No Docker images to save");
+    } else {
+      await execBashCommand(
+        `docker save --output ${DOCKER_IMAGES_PATH} ${newDockerImages.join(
+          " "
+        )}`
+      );
+      await saveCache([DOCKER_IMAGES_PATH], key);
+    }
   }
 };
 
-export { saveDockerImages, loadDockerImages, CACHE_HIT, DOCKER_IMAGES_PATH };
+export {
+  saveDockerImages,
+  loadDockerImages,
+  CACHE_HIT,
+  DOCKER_IMAGES_LIST,
+  DOCKER_IMAGES_PATH,
+};

src/integration.test.ts

@@ -15,6 +15,10 @@ const getKey = (paths: string[], key: string): string => {
 describe("Integration Test", (): void => {
   const KEY = "a-cache-key";
   const EXEC_OPTIONS = { shell: "/usr/bin/bash" };
+  const STDOUT = "standard output from Bash command";
+  const STDERR = "error output from Bash command";
+
+  const dockerImages: string[] = [];
 
   let child_process: Mocked<typeof import("node:child_process")>;
   let cache: Mocked<typeof import("@actions/cache")>;
@@ -33,6 +37,25 @@ describe("Integration Test", (): void => {
     inMemoryCache = {};
     state = {};
 
+    let callCount = 0;
+
+    child_process.exec.mockImplementation(
+      (_command: any, _options: any, callback: any): any => {
+        let stdout;
+        if (
+          _command ==
+          'docker image list --format "{{ .Repository }}:{{ .Tag }}"'
+        ) {
+          callCount += 1;
+          dockerImages.push(`test-docker-image:v${callCount}`); // When docker images list is called and Docker is running as root, docker images list generates a list that includes a standard set of Docker images.  The standard set of Docker images are pre-cached by GitHub Actions. In our test workflow, the goal is to only save an empty test Docker image instead of a comprehensive list (to minimize cache use). Generating a different Docker image list in the save step vs. the load step achieves this goal since the production code saves the difference between the two lists.
+          stdout = dockerImages.join("\n");
+        } else {
+          stdout = STDOUT;
+        }
+        callback(null, { stdout, stderr: STDERR });
+      }
+    );
+
     core.getInput.mockReturnValue(KEY);
 
     cache.saveCache.mockImplementation(
@@ -58,51 +81,40 @@ describe("Integration Test", (): void => {
     });
   });
 
-  const mockedExec = async (load: boolean, command: string): Promise<void> => {
-    const stdout = "standard output from Bash command";
-    const stderr = "error output from Bash command";
-    child_process.exec.mockImplementationOnce(
-      (_command: any, _options: any, callback: any): any => {
-        callback(null, { stdout, stderr });
-      }
-    );
-
-    await (load ? docker.loadDockerImages() : docker.saveDockerImages());
-
+  const execAsserts = (command: string, n: number): void => {
     expect(core.getInput).nthCalledWith(1, "key", { required: true });
-    expect(core.info).nthCalledWith<[string]>(1, command);
     expect(child_process.exec).lastCalledWith(
       command,
       EXEC_OPTIONS,
       expect.anything()
     );
-    expect(core.info).lastCalledWith(stdout);
-    expect(core.error).lastCalledWith(stderr);
+    expect(core.info).nthCalledWith<[string]>(n, command);
+    expect(core.error).lastCalledWith(STDERR);
     expect(core.setFailed).not.toHaveBeenCalled();
   };
 
   test("cache misses, then hits", async (): Promise<void> => {
     // Attempt first cache restore.
+    const listCommand =
+      'docker image list --format "{{ .Repository }}:{{ .Tag }}"';
     await docker.loadDockerImages();
+    execAsserts(listCommand, 1);
+    expect(core.info).lastCalledWith(dockerImages.join("\n"));
 
     // Expect cache miss since cache has never been saved.
-    expect(core.getInput).nthCalledWith(1, "key", { required: true });
     expect(core.setOutput).lastCalledWith(docker.CACHE_HIT, false);
-    expect(child_process.exec).not.toHaveBeenCalled();
-    expect(core.setFailed).not.toHaveBeenCalled();
     jest.clearAllMocks();
 
     // Run post step first time, expecting cache saved on cache miss.
-    const saveCommand =
-      'docker image list --format "{{ .Repository }}:{{ .Tag }}" | ' +
-      '2>&1 xargs --delimiter="\n" --no-run-if-empty --verbose --exit ' +
-      `docker save --output ${docker.DOCKER_IMAGES_PATH}`;
-    await mockedExec(false, saveCommand);
+    const saveCommand = `docker save --output ${docker.DOCKER_IMAGES_PATH} test-docker-image:v2`;
+    await docker.saveDockerImages();
+    execAsserts(saveCommand, 3);
     jest.clearAllMocks();
 
     // Attempt second cache restore.
     const loadCommand = `docker load --input ${docker.DOCKER_IMAGES_PATH}`;
-    await mockedExec(true, loadCommand);
+    await docker.loadDockerImages();
+    execAsserts(loadCommand, 1);
 
     // Expect cache hit since cache has been saved.
     expect(core.setOutput).lastCalledWith(docker.CACHE_HIT, true);

src/util.ts

@@ -4,15 +4,18 @@ const execAsPromised = promisify(exec);
 
 import { error, info, setFailed } from "@actions/core";
 
-const execBashCommand = async (command: string): Promise<void> => {
+const execBashCommand = async (command: string): Promise<string> => {
   info(command);
+  let output = "";
   try {
     const result = await execAsPromised(command, { shell: "/usr/bin/bash" });
-    info(result.stdout);
+    output = result.stdout;
+    info(output);
     error(result.stderr);
   } catch (error: any) {
     setFailed(error.toString());
   }
+  return output;
 };
 
 export { execBashCommand };