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.

Sign up for GitHub

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

Jump to bottom

fix(workflows): Disable Go cache for non-test jobs in AWS workflow #9656

Closed
wants to merge 3 commits into from
Closed

fix(workflows): Disable Go cache for non-test jobs in AWS workflow #9656

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2badfe
use caches for AWS source plugin
candiduslynx Apr 4, 2023
008bcc3
cache only from test
candiduslynx Apr 4, 2023
edf89e6
Merge branch 'main' into feat/cache/aws
candiduslynx Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 50 additions & 15 deletions .github/workflows/source_aws.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,26 @@ jobs:
- uses: actions/checkout@v3
with:
fetch-depth: 2
- name: Set up Go 1.x
- name: Set up Go
id: set-up-go
uses: actions/setup-go@v3
with:
go-version-file: plugins/source/aws/go.mod
cache: true
cache-dependency-path: plugins/source/aws/go.sum
- name: Restore build cache
uses: actions/cache/restore@v3
with:
path: |
~/.cache/go-build
erezrokah marked this conversation as resolved.
Show resolved Hide resolved
~/Library/Caches/go-build
~\AppData\Local\go-build
key: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-${{ hashFiles('plugins/source/aws/go.sum') }}
# no direct cache hit, however, even a partial hit might be beneficial
restore-keys: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-
- name: Get dependencies
run: go get -t -d ./...
# Lint should run after deps, as the dependencies might introduce deprecation to used functions
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
Expand All @@ -61,8 +75,6 @@ jobs:
args: "--config ../../.golangci.yml"
skip-pkg-cache: true
skip-build-cache: true
- name: Get dependencies
run: go get -t -d ./...
- name: gen
if: github.event_name == 'pull_request'
run: make gen
Expand All @@ -88,6 +100,15 @@ jobs:
- name: Run sync
if: startsWith(github.head_ref, 'release-please--branches--main--components')
run: cloudquery sync test/sanity.yml --log-console
- name: Save build cache
if: github.ref == 'refs/heads/main'
erezrokah marked this conversation as resolved.
Show resolved Hide resolved
uses: actions/cache/save@v3
with:
path: |
~/.cache/go-build
~/Library/Caches/go-build
~\AppData\Local\go-build
key: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-${{ hashFiles('plugins/source/aws/go.sum') }}
validate-release:
timeout-minutes: 60
needs: [resolve-runner]
Expand All @@ -98,20 +119,25 @@ jobs:
- name: Checkout
if: startsWith(github.head_ref, 'release-please--branches--main--components') || github.event_name == 'push'
uses: actions/checkout@v3
- uses: actions/cache@v3
if: startsWith(github.head_ref, 'release-please--branches--main--components') || github.event_name == 'push'
with:
path: |
~/.cache/go-build
~/go/pkg/mod
key: ${{ runner.os }}-go-1.19.5-release-cache-${{ hashFiles('plugins/source/aws/go.sum') }}
restore-keys: |
${{ runner.os }}-go-1.19.5-release-cache-plugins-source-aws
- name: Set up Go
if: startsWith(github.head_ref, 'release-please--branches--main--components') || github.event_name == 'push'
id: set-up-go
uses: actions/setup-go@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uses: actions/setup-go@v3
uses: erezrokah/setup-go@feat/add_cache_prefix
with:
go-version-file: plugins/source/aws/go.mod
cache-key-prefix: test-cache-

And for policies we can use cache-key-prefix: policies-cache-.

Waiting for actions/setup-go#366 to be reviewed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that we might be getting into another issue with this solution:

  • the cache limit is 10 GiB
  • each Go cache is about 100-300 MiB, let's use 150 MiB in the calculations to be on the conservative side (300 MiB likely comes from the huge plugins like AWS or Azure)
  • that means we can store something like 50 caches to be used effectively before they start being evicted
  • we already have 38 source plugins & 21 destination plugins (59 in total, add CLI to it to have 60)
  • If we don't store a single cache but, instead, use 3 different caches, we effectively will store 150 (test) + 100 + 100 (2 x build & mod) = 350 MiB for a plugin (= 21 GB for all plugins + CLI, so we will be evicting a lot of data)

Looking at this, I suggest storing a single cache for plugin, that will include:

  • mod cache
  • build & test cache

To make most of it, we'll have to store it after the testing (and, better yet, from the main branch for it to be properly reused), and to restore in other builds related to a plugin (or CLI).

Copy link
Contributor

@erezrokah erezrokah Apr 9, 2023 •
edited

Choose a reason for hiding this comment

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

I suggest we first fix the bug in our CI, and do any cache optimization later on. Once we fix the bug we should be able so see if we're hitting any cache limits issues, the should be able to do benchmarks and see if that can be improved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the cache limits we can take a look here: https://github.com/cloudquery/cloudquery/actions/caches

I'll be updating the workflow files for the heavy sources (aws, azure, gcp, k8s) to work with go cache properly, we can discuss the different approach separately.

Copy link
Contributor

@erezrokah erezrokah Apr 9, 2023 •
edited

Choose a reason for hiding this comment

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

we already have 38 source plugins & 21 destination plugins (59 in total, add CLI to it to have 60)

We only need this fix for 4 plugins, the ones that have policy tests

with:
go-version-file: plugins/source/aws/go.mod
cache: true
cache-dependency-path: plugins/source/aws/go.sum
- name: Restore build cache
if: startsWith(github.head_ref, 'release-please--branches--main--components') || github.event_name == 'push'
uses: actions/cache/restore@v3
with:
path: |
~/.cache/go-build
~/Library/Caches/go-build
~\AppData\Local\go-build
erezrokah marked this conversation as resolved.
Show resolved Hide resolved
key: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-${{ hashFiles('plugins/source/aws/go.sum') }}
erezrokah marked this conversation as resolved.
Show resolved Hide resolved
# no direct cache hit, however, even a partial hit might be beneficial
restore-keys: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-
- name: Install GoReleaser
if: startsWith(github.head_ref, 'release-please--branches--main--components') || github.event_name == 'push'
uses: goreleaser/goreleaser-action@v3
Expand Down Expand Up @@ -149,12 +175,23 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Set up Go 1.x
- name: Set up Go
id: set-up-go
uses: actions/setup-go@v3
with:
go-version-file: plugins/source/aws/go.mod
cache: true
cache-dependency-path: plugins/source/aws/go.sum
- name: Restore build cache
uses: actions/cache/restore@v3
with:
path: |
~/.cache/go-build
~/Library/Caches/go-build
~\AppData\Local\go-build
key: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-${{ hashFiles('plugins/source/aws/go.sum') }}
# no direct cache hit, however, even a partial hit might be beneficial
restore-keys: source-aws-${{ runner.os }}-go-${{ steps.set-up-go.outputs.go-version }}-
- name: Build
run: go build .
- name: Setup CloudQuery
Expand All @@ -169,5 +206,3 @@ jobs:
run: cd policies && psql -h localhost -p 5432 -U postgres -d postgres -w -f ./policy.sql
env:
PGPASSWORD: pass