Skip to content

Commit

Permalink
feat(docker): Support dangling Docker images
Browse files Browse the repository at this point in the history
Docker images that were overwritten by a new one
with the same name and tag do not have a name or
tag. Docker images that were pulled by digest do
not have a tag. Support the former by falling back
on their ID and the latter by using their name.

These changes prevent the action from crashing on
such images. Continue using both the name and tag
for images that have them so that they are
retained upon restoring the cache.
  • Loading branch information
Kurt-von-Laven committed Mar 27, 2024
1 parent 85c8315 commit 890a6fb
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 16 deletions.
16 changes: 11 additions & 5 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,15 @@ jobs:
uses: ./
with:
key: docker-cache-test-${{ matrix.os }}-${{ github.run_id }}-${{ github.run_attempt }}
- name: Build an empty test Docker image.
- name: Build one tagged and one dangling empty test Docker image.
run: |
if [[ "${{ matrix.os }}" =~ 'windows' ]]; then
DOCKERFILE='Dockerfile.windows'
else
DOCKERFILE='Dockerfile'
fi
docker build --tag empty . --file "$DOCKERFILE"
docker build --tag empty . --file "$DOCKERFILE" # Dangling image
docker build --tag empty --no-cache . --file "$DOCKERFILE" # Tagged image
shell: bash
restore-cache:
name: Restore Cache
Expand All @@ -115,10 +116,15 @@ jobs:
uses: ./
with:
key: docker-cache-test-${{ matrix.os }}-${{github.run_id }}-${{ github.run_attempt }}
- name: Verify Docker loaded empty image from cache.
- name: Verify Docker loaded both empty images from cache.
run: |
description="$(docker inspect --format '{{ index .Config.Labels "description" }}' empty)"
[[ $description == 'empty image' ]]
function emptyImageExists() {
id="$(docker images --quiet --filter "label=description=empty image" "$@")"
[[ -n "$id" ]]
}
emptyImageExists empty
emptyImageExists --filter "dangling=true"
shell: bash
- name: Delete test cache if permitted (i.e., workflow not triggered from fork).
if: always()
Expand Down
2 changes: 1 addition & 1 deletion dist/main/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/post/index.js

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/docker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ const assertCalledInOrder = <T extends FunctionLike>(
};

describe("Docker images", (): void => {
const LIST_COMMAND =
"docker image list --format '" +
'{{ if ne .Repository "<none>" }}{{ .Repository }}' +
`{{ if ne .Tag "<none>" }}:{{ .Tag }}{{ end }}{{ else }}{{ .ID }}{{ end }}'`;

const assertLoadDockerImages = (key: string, cacheHit: boolean): void => {
expect(core.getInput).lastCalledWith("key", { required: true });
expect(cache.restoreCache).lastCalledWith([docker.DOCKER_IMAGES_PATH], key);
Expand All @@ -64,9 +69,7 @@ describe("Docker images", (): void => {
`docker load --input ${docker.DOCKER_IMAGES_PATH}`,
);
} else {
expect(util.execBashCommand).lastCalledWith(
'docker image list --format "{{ .Repository }}:{{ .Tag }}"',
);
expect(util.execBashCommand).lastCalledWith(LIST_COMMAND);
}
expect(util.execBashCommand).toHaveBeenCalledTimes(1);
};
Expand Down Expand Up @@ -106,10 +109,7 @@ describe("Docker images", (): void => {
1,
"Listing Docker images.",
);
expect(util.execBashCommand).nthCalledWith<[string]>(
1,
'docker image list --format "{{ .Repository }}:{{ .Tag }}"',
);
expect(util.execBashCommand).nthCalledWith<[string]>(1, LIST_COMMAND);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const CACHE_HIT = "cache-hit";
const DOCKER_IMAGES_LIST = "docker-images-list";
const DOCKER_IMAGES_PATH = "~/.docker-images.tar";
const LIST_COMMAND =
'docker image list --format "{{ .Repository }}:{{ .Tag }}"';
"docker image list --format '" +
'{{ if ne .Repository "<none>" }}{{ .Repository }}' +
`{{ if ne .Tag "<none>" }}:{{ .Tag }}{{ end }}{{ else }}{{ .ID }}{{ end }}'`;

const loadDockerImages = async (): Promise<void> => {
const requestedKey = getInput("key", { required: true });
Expand Down
4 changes: 3 additions & 1 deletion src/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ const getKey = (paths: string[], primaryKey: string): string =>
describe("Integration Test", (): void => {
const EXEC_OPTIONS = { shell: "/usr/bin/bash" };
const LIST_COMMAND =
'docker image list --format "{{ .Repository }}:{{ .Tag }}"';
"docker image list --format '" +
'{{ if ne .Repository "<none>" }}{{ .Repository }}' +
`{{ if ne .Tag "<none>" }}:{{ .Tag }}{{ end }}{{ else }}{{ .ID }}{{ end }}'`;

let loadCommand: string;
let inMemoryCache: Map<string, string>;
Expand Down

0 comments on commit 890a6fb

Please sign in to comment.