Opt-in layer caching #151

ekcasey · 2020-10-12T21:56:17Z

Fixes #132

Note:
Parts of this change were a bit awkward b/c the buildpack spec is missing the restore phase. However, I think adding a restore phase and/or reorganizing the buildpack spec is beyond the scope of this issue.

Signed-off-by: Emily Casey <[email protected]>

buildpack.md

jkutner · 2020-10-13T13:14:04Z

buildpack.md


 - Whether files in the `<app>` directory have changed since the layer was created.
 - Whether the environment has changed since the layer was created.
 - Whether the buildpack version has changed since the layer was created.
 - Whether new application dependency versions have been made available since the layer was created.

+At the start of the build phase a buildpack MAY find:
+- Partial `<layers>/<layer>.toml` files describing layers from the previous builds. The restored Layer Content Metadata SHALL NOT contain `launch`, `build`, or `cache` booleans.


Given that this might be surprising, maybe we should call-out that it "SHALL NOT" even if those values were set on a previous build.

Signed-off-by: Emily Casey <[email protected]> Co-authored-by: Joe Kutner <[email protected]>

…previously Signed-off-by: Emily Casey <[email protected]>

sclevine

Approving, but see comment on .ignore

sclevine · 2020-10-14T16:43:17Z

buildpack.md

-  - MUST NOT store the layer or the Layer Content Metadata in the cache.
+At the end of each individual buildpack's build phase:
+- The lifecycle:
+  - MUST rename `<layers>/<layer>/` to `<layers>/<layer>.ignore/` for all layers where `launch = false`, `build = false`, and `cache = false`.


Do we need to state how the lifecycle treats layers with a .ignore suffix? An interpretation of this could suggest that <layer>.ignore.toml controls the behavior of the ignored layer, when really we're just trying to get the layer out of the way / change its path.

Hmm, given we only rename to <layer>.ignore when all of launch, build, and cache are false, technically the lifecycle doesn't need to treat a layer with an .ignore suffix any differently than a regular layer. The lifecycle already does nothing with this type of layer.

However, I can add a sentence to clear up the intent behind the rename here.

Signed-off-by: Emily Casey <[email protected]>

ekcasey · 2020-10-14T20:05:39Z

There was one oversight in the original design of this RFC:

Original Assumption: We assumed that Bash buildpacks would be able to easily opt into layer reuse by appending lines to .toml. E.g:

echo "launch = true" >> layer.toml

Problem: However, b/c launch, build, and cache are top level keys they cannot be set this way. The above command would add a launch key to the [metadata] table, not the top level

Proposed Resolution: Therefore, after some discussion in slack we have decided to hold this change and merge it with another change that moves these bools into a label table. E.g

[layer]
launch = true
build = false
cache = false

Since we are moving the keys, this would also be an opportunity to rename them if we wish to before 1.0.

Result To avoid blocking buildpack API 0.5 on the proposed changes I am moving this change to the buildpack API 0.6 milestone and converting this PR to a draft.

TODO After buildpack API 0.5 is release and the buildpack/0.6 branch is created, this PR should be pointed at the new branch.

natalieparellano · 2021-03-09T16:55:27Z

buildpack.md


-Layers marked `launch = false`, `build = false`, and `cache = false` behave like temporary directories, available only to the authoring buildpack, that exist for the duration of a single build.
+The lifecycle restores all layers from previous builds as ignored layers. Buildpacks MUST set `launch`, `build`, and/or `cache` in `<layers>/<layer>.toml` to use a restored layer.


It might be worth clarifying that "as ignored layers" means the types flags are set to false in the toml file, otherwise this could be interpreted to mean that the directory is also named with .ignore.

natalieparellano · 2021-03-09T17:09:30Z

buildpack.md

- The lifecycle:
-  - MUST either restore the entire `<layers>/<layer>.toml` file and corresponding `<layers>/<layer>/` directory from any previous build to the same paths or
-  - MUST restore neither the `<layers>/<layer>.toml` file nor corresponding `<layers>/<layer>/` directory.
+- The lifecycle MUST do both or neither of the following:


Is it worth calling out that "both" is for cache = true and "neither" is for cache = false?

Also there is that edge case where cache = false and launch = true. I don't know if that's too much detail...

I think it's important that the spec will be clear enough and explain all of the cases. I'll try to make it clearer.

natalieparellano · 2021-03-09T17:35:23Z

buildpack.md

-7. Write a partial Bill-of-Material to `<layers>/launch.toml` describing any provided application dependencies.
-8. Write a partial Bill-of-Material to `<layers>/build.toml` describing any provided build dependencies.
+1. Provide the application with dependencies for launch in `<layers>/<layer>`.
+1. Reuse application dependencies from a previous image by appending by `launch = true` to `<layers>/<layer>.toml`.


Suggested change

