build,cache: support remote cache sources ( via `--cache-to`, `--cache-from` ) and introduce `cacheKey/layerKey` #4118

flouthoc · 2022-07-18T06:17:51Z

Following commit

Initiates cacheKey or layerKey for intermediate images generated
for layers. ( only used when working with remote sources )
Allows end users to upload cached layers with cacheKey to remote
sources using --cache-to. --cache-to is a optional flag to be used
with buildah build which publishes cached layers to remote sources.
Allows end users to use cached layers from remote sources with
--cache-from. --cache-from is a optional flag to be used with
buildah build and it pulls cached layers from remote sources in a step
by step manner only if is a valid cache hit.

Example

Populate cache source or use cached layers if already present

buildah build -t test --layers --cache-to registry/myrepo/cache --cache-from registry/myrepo/cache .

Future Improvements:

Also support pushing cache to other storage units rather than conventional container registry like object storage ( AWS S3, Redis, Minio etc )
cacheKey or layerKey model is only being used when working with
remote sources however local cache lookup can be also optimized if its
is altered to use cacheKey model instead of iterating through all the
images in local storage. As discussed here: LayerKey model for the local cache #3406

References:

Feature is quite similar to kaniko's --cache-repo: https://github.com/GoogleContainerTools/kaniko#--cache-repo

Closes: #620

Does this PR introduce a user-facing change?

build: adds supports for --cache-to i.e alllow pushing cache layers to remote source
build: adds support for --cache-from i.e allow using cache layers from remote sources

flouthoc · 2022-07-18T07:46:11Z

@nalind PTAL

imagebuildah/stage_executor.go

nalind

Looks generally good.
We need to be very precise in use of terminology in documentation. People already get confused about the difference between containers and images when they're starting out with containers, and we don't want to muddy things any further.
Question: when we have a local cache hit, why don't we try to push to a --cache-to location? The image which counted as a cache hit may not have come from there.

docs/buildah-build.1.md

imagebuildah/stage_executor.go

flouthoc · 2022-07-19T06:33:20Z

Question: when we have a local cache hit, why don't we try to push to a --cache-to location?

@nalind My initial idea was to save resources by only pushing for steps which are just built and not which are already present in local storage. But upon thinking again I think it will be right to push everytime except for the case when intermediate image was just pulled for the step.

So amended source and test case to match:

If --cache-to is specified always push intermediate image expect for the case when intermediate is just pulled for the STEP which is currently being evaluated.

mtrmac · 2022-07-19T13:09:21Z

Also support pushing cache to other storage units rather than conventional container registry like object storage ( AWS S3, Redis, Minio etc )

What do you anticipate the API, and UI, to look like? Entirely new options, or somehow overloading the semantics of the existing input strings (and if so, how was it designed to avoid conflicts?)

mtrmac

I’m afraid this is a rather sloppy review, I didn’t quite take the time to fully understand the extensive logic dealing with various alternative possibilities.

Highlights:

Should the cache locations be c/image generic names at all, or strictly registry repositories?
Don’t use reportWriter output to make decisions.

imagebuildah/stage_executor.go

flouthoc · 2022-07-19T14:45:38Z

Also support pushing cache to other storage units rather than conventional container registry like object storage ( AWS S3, Redis, Minio etc )

What do you anticipate the API, and UI, to look like? Entirely new options, or somehow overloading the semantics of the existing input strings (and if so, how was it designed to avoid conflicts?)

@mtrmac I am not sure about actual implementation of this yet but I just wrote it cause it looked like a nice idea to be able to allow buildah to distribute cache to other storage backends like redis, s3 etc where is it more easier to control eviction of least used cache, scaling etc. At first instance i think implementation of new transports at c/image which can encapsulate image and put/get from redis, s3 etc but this is just an idea of now.

docs/buildah-build.1.md

flouthoc · 2022-07-20T08:28:28Z

Don’t use reportWriter output to make decisions.

New push is ensures we are not using this anymore. Still some comments are pending but I hope in another push we can get them resolved.

mtrmac · 2022-07-20T17:23:07Z

imagebuildah/stage_executor.go

