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: Use buildkit caching when building Dockerfile.ci #3428

Merged
merged 2 commits into from
Apr 6, 2021

Conversation

feelepxyz
Copy link
Contributor

@feelepxyz feelepxyz commented Apr 1, 2021

Leverage Docker's buildkit caching when building Dockerfile.ci.

Using docker volumes instead of copying all the files over to the ci image as the buildkit cache was previously hanging when building the ci image.

The caching change was previously reverted in this commit.

@feelepxyz feelepxyz requested a review from a team as a code owner April 1, 2021 13:35
@jurre
Copy link
Member

jurre commented Apr 1, 2021

I think this needs to be rebased on main before it'll work?

@feelepxyz feelepxyz force-pushed the feelepxyz/add-buildkit-caching-to-ci-image branch from 496b814 to a35683c Compare April 1, 2021 14:08
@feelepxyz
Copy link
Contributor Author

This can be simplified once #3430 lands

if [ -n "${{ secrets.DOCKER_USERNAME }}" ] && [ -n "${{ secrets.DOCKER_PASSWORD }}" ]; then
echo ${{ secrets.DOCKER_PASSWORD }} | docker login -u ${{ secrets.DOCKER_USERNAME }} --password-stdin
if [[ -n "${{ secrets.GHCR_PAT }}" ]]; then
echo "${{ secrets.GHCR_PAT }}" | docker login ghcr.io -u ${{ github.actor }} --password-stdin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to use the GITHUB_TOKEN here but doesn't seem to work even after giving the repo write access to the org package

"cd /home/dependabot/dependabot-core/bundler/helpers/v2 && BUNDLER_VERSION=2 bundle install && BUNDLER_VERSION=2 bundle exec rspec spec"
- name: Run ${{ matrix.suite.name }} tests
run: |
docker run --env "CI=true" --env "DEPENDABOT_TEST_ACCESS_TOKEN=$DEPENDABOT_TEST_ACCESS_TOKEN" --env "SUITE_NAME=${{ matrix.suite.name }}" --rm "$CORE_CI_IMAGE" bash -c \
docker run \
-v "$(pwd)/.rubocop.yml:$CODE_DIR/.rubocop.yml" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be nice to share this between the dev-shell setup

Copy link
Contributor Author

@feelepxyz feelepxyz Apr 6, 2021

Choose a reason for hiding this comment

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

Going to try and clean this up in a follow up, with a shared docker-shell script we can use from here and docker-dev-shell

@feelepxyz feelepxyz changed the title CI: Use buildkit caching for Dockerfile.ci CI: Remove Dockerfile.ci and use ghcr.io for ci images Apr 1, 2021
@feelepxyz feelepxyz force-pushed the feelepxyz/add-buildkit-caching-to-ci-image branch from 6300b4d to e164687 Compare April 6, 2021 09:58
Comment on lines 3 to 5
RUN pyenv install -s 3.6.13
RUN pyenv install -s 3.7.1
RUN pyenv install -s 3.7.10
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this PR, but I noticed that these still aren't being cached currently. Do you have any idea why that could be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, are they being installed again in the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First hunch was some permission issue but I think these, as the tests, are running as root so should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

No, they are being installed in the docker-ci build step in the action actually, when IMO all these layers should be cached?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wait you mean on this PRs build? I busted all the docker images used here so everything needs to rebuild.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah I see, I wonder if the change in this PR to use the inline buildkit cache would help with this?

Copy link
Member

Choose a reason for hiding this comment

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

Curious to find out! It might 🤞

@feelepxyz feelepxyz changed the title CI: Remove Dockerfile.ci and use ghcr.io for ci images CI: Simplify Dockerfile.ci and use ghcr.io for ci images Apr 6, 2021
@feelepxyz feelepxyz changed the title CI: Simplify Dockerfile.ci and use ghcr.io for ci images CI: Simplify Dockerfile.ci Apr 6, 2021
@feelepxyz feelepxyz changed the title CI: Simplify Dockerfile.ci CI: Use buildkit caching when building Dockerfile.ci Apr 6, 2021
@feelepxyz feelepxyz requested a review from jurre April 6, 2021 11:23
@@ -44,6 +44,7 @@ jobs:
echo "BASE_IMAGE=ubuntu:18.04" >> $GITHUB_ENV
echo "CORE_IMAGE=dependabot/dependabot-core" >> $GITHUB_ENV
echo "CORE_CI_IMAGE=dependabot/dependabot-core-ci" >> $GITHUB_ENV
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went back to using the docker registry as it seemed like ghcr doesn't do well with the inline caching, possibly not storing the cache properly so was re-building every time

run: |
docker push "dependabot/dependabot-core:latest"
- name: Push image to packages (tagged)
docker push "$CORE_IMAGE:latest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to always push the latest tag, can't see a reason where we don't want to push latest when tagging a release

Use ghcr.io for ci image builds and leve leverage Docker's buildkit
caching when building it.

Using docker volumes instead of copying all the files over to the ci image.

The caching change was previously [reverted in this
commit](a89aca9#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f).

I believe this will work with [https://github.com/dependabot/dependabot-core/pull/3426](this CI change) as I have vague recalls that
the failures happened because we where running all the bundle installs in `Dockerfile.ci`.
@feelepxyz feelepxyz force-pushed the feelepxyz/add-buildkit-caching-to-ci-image branch from 09bad5c to 4aa0b92 Compare April 6, 2021 11:31
Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

I think the changes look good and so far the logs here look promising 👍

@feelepxyz feelepxyz merged commit 2e5dd94 into main Apr 6, 2021
@feelepxyz feelepxyz deleted the feelepxyz/add-buildkit-caching-to-ci-image branch April 6, 2021 12:50
@thepwagner thepwagner mentioned this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants