Skip to content

Further improve image caching for Breeze#22625

Merged
potiuk merged 1 commit intoapache:mainfrom
potiuk:even-better-image-caching
Mar 31, 2022
Merged

Further improve image caching for Breeze#22625
potiuk merged 1 commit intoapache:mainfrom
potiuk:even-better-image-caching

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented Mar 30, 2022

In order to allow "big" rebuilds we remove the image tag
before rebuilding, to make sure that the remote and not local image
is used as source of cache. This is consequence of using inlined
cache.

Files are added to the Python Base image image - the base
layers will be shared with other stages but we will make sure
that the script stages are different for different platforms.

Also - when there is no image at all we fail pre-commit. This
should handle the situation when we tried to build the image and
stopped it in-between.

Hadolint released a new version of their checker - with support
for the new Dockerfile buildkit features and native support for
ARM so we are enabling it back.

See hadolint/hadolint#803

Finally we still need some files that we cannot inline, because
they are only needed for source build and they are too long
to inline (yarn.lock for example). In order to keep the cache
working for all umasks we need to bring back group fixing.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:dev-tools area:production-image Production image improvements and fixes labels Mar 30, 2022
@potiuk potiuk requested a review from ephraimbuddy March 30, 2022 16:22
@potiuk potiuk force-pushed the even-better-image-caching branch from 96a2f88 to 3e0b95e Compare March 30, 2022 16:23
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Mar 30, 2022

This will nicely handle moving "to" the new cache. The "inline" image cache should always be used from the remote image in ghcr.io to make sure latest version is used. Therefore we should remove the local image before the build. This is equivalent to "pull and force" but it has the nice property that buildx uses both caches for building so:

  1. If you built the image locally and the cache is "better" -it will be used to quickly rebuild the image
  2. But if the image changed significantly, the remote cache will be used

@potiuk potiuk force-pushed the even-better-image-caching branch 2 times, most recently from f3f9820 to 043b2ce Compare March 30, 2022 19:15
@potiuk potiuk requested review from kaxil and malthe March 30, 2022 19:15
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Mar 30, 2022

I hope that one will make it finally solved with multi-staging and multi-platform builds. I've learned a TON about it and there are still some weird behaviours.

There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors.

Very, very true.

@potiuk potiuk force-pushed the even-better-image-caching branch 3 times, most recently from 9f83b28 to 020a490 Compare March 30, 2022 22:45
@potiuk potiuk changed the title Further improves image caching for Breeze Further improve image caching for Breeze Mar 30, 2022
@potiuk potiuk force-pushed the even-better-image-caching branch 3 times, most recently from 8df1640 to eede348 Compare March 31, 2022 09:12
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Mar 31, 2022

Hello! I would really love that one to merge :). It also includes re-enabling hadolint which also brought ARM support last night (and already supports the new dockerfile:1.4 features that we use.

@github-actions
Copy link
Copy Markdown

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 31, 2022
@potiuk potiuk force-pushed the even-better-image-caching branch from eede348 to f7a1380 Compare March 31, 2022 09:48
In order to allow "big" rebuilds we remove the image tag
before rebuilding, to make sure that the remote and not local image
is used as source of cache. This is consequence of using inlined
cache.

Files are added to the Python Base image image - the base
layers will be shared with other stages but we will make sure
that the script stages are different for different platforms.

Also - when there is no image at all we fail pre-commit. This
should handle the situation when we tried to build the image and
stopped it in-between.

Hadolint released a new version of their checker - with support
for the new Dockerfile buildkit features and native support for
ARM so we are enabling it back.

See hadolint/hadolint#803

Finally we still need some files that we cannot inline, because
they are only needed for source build and they are too long
to inline (yarn.lock for example). In order to keep the cache
working for all umasks we need to bring back group fixing.
@potiuk potiuk force-pushed the even-better-image-caching branch from f7a1380 to 84dec5a Compare March 31, 2022 10:24
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Mar 31, 2022

Random failure. Merging :)

@potiuk potiuk merged commit 25b62a7 into apache:main Mar 31, 2022
@potiuk potiuk deleted the even-better-image-caching branch March 31, 2022 11:39
@potiuk potiuk restored the even-better-image-caching branch April 26, 2022 20:49
@potiuk potiuk deleted the even-better-image-caching branch July 29, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools area:production-image Production image improvements and fixes full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants