Optimize dockerfiles for local rebuilds #20238

potiuk · 2021-12-12T20:01:17Z

This is one of the last final refactorings of the image before
it is eligible to become an "official image".

When you build dockerfiles locally for development the layer
invalidation could happen earlier than you wanted - some of the
variables (like COMMIT_SHA) were affecting the cache of Docker
in the way that they forced either invalidation of the pre-cached
packages installed or forced to recreate assets when they were
not touched.

Similarly when no webpack/yarn/packages/static are modified,
the node asset compilation should not happen. It makes
no sense to compile all the assets on docker rebuild when
none of the www files changed.

In case of CI build we can also separate node modules
preparation and asset compilation, because node modules
should remain in the image anyway for incremental changes.

Fixes: #20259

This PR improves the experience of iterating over docker image
building by decreasing unnecesary layer invalidations.

^ 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.

potiuk · 2021-12-13T15:57:53Z

This is one of the final optimizations/refactorings and cleanups of the image before I will submit it to become an official image. Some last optimizations/clarifications (cleaning up the stderr output etc. )

If you think it is to big I can attempt to split it into smaller pieces, but there are a number of changes in there that invalidate current layers of Airflow images, making the cache less effective and merging it in one go would only require image refresh once.

Since the image is going to be "official" I had to make sure that all the potential "errors" are either gone or are explained. There re many requirements to fulfill to become an "official image" - and I think with that one, we are getting closer to having them all fulfilled (https://github.com/docker-library/official-images).

One thing that I had to do however is to add this reassuring message that we know what we are doing by using root and not using virtualenv. I tried to solve the problem in PIP or even PEP 668 level but I failed, so I had to revert to this reassuring message:

Looking forward to reviews!

potiuk · 2021-12-14T18:20:38Z

Looking forward to reviews on that one- it should be helpful in finalizing AIP-26 (finally)

potiuk · 2021-12-16T23:27:21Z

Anyone :) ?

raphaelauv · 2021-12-21T13:12:03Z

python2 is still in the DockerFile , maybe the opportunity to remove it ?

potiuk · 2021-12-21T13:29:40Z

python2 is still in the DockerFile , maybe the opportunity to remove it ?

It's there for a reason. Python2 + Python Virtualenv is our offer to the users who still (?) have a need to run Python2 code.

It's part of the migration process we have:

https://airflow.apache.org/docs/apache-airflow/stable/upgrading-from-1-10/index.html#step-1-switch-to-python-3

We have a lot of enterprise users who are (hopefully) going to move away soon from it. And since it does not cost us much, we have Python 2 added to our image and even tests in our CI that test if PythonVirtualenv work for Python2.

I think removal of Python 2 from our tests/image should be done after voting that we want to drop it.

potiuk · 2021-12-21T13:44:13Z

I started PROPOSAL thread on the devlist: https://lists.apache.org/thread/rjyqw3cwsh4vgg6jycsbr1jr0slnych3 @raphaelauv . I think 2 years of EOL anniversary is a good time to put the final nail in the coffin for it.

potiuk · 2021-12-22T00:25:46Z

I updated the images to remove Python 2.

I also got rid of the warning from PIP in a different way. I complicated the Base image to add airflow user there (even if it is not needed), Just to make sure there are no warnings whe I pass it to the official image verification.

potiuk · 2021-12-22T00:27:49Z

I would love some reviews - happy to split off some things (though it would be rather difficult to split).

But it would be great to get this one in to (almost) end the prod image story.

