Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: cache test images for integration, VM and module tests #7599

Merged
merged 6 commits into from
Sep 26, 2024
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,24 @@ jobs:
with:
aqua_version: v1.25.0

- name: Restore test images from cache
uses: actions/cache@v4
id: restore-test-images
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We no longer need id.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to merge this PR soon. We can delete it later.

with:
path: integration/testdata/fixtures/images
key: cache-test-images-2024-09-10 # trivy-test-images last update date
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 I added trivy-test-images (trivy-test-vm-images) last update date to add the ability to control and influence the cache.
Tell me if you have other ideas.

Copy link
Collaborator

@knqyf263 knqyf263 Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to automatically invalidate the cache when a new image is added to trivy-test-images rather than modifying the key manually. How about something like this?

      - name: Generate image list digest
        id: image-digest
        run: |
          IMAGE_LIST=$(skopeo list-tags docker://ghcr.io/aquasecurity/trivy-test-images) # skopeo is pre-installed
          DIGEST=$(echo "$IMAGE_LIST" | sha256sum | cut -d' ' -f1)
          echo "digest=$DIGEST" >> $GITHUB_OUTPUT

      - name: Restore test images from cache
        uses: actions/cache@v4
        id: restore-test-images
        with:
          path: integration/testdata/fixtures/images
          key: cache-test-images-${{ steps.image-digest.outputs.digest }}
          restore-keys:
            cacche-test-images-

      - name: Download missing test images
        if: steps.restore-test-images.outputs.cache-hit != 'true'
        run: mage test:fixtureContainerImages

It doesn't work if the existing images are updated, though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we don't need a step to save cache because it automatically creates a new cache.

On a cache miss, the action automatically creates a new cache if the job completes successfully.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows#matching-a-cache-key

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work if the existing images are updated, though.

Ideally, we should calculate a cache key from each image manifest, but a list of images should be enough at the moment. We can think about it later.

          REPO="ghcr.io/aquasecurity/trivy-test-images"
          TAGS=$(crane ls $REPO)
          COMBINED_DIGEST=""
          for TAG in $TAGS; do
            DIGEST=$(crane manifest $REPO:$TAG | sha256sum | cut -d' ' -f1)
            COMBINED_DIGEST="${COMBINED_DIGEST}${DIGEST}"
          done
          FINAL_DIGEST=$(echo "$COMBINED_DIGEST" | sha256sum | cut -d' ' -f1)
          echo "digest=$FINAL_DIGEST" >> $GITHUB_OUTPUT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we should calculate a cache key from each image manifest, but a list of images should be enough at the moment. We can think about it later.

we should definitely do this in the future, because when updating the test image - the digest will remain the same

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should definitely do this in the future, because when updating the test image - the digest will remain the same

Exactly. However, since we pin the digest, it is less likely to happen unless we intentionally change the digest.
https://github.com/aquasecurity/trivy-test-images/blob/bcbd1ca7b1a9d001b7f21f41310b8a17bc355dd2/copy-images.sh#L16

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 Used your logic and removed the save step c015645


- name: Download test images
if: steps.restore-test-images.outputs.cache-hit != 'true'
knqyf263 marked this conversation as resolved.
Show resolved Hide resolved
run: mage test:fixtureContainerImages

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this step anymore. It is transparently called.

mg.Deps(t.FixtureContainerImages)

And it's efficient as it skips images if it exists locally.

if exists(filePath) {
continue
}

Now, we call this check twice, which is just time-consuming (and API-consuming).

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, since the cache will still be saved at the end of the job!

Removed download steps in 0609e5d

- name: Save test images into cache
if: steps.restore-test-images.outputs.cache-hit != 'true'
uses: actions/cache@v4
with:
path: integration/testdata/fixtures/images
key: cache-test-images-2024-09-10 # trivy-test-images last update date

- name: Run integration tests
run: mage test:integration

Expand Down Expand Up @@ -122,6 +140,24 @@ jobs:
with:
aqua_version: v1.25.0

- name: Restore test images from cache
uses: actions/cache@v4
id: restore-test-images
with:
path: integration/testdata/fixtures/images
key: cache-test-images-2024-09-10 # trivy-test-images last update date

- name: Download test images
if: steps.restore-test-images.outputs.cache-hit != 'true'
run: mage test:fixtureContainerImages

- name: Save test images into cache
if: steps.restore-test-images.outputs.cache-hit != 'true'
uses: actions/cache@v4
with:
path: integration/testdata/fixtures/images
key: cache-test-images-2024-09-10 # trivy-test-images last update date

- name: Run module integration tests
shell: bash
run: |
Expand All @@ -142,6 +178,25 @@ jobs:
uses: aquaproj/aqua-installer@v3.0.1
with:
aqua_version: v1.25.0

- name: Restore test VM images from cache
uses: actions/cache@v4
id: restore-test-vm-images
with:
path: integration/testdata/fixtures/vm-images
key: cache-test-vm-images-2023-06-18 # trivy-test-vm-images last update date

- name: Download test VM images
if: steps.restore-test-vm-images.outputs.cache-hit != 'true'
run: mage test:fixtureVMImages

- name: Save test VM images into cache
if: steps.restore-test-vm-images.outputs.cache-hit != 'true'
uses: actions/cache@v4
with:
path: integration/testdata/fixtures/vm-images
key: cache-test-vm-images-2023-06-18 # trivy-test-vm-images last update date

- name: Run vm integration tests
run: |
mage test:vm
Expand Down
Loading