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

fix: allow caching run layers with no filesystem changes #355

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

mafredri
Copy link
Member

Fixing this issue turned out to be quite easy as Kaniko had an option for it already. We now enable caching of empty layers by enabling the ForceBuildMetadata option whenever push image is set.

Fixes #326

@mafredri mafredri self-assigned this Sep 25, 2024
NoPush: !opts.PushImage || len(destinations) == 0,
CacheRunLayers: true,
CacheCopyLayers: true,
ForceBuildMetadata: opts.PushImage, // Force layers with no changes to be cached, required for cache probing.
Copy link
Member

Choose a reason for hiding this comment

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

Is there any downside to always enabling this option regardless of PushImage?

Copy link
Member Author

@mafredri mafredri Sep 25, 2024

Choose a reason for hiding this comment

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

I'm not sure. It'll be an additional cache layer to download during builds vs executing the command and ending up with the same result. Mainly added it to be cautious.

Imagine the following scenario:

RUN if [[ $(date +%s) -gt 1734998400 ]]; then touch "/'member-christmas?"; fi

With force build metadata, we would still use the cache here and not produce a new image (assuming cache exists from before).

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, let's leave as-is for now 👍

@mafredri mafredri merged commit 67cb94c into main Sep 25, 2024
4 checks passed
@mafredri mafredri deleted the mafredri/fix-cache-probing-missing-layer branch September 25, 2024 10:19
johnstcn pushed a commit that referenced this pull request Sep 26, 2024
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.

Allow cache probing to work when instructions (e.g. RUN) don't change any files
2 participants