The "buildkit" is much more modern docker build mechanism and supports multiarchitecture builds which makes it suitable for our future ARM support, it also has nicer UI and much more sophisticated caching mechanisms as well as supports better multi-segment builds. BuildKit has been promoted to official for quite a while and it is rather stable now. Also we can now install BuildKit Plugin to docker that add capabilities of building and managin cache using dedicated builders (previously BuildKit cache was managed using rather complex external tools). This gives us an opportunity to vastly simplify our build scripts, because it has now much more robust caching mechanism than the old docker build (which forced us to pull images before using them as cache). We had a lot of complexity involved in efficient caching but with BuildKit all that can be vastly simplified and we can get rid of: * keeping base python images in our registry * keeping build segments for prod image in our registry * keeping manifest images in our registry * deciding when to pull or pull&build image (not needed now, we can always build image with --cache-from and buildkit will pull cached layers as needed * building the image when performing pre-commit (rather than that we simply encourage users to rebuild the image via breeze command) * pulling the images before building * separate 'build' cache kept in our registry (not needed any more as buildkit allows to keep cache for all segments of multi-segmented build in a single cache * the nice animated tty UI of buildkit eliminates the need of manual spinner * and a number of other complexities. Depends on apache#20238

The "buildkit" is much more modern docker build mechanism and supports multiarchitecture builds which makes it suitable for our future ARM support, it also has nicer UI and much more sophisticated caching mechanisms as well as supports better multi-segment builds. BuildKit has been promoted to official for quite a while and it is rather stable now. Also we can now install BuildKit Plugin to docker that add capabilities of building and managin cache using dedicated builders (previously BuildKit cache was managed using rather complex external tools). This gives us an opportunity to vastly simplify our build scripts, because it has now much more robust caching mechanism than the old docker build (which forced us to pull images before using them as cache). We had a lot of complexity involved in efficient caching but with BuildKit all that can be vastly simplified and we can get rid of: * keeping base python images in our registry * keeping build segments for prod image in our registry * keeping manifest images in our registry * deciding when to pull or pull&build image (not needed now, we can always build image with --cache-from and buildkit will pull cached layers as needed * building the image when performing pre-commit (rather than that we simply encourage users to rebuild the image via breeze command) * pulling the images before building * separate 'build' cache kept in our registry (not needed any more as buildkit allows to keep cache for all segments of multi-segmented build in a single cache * the nice animated tty UI of buildkit eliminates the need of manual spinner * and a number of other complexities. Depends on #20238

* remove PIP_INSTALL_USER variable * upgrade PIP to 21.3.1 * remove AIRFLOW_INSTALL_USER_FLAG as it is not needed * remove spurious usage of --upgrade flag for PIP * add better diagnostics during the build for PIP location and version Separated out from #20238 (cherry picked from commit 0635b97)

There was some "junk" output generated by the scripts that are used in Airflow image building. The junk has been cleaned up so that no unnecessary warnings are generated. This change includes: * making sure that when everything is fine, there are no warnnings generated by PROD docker build proces * making sure that when CI image is build the only remaining warning is "Using root" - this warning cannot be silenced pypa/pip#10556 and instead in CI build we explain in green that this is invalid warning * the "scripted" steps of docker build have nicely blue headers that visually separate steps of building the iamge and give more information on what's going on * the current way of printing ouput will play very nicely with BUILDKIT UI where Blue color indicates progress in building Separated out from #20238 (cherry picked from commit 4d33ebf)

When you build dockerfiles locally for development the layer invalidation could happen earlier than you wanted - some of the variables (like COMMIT_SHA) were affecting the cache of Docker in the way that they forced either invalidation of the pre-cached packages installed or forced to recreate assets when they were not touched. Similarly when no webpack/yarn/packages/static are modified, the node asset compilation should not happen. It makes no sense to compile all the assets on docker rebuild when none of the www files changed. In case of CI build we can also separate node modules preparation and asset compilation, because node modules should remain in the image anyway for incremental changes. Fixes: #20259 This PR improves the experience of iterating over docker image building by decreasing unnecesary layer invalidations. (cherry picked from commit 4620770)

The "buildkit" is much more modern docker build mechanism and supports multiarchitecture builds which makes it suitable for our future ARM support, it also has nicer UI and much more sophisticated caching mechanisms as well as supports better multi-segment builds. BuildKit has been promoted to official for quite a while and it is rather stable now. Also we can now install BuildKit Plugin to docker that add capabilities of building and managin cache using dedicated builders (previously BuildKit cache was managed using rather complex external tools). This gives us an opportunity to vastly simplify our build scripts, because it has now much more robust caching mechanism than the old docker build (which forced us to pull images before using them as cache). We had a lot of complexity involved in efficient caching but with BuildKit all that can be vastly simplified and we can get rid of: * keeping base python images in our registry * keeping build segments for prod image in our registry * keeping manifest images in our registry * deciding when to pull or pull&build image (not needed now, we can always build image with --cache-from and buildkit will pull cached layers as needed * building the image when performing pre-commit (rather than that we simply encourage users to rebuild the image via breeze command) * pulling the images before building * separate 'build' cache kept in our registry (not needed any more as buildkit allows to keep cache for all segments of multi-segmented build in a single cache * the nice animated tty UI of buildkit eliminates the need of manual spinner * and a number of other complexities. Depends on #20238 (cherry picked from commit ad28f69)

* remove PIP_INSTALL_USER variable * upgrade PIP to 21.3.1 * remove AIRFLOW_INSTALL_USER_FLAG as it is not needed * remove spurious usage of --upgrade flag for PIP * add better diagnostics during the build for PIP location and version Separated out from #20238 (cherry picked from commit 0635b97)

There was some "junk" output generated by the scripts that are used in Airflow image building. The junk has been cleaned up so that no unnecessary warnings are generated. This change includes: * making sure that when everything is fine, there are no warnnings generated by PROD docker build proces * making sure that when CI image is build the only remaining warning is "Using root" - this warning cannot be silenced pypa/pip#10556 and instead in CI build we explain in green that this is invalid warning * the "scripted" steps of docker build have nicely blue headers that visually separate steps of building the iamge and give more information on what's going on * the current way of printing ouput will play very nicely with BUILDKIT UI where Blue color indicates progress in building Separated out from #20238 (cherry picked from commit 4d33ebf)

When you build dockerfiles locally for development the layer invalidation could happen earlier than you wanted - some of the variables (like COMMIT_SHA) were affecting the cache of Docker in the way that they forced either invalidation of the pre-cached packages installed or forced to recreate assets when they were not touched. Similarly when no webpack/yarn/packages/static are modified, the node asset compilation should not happen. It makes no sense to compile all the assets on docker rebuild when none of the www files changed. In case of CI build we can also separate node modules preparation and asset compilation, because node modules should remain in the image anyway for incremental changes. Fixes: #20259 This PR improves the experience of iterating over docker image building by decreasing unnecesary layer invalidations. (cherry picked from commit 4620770)

The "buildkit" is much more modern docker build mechanism and supports multiarchitecture builds which makes it suitable for our future ARM support, it also has nicer UI and much more sophisticated caching mechanisms as well as supports better multi-segment builds. BuildKit has been promoted to official for quite a while and it is rather stable now. Also we can now install BuildKit Plugin to docker that add capabilities of building and managin cache using dedicated builders (previously BuildKit cache was managed using rather complex external tools). This gives us an opportunity to vastly simplify our build scripts, because it has now much more robust caching mechanism than the old docker build (which forced us to pull images before using them as cache). We had a lot of complexity involved in efficient caching but with BuildKit all that can be vastly simplified and we can get rid of: * keeping base python images in our registry * keeping build segments for prod image in our registry * keeping manifest images in our registry * deciding when to pull or pull&build image (not needed now, we can always build image with --cache-from and buildkit will pull cached layers as needed * building the image when performing pre-commit (rather than that we simply encourage users to rebuild the image via breeze command) * pulling the images before building * separate 'build' cache kept in our registry (not needed any more as buildkit allows to keep cache for all segments of multi-segmented build in a single cache * the nice animated tty UI of buildkit eliminates the need of manual spinner * and a number of other complexities. Depends on #20238 (cherry picked from commit ad28f69)

potiuk requested review from ashb, jedcunningham, kaxil and mik-laj as code owners December 12, 2021 20:01

boring-cyborg bot added area:dev-tools area:production-image Production image improvements and fixes kind:documentation labels Dec 12, 2021

potiuk requested a review from uranusjr December 12, 2021 20:01

potiuk mentioned this pull request Dec 12, 2021

Swtich both PROD and CI images to use virtual environments #19189

Closed

potiuk force-pushed the optimizes-dockerfile branch 4 times, most recently from 5c29de2 to 7a71228 Compare December 13, 2021 15:45

potiuk mentioned this pull request Dec 13, 2021

Add an opt-out for the “running as root” warning pypa/pip#10556

Closed

1 task

potiuk force-pushed the optimizes-dockerfile branch from 7a71228 to b55b3a2 Compare December 13, 2021 16:08

potiuk closed this Dec 13, 2021

potiuk reopened this Dec 13, 2021

potiuk force-pushed the optimizes-dockerfile branch from b55b3a2 to abcd0c8 Compare December 20, 2021 16:55

potiuk force-pushed the optimizes-dockerfile branch 2 times, most recently from 36df2da to 74bd750 Compare December 22, 2021 00:24

potiuk force-pushed the optimizes-dockerfile branch 2 times, most recently from 611bf18 to 67a594c Compare December 22, 2021 00:31

potiuk added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 22, 2022

potiuk added this to the Airflow 2.2.4 milestone Jan 22, 2022

potiuk restored the optimizes-dockerfile branch April 26, 2022 20:48

potiuk deleted the optimizes-dockerfile branch July 29, 2022 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Optimize dockerfiles for local rebuilds #20238

Optimize dockerfiles for local rebuilds #20238

Uh oh!

potiuk commented Dec 12, 2021 •

edited

Loading

Uh oh!

potiuk commented Dec 13, 2021 •

edited

Loading

Uh oh!

potiuk commented Dec 14, 2021

Uh oh!

potiuk commented Dec 16, 2021

Uh oh!

raphaelauv commented Dec 21, 2021

Uh oh!

potiuk commented Dec 21, 2021 •

edited

Loading

Uh oh!

potiuk commented Dec 21, 2021

Uh oh!

potiuk commented Dec 22, 2021

Uh oh!

potiuk commented Dec 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Optimize dockerfiles for local rebuilds #20238

Optimize dockerfiles for local rebuilds #20238

Uh oh!

Conversation

potiuk commented Dec 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Dec 14, 2021

Uh oh!

potiuk commented Dec 16, 2021

Uh oh!

raphaelauv commented Dec 21, 2021

Uh oh!

potiuk commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Dec 21, 2021

Uh oh!

potiuk commented Dec 22, 2021

Uh oh!

potiuk commented Dec 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

potiuk commented Dec 12, 2021 •

edited

Loading

potiuk commented Dec 13, 2021 •

edited

Loading

potiuk commented Dec 21, 2021 •

edited

Loading