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

Eliminate auto-removal of empty layers #2651

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

vladaionescu
Copy link
Contributor

This fixes an issue related to inline caching resulting in cache import errors like this one: invalid layer index 29. The cause of this is the image writer optimizing the final image by removing empty layers. Because this takes place after the inline cache manifest has been constructed, the indexes of the layers from the inline cache manifest end up being incorrect. Sometimes they end up being out of bounds.

Originally reported as an issue in Earthly here: earthly/earthly#1635.

CC @tonistiigi

Signed-off-by: Vlad A. Ionescu [email protected]

Signed-off-by: Vlad A. Ionescu <[email protected]>
@tonistiigi
Copy link
Member

Should also fix #2551

Did you understand why it goes out of sync if the empty layer removal appears both on image and cache creation?

@vladaionescu
Copy link
Contributor Author

Did you understand why it goes out of sync if the empty layer removal appears both on image and cache creation?

Not really... I played around with some builds, but I couldn't really get to the bottom of it.

@tonistiigi
Copy link
Member

Could you add a test that fails with previous behavior because of the wrong indexing? Or at least show a small reproducer for the issue.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Bringing this in for now but we should test this more before GA.

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