+				// is ignored and will be automatically logged for --log-level debug
+				if id, err := s.pullCache(ctx, src); id != "" && err == nil {
+					logCachePulled(src)
+					cacheID, err = s.intermediateImageExists(ctx, node, addedContentSummary, s.stepRequiresLayer(step))


BTW it might be worthwhile to extract the “does $candidateImage match the current image and next step” logic from intermediateImageExists into a separate function, and here call it only with the just-pulled image.

intermediateImageExists seems to do several a few c/storage operations (at least the .Layer call is not cached), each with its own locking syscalls and related overhead, so doing an O(1) check instead of an O(existing images) check would be nice.

(Refactorings like this would be best done in separate commits, or maybe deferred to a separate PR.)

Will tackle this in a different PR.

imagebuildah/stage_executor.go

There is almost no reason for us to run through cache iteration twice just for `COPY` and `ADD` instruction where we can sure that it will always miss the first pass so optimize cache processing by making sure that we populate `contentSummary` in the right way so we always end up hitting cache in first pass. This should ensure * We always hit cache in the first pass and so second attempt should be made See discussion here: * containers#4118 (comment) * containers#4118 (comment) Signed-off-by: Aditya R <[email protected]>

docs/buildah-build.1.md

tests/bud.bats

imagebuildah/stage_executor.go

flouthoc · 2022-07-25T20:15:55Z

Pushed updated tests for ADD and COPY case.

Following commit * Initiates `cacheKey` or `layerKey` for intermediate images generated for layers. * Allows end users to upload cached layers with `cacheKey` to remote sources using `--cache-to`. `--cache-to` is a optional flag to be used with `buildah build` which publishes cached layers to remote sources. * Allows end users to use cached layers from `remote` sources with `--cache-from`. `--cache-from` is a optional flag to be used with `buildah build` and it pulls cached layers from remote sources in a step by step manner only if is a valid cache hit. Example * Populate cache source or use cached layers if already present ```bash buildah build -t test --layers --cache-to registry/myrepo/cache --cache-from registry/myrepo/cache . ``` Future: * `cacheKey` or `layerKey` model is only being used when working with remote sources however local cache lookup can be also optimized if its is altered to use `cacheKey` model instead of iterating through all the images in local storage. As discussed here References: * Feature is quite similar to `kaniko`'s `--cache-repo`: https://github.com/GoogleContainerTools/kaniko#--cache-repo Closes: issues#620 Signed-off-by: Aditya R <[email protected]>

openshift-ci · 2022-07-25T20:31:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, mtrmac, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [flouthoc,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mtrmac · 2022-07-25T20:32:21Z

/lgtm

Thanks!

rhatdan · 2022-07-25T20:34:01Z

/hold

TomSweeneyRedHat · 2022-07-25T22:02:14Z

Happy green test buttons @rhatdan. I'm not going to cancel the hold as I'm not sure if you were holding on tests, or holding until release with this one.

rhatdan · 2022-07-26T00:55:11Z

/hold cancel

mukulk2020 · 2023-01-02T04:44:20Z

@flouthoc : How can i install latest version of buildah in my centos machine so that i can use these parameters "--cache-to, --cache-from" along with buildah bud command . I checked the install.md (https://github.com/containers/buildah/blob/main/install.md) but this will install older version of buildah i.e. 1.11.xxx.

Please help me on this so that i can run below command to achieve cache .

Exam : buildah bud -t cache-test --layers --cache-to mukulk/cache-test --cache-from mukulk/cache-test .

flouthoc · 2023-01-02T07:43:46Z

@mukulk2020 Thanks for reaching out it seems package on centos has not been updated for a while, I think @lsm5 and @rhatdan can confirm about who maintains it and who can update it.

mukulk2020 · 2023-01-02T10:57:21Z

@lsm5 @rhatdan Please check and update the docs and code accordingly..

flouthoc · 2023-01-02T11:00:09Z

I don't think @lsm5 or @rhatdan actually maintains packages on centos but they can guide or tag the right person who can do it. :)

lsm5 · 2023-01-03T16:00:17Z

@mukulk2020 your screenshot tells me you're on CentOS 7. We're not doing updates for CentOS 7 anymore. Please consider switching to centos stream 8 / 9.

openshift-ci bot added the approved label Jul 18, 2022

flouthoc force-pushed the support-remote-cache branch 4 times, most recently from a839f96 to e956848 Compare July 18, 2022 06:43

flouthoc requested review from nalind and rhatdan July 18, 2022 07:46

rhatdan reviewed Jul 18, 2022

View reviewed changes

imagebuildah/stage_executor.go Outdated Show resolved Hide resolved

nalind requested changes Jul 18, 2022

View reviewed changes

rhatdan reviewed Jul 18, 2022

View reviewed changes

imagebuildah/stage_executor.go Outdated Show resolved Hide resolved

rhatdan approved these changes Jul 18, 2022

View reviewed changes

flouthoc force-pushed the support-remote-cache branch 2 times, most recently from 5d52fbf to 7408074 Compare July 19, 2022 06:13

flouthoc requested a review from nalind July 19, 2022 06:33

mtrmac reviewed Jul 19, 2022

View reviewed changes

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

nalind reviewed Jul 19, 2022

View reviewed changes

docs/buildah-build.1.md Outdated Show resolved Hide resolved

flouthoc force-pushed the support-remote-cache branch from 7408074 to 04ee66c Compare July 20, 2022 08:11

mtrmac reviewed Jul 20, 2022

View reviewed changes

flouthoc mentioned this pull request Jul 21, 2022

imagebuildah: optimize cache hits for COPY and ADD instructions #4128

Merged

mtrmac reviewed Jul 25, 2022

View reviewed changes

flouthoc force-pushed the support-remote-cache branch from 8220053 to bb62b23 Compare July 25, 2022 20:14

flouthoc requested a review from mtrmac July 25, 2022 20:15

flouthoc force-pushed the support-remote-cache branch from bb62b23 to 6f660bd Compare July 25, 2022 20:19

mtrmac approved these changes Jul 25, 2022

View reviewed changes

openshift-ci bot assigned mtrmac Jul 25, 2022

openshift-ci bot added the lgtm label Jul 25, 2022

openshift-ci bot added the do-not-merge/hold label Jul 25, 2022

openshift-ci bot removed the do-not-merge/hold label Jul 26, 2022

openshift-merge-robot merged commit d7d7804 into containers:main Jul 26, 2022

This was referenced Jul 26, 2022

buildah bud --cache-from functionality needed #620

Closed

Efficient building for CI with distributed cache and direct registry tagging #3382

Closed

manics mentioned this pull request Jul 26, 2022

podman --cache-from should work soon manics/repo2podman#30

Closed

flouthoc mentioned this pull request Oct 13, 2022

imagebuildah, cache-from: pull cache from remote repo after adding content summary #4326

Merged

flouthoc mentioned this pull request Nov 30, 2022

LayerKey model for the local cache #3406

Closed

StarpTech mentioned this pull request May 29, 2023

Allow to push finalized layers to the registry during build containers/podman#18642

Open

github-actions bot added the locked - please file new issue/PR label Sep 1, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build,cache: support remote cache sources ( via `--cache-to`, `--cache-from` ) and introduce `cacheKey/layerKey` #4118

build,cache: support remote cache sources ( via `--cache-to`, `--cache-from` ) and introduce `cacheKey/layerKey` #4118

flouthoc commented Jul 18, 2022 •

edited

Loading

flouthoc commented Jul 18, 2022

nalind left a comment

flouthoc commented Jul 19, 2022 •

edited

Loading

mtrmac commented Jul 19, 2022

mtrmac left a comment

flouthoc commented Jul 19, 2022 •

edited

Loading

flouthoc commented Jul 20, 2022

mtrmac Jul 20, 2022

mtrmac Jul 20, 2022

flouthoc Jul 25, 2022

flouthoc commented Jul 25, 2022

openshift-ci bot commented Jul 25, 2022

mtrmac commented Jul 25, 2022

rhatdan commented Jul 25, 2022

TomSweeneyRedHat commented Jul 25, 2022

rhatdan commented Jul 26, 2022

mukulk2020 commented Jan 2, 2023 •

edited

Loading

flouthoc commented Jan 2, 2023

mukulk2020 commented Jan 2, 2023

flouthoc commented Jan 2, 2023

lsm5 commented Jan 3, 2023

build,cache: support remote cache sources ( via --cache-to, --cache-from ) and introduce cacheKey/layerKey #4118

build,cache: support remote cache sources ( via --cache-to, --cache-from ) and introduce cacheKey/layerKey #4118

Conversation

flouthoc commented Jul 18, 2022 • edited Loading

Does this PR introduce a user-facing change?

flouthoc commented Jul 18, 2022

nalind left a comment

Choose a reason for hiding this comment

flouthoc commented Jul 19, 2022 • edited Loading

mtrmac commented Jul 19, 2022

mtrmac left a comment

Choose a reason for hiding this comment

flouthoc commented Jul 19, 2022 • edited Loading

flouthoc commented Jul 20, 2022

mtrmac Jul 20, 2022

Choose a reason for hiding this comment

mtrmac Jul 20, 2022

Choose a reason for hiding this comment

flouthoc Jul 25, 2022

Choose a reason for hiding this comment

flouthoc commented Jul 25, 2022

openshift-ci bot commented Jul 25, 2022

mtrmac commented Jul 25, 2022

rhatdan commented Jul 25, 2022

TomSweeneyRedHat commented Jul 25, 2022

rhatdan commented Jul 26, 2022

mukulk2020 commented Jan 2, 2023 • edited Loading

flouthoc commented Jan 2, 2023

mukulk2020 commented Jan 2, 2023

flouthoc commented Jan 2, 2023

lsm5 commented Jan 3, 2023

build,cache: support remote cache sources ( via `--cache-to`, `--cache-from` ) and introduce `cacheKey/layerKey` #4118

build,cache: support remote cache sources ( via `--cache-to`, `--cache-from` ) and introduce `cacheKey/layerKey` #4118

flouthoc commented Jul 18, 2022 •

edited

Loading

flouthoc commented Jul 19, 2022 •

edited

Loading

flouthoc commented Jul 19, 2022 •

edited

Loading

mukulk2020 commented Jan 2, 2023 •

edited

Loading