1. Reuse application dependencies from a previous image by appending by `launch = true` to `<layers>/<layer>.toml`.

1. Reuse application dependencies from a previous image by appending `launch = true` to `<layers>/<layer>.toml`.

This also requires that the buildpack does NOT create a <layers>/<layer> of its own... IDK if that is implied or would be worth spelling out. Personally I found myself re-reading this a couple of times...

natalieparellano · 2021-03-09T17:43:54Z

It might make sense to include #185 in this PR, so that it could all go in together. I had a question about how #185 should be reflected, probably it affects this PR as well: #185 (comment)

yaelharel · 2021-03-23T18:24:55Z

Hi @ekcasey, I opened #209 so I think this PR can be closed (sorry but I don't have permissions to do that).

ekcasey · 2021-03-25T19:46:24Z

Closing in favor of #209

Opt-in layer caching

3f72dd0

Signed-off-by: Emily Casey <[email protected]>

ekcasey requested a review from a team as a code owner October 12, 2020 21:56

ekcasey requested review from hone, jabrown85, jkutner, natalieparellano, nebhale, sclevine and yaelharel October 12, 2020 21:56

ekcasey added this to the Buildpack 0.5 milestone Oct 12, 2020

ekcasey linked an issue Oct 12, 2020 that may be closed by this pull request

[RFC 0052] Opt-in Layer Caching #132

Closed

Fix TOC

f95db0e

Signed-off-by: Emily Casey <[email protected]>

ekcasey added the api/buildpack label Oct 12, 2020

jkutner approved these changes Oct 13, 2020

View reviewed changes

ekcasey and others added 2 commits October 13, 2020 10:43

Apply suggestions from code review

05a09fd

Signed-off-by: Emily Casey <[email protected]> Co-authored-by: Joe Kutner <[email protected]>

Explicitly mention that bools are not restored even if they were set …

6fcc092

…previously Signed-off-by: Emily Casey <[email protected]>

ekcasey force-pushed the opt-in-layer-caching branch from dc95245 to 6fcc092 Compare October 13, 2020 17:53

hone approved these changes Oct 14, 2020

View reviewed changes

sclevine approved these changes Oct 14, 2020

View reviewed changes

Clarify the intent behind renaming ignored layers

fd06da1

Signed-off-by: Emily Casey <[email protected]>

ekcasey marked this pull request as draft October 14, 2020 20:05

ekcasey modified the milestones: Buildpack 0.5, Buildpack 0.6 Oct 14, 2020

natalieparellano reviewed Mar 9, 2021

View reviewed changes

ekcasey linked an issue Mar 11, 2021 that may be closed by this pull request

[RFC 0074] Move layer types to new table in layer.toml #185

Closed

This was unlinked from issues Mar 25, 2021

[RFC 0052] Opt-in Layer Caching #132

Closed

[RFC 0074] Move layer types to new table in layer.toml #185

Closed

ekcasey closed this Mar 25, 2021

natalieparellano mentioned this pull request Apr 1, 2021

Add migration guide for buildpack api 0.6 buildpacks/docs#331

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opt-in layer caching #151

Opt-in layer caching #151

ekcasey commented Oct 12, 2020

jkutner Oct 13, 2020

ekcasey Oct 13, 2020

sclevine left a comment

sclevine Oct 14, 2020

ekcasey Oct 14, 2020

ekcasey commented Oct 14, 2020 •

edited

Loading

natalieparellano Mar 9, 2021

natalieparellano Mar 9, 2021

natalieparellano Mar 9, 2021

yaelharel Mar 11, 2021

natalieparellano Mar 9, 2021

natalieparellano Mar 9, 2021 •

edited

Loading

natalieparellano commented Mar 9, 2021

yaelharel commented Mar 23, 2021

ekcasey commented Mar 25, 2021


		Layers marked `launch = false`, `build = false`, and `cache = false` behave like temporary directories, available only to the authoring buildpack, that exist for the duration of a single build.
		The lifecycle restores all layers from previous builds as ignored layers. Buildpacks MUST set `launch`, `build`, and/or `cache` in `<layers>/<layer>.toml` to use a restored layer.

	1. Reuse application dependencies from a previous image by appending by `launch = true` to `<layers>/<layer>.toml`.
	1. Reuse application dependencies from a previous image by appending `launch = true` to `<layers>/<layer>.toml`.

Opt-in layer caching #151

Opt-in layer caching #151

Conversation

ekcasey commented Oct 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sclevine left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekcasey commented Oct 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalieparellano Mar 9, 2021 • edited Loading

Choose a reason for hiding this comment

natalieparellano commented Mar 9, 2021

yaelharel commented Mar 23, 2021

ekcasey commented Mar 25, 2021

ekcasey commented Oct 14, 2020 •

edited

Loading

natalieparellano Mar 9, 2021 •

edited

Loading