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

Use content digests in ADD/COPY history entries #1792

Closed
wants to merge 2 commits into from

Conversation

nalind
Copy link
Member

@nalind nalind commented Aug 14, 2019

Use digests of the added content in history entries that we create for ADD and COPY instructions, tightening up cache checking just a little bit more. This should fix #1780.

Add a DryRun flag to AddAndCopyOptions, so that we can "copy" content to
digest it.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the hashes-in-history branch 2 times, most recently from ad3ede0 to b377385 Compare August 14, 2019 20:32
Use digests of the added content in history entries that we create for
ADD and COPY instructions, tightening up cache checking just a little
bit more.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind force-pushed the hashes-in-history branch from b377385 to 0f44e6b Compare August 14, 2019 23:36
@rhatdan
Copy link
Member

rhatdan commented Aug 15, 2019

LGTM

@rhatdan
Copy link
Member

rhatdan commented Aug 15, 2019

@TomSweeneyRedHat
Copy link
Member

LGTM and passes baseline tests

@TomSweeneyRedHat
Copy link
Member

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 0f44e6b has been approved by TomSweeneyRedHat

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 0f44e6b with merge ebf6f51...

rh-atomic-bot pushed a commit that referenced this pull request Aug 16, 2019
Use digests of the added content in history entries that we create for
ADD and COPY instructions, tightening up cache checking just a little
bit more.

Signed-off-by: Nalin Dahyabhai <[email protected]>

Closes: #1792
Approved by: TomSweeneyRedHat
@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr, status-travis
Approved by: TomSweeneyRedHat
Pushing ebf6f51 to master...

@nalind nalind deleted the hashes-in-history branch August 19, 2019 13:54
TomSweeneyRedHat added a commit to TomSweeneyRedHat/buildah that referenced this pull request Oct 16, 2019
Add back the file checking mechanisms that were removed as part of the
reworking of the cache in containers#1792.  If we have a 'COPY mytar.tar.xz' command
in the Containerfile and the tar file was regenerated after an intial build,
secondary 'buildah bud' commands would use the cache from the first build
for the tar file instead of regenerating it.

The historyMatches() function (https://github.com/containers/buildah/blob/master/imagebuildah/executor.go#L287)
does not seem to pick up this type of change and I wasn't able to quickly
rework that to do so.  We may want to revert back to this check for now,
then tweak the historyMatches to work properly in a follow up PR.

Regardless, I've added a test as part of this PR to catch this issue
going forward.

Fixes:  containers#1906

Signed-off-by: TomSweeneyRedHat <[email protected]>
TomSweeneyRedHat added a commit to TomSweeneyRedHat/buildah that referenced this pull request Oct 16, 2019
Add back the file checking mechanisms that were removed as part of the
reworking of the cache in containers#1792.  If we have a 'COPY mytar.tar.xz' command
in the Containerfile and the tar file was regenerated after an intial build,
secondary 'buildah bud' commands would use the cache from the first build
for the tar file instead of regenerating it.

The historyMatches() function (https://github.com/containers/buildah/blob/master/imagebuildah/executor.go#L287)
does not seem to pick up this type of change and I wasn't able to quickly
rework that to do so.  We may want to revert back to this check for now,
then tweak the historyMatches to work properly in a follow up PR.

Regardless, I've added a test as part of this PR to catch this issue
going forward.

Fixes:  containers#1906

Signed-off-by: TomSweeneyRedHat <[email protected]>
TomSweeneyRedHat added a commit to TomSweeneyRedHat/buildah that referenced this pull request Oct 16, 2019
Add back the file checking mechanisms that were removed as part of the
reworking of the cache in containers#1792.  If we have a 'COPY mytar.tar.xz' command
in the Containerfile and the tar file was regenerated after an intial build,
secondary 'buildah bud' commands would use the cache from the first build
for the tar file instead of regenerating it.

The historyMatches() function (https://github.com/containers/buildah/blob/master/imagebuildah/executor.go#L287)
does not seem to pick up this type of change and I wasn't able to quickly
rework that to do so.  We may want to revert back to this check for now,
then tweak the historyMatches to work properly in a follow up PR.

Regardless, I've added a test as part of this PR to catch this issue
going forward.

Fixes:  containers#1906

Signed-off-by: TomSweeneyRedHat <[email protected]>
TomSweeneyRedHat added a commit to TomSweeneyRedHat/buildah that referenced this pull request Oct 16, 2019
Add back the file checking mechanisms that were removed as part of the
reworking of the cache in containers#1792.  If we have a 'COPY mytar.tar.xz' command
in the Containerfile and the tar file was regenerated after an intial build,
secondary 'buildah bud' commands would use the cache from the first build
for the tar file instead of regenerating it.

The historyMatches() function (https://github.com/containers/buildah/blob/master/imagebuildah/executor.go#L287)
does not seem to pick up this type of change and I wasn't able to quickly
rework that to do so.  We may want to revert back to this check for now,
then tweak the historyMatches to work properly in a follow up PR.

Regardless, I've added a test as part of this PR to catch this issue
going forward.

Fixes:  containers#1906

Signed-off-by: TomSweeneyRedHat <[email protected]>
TomSweeneyRedHat added a commit to TomSweeneyRedHat/buildah that referenced this pull request Oct 16, 2019
Try 2 after I accidentally nuked my sandbox for containers#1925

Add back the file checking mechanisms that were removed as part of the
reworking of the cache in containers#1792.  If we have a 'COPY mytar.tar.xz' command
in the Containerfile and the tar file was regenerated after an intial build,
secondary 'buildah bud' commands would use the cache from the first build
for the tar file instead of regenerating it.

The historyMatches() function (https://github.com/containers/buildah/blob/master/imagebuildah/executor.go#L287)
does not seem to pick up this type of change and I wasn't able to quickly
rework that to do so.  We may want to revert back to this check for now,
then tweak the historyMatches to work properly in a follow up PR.

Regardless, I've added a test as part of this PR to catch this issue
going forward.

FWIW, with the exception of the test code, all of the code is a reversion to pre-existing code.

Fixes:  containers#1906

Signed-off-by: TomSweeneyRedHat <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bud: COPY . . too aggressive with caching
4 participants