RFC: lifecycle/all #46

ekcasey · 2020-01-22T18:13:09Z

text/0000-lifecycle-all.md

jabrown85 · 2020-01-22T20:44:04Z

If bin/develop gets built and get it's own binary at /cnb/lifecycle/develop should we consider /cnb/lifecycle/build or another name?

nebhale · 2020-01-22T21:03:19Z

text/0000-lifecycle-all.md

+
+Each of these phases will continue to be available individually as provided by the current lifecycle.
+
+### Platform API 0.3


Love this callout and I don't have a problem being "more" breaking than would be strictly necessary.

text/0000-lifecycle-all.md

matthewmcnew · 2020-01-23T02:57:57Z

text/0000-lifecycle-all.md

+This new binary supports two main goals.
+
+### Goal 1: faster `pack` builds
+By creating a single container and invoking `/cnb/lifecycle/all` instead of creating a container per lifecycle phase, pack can shave approximate `6s` from the execution time of `pack build`.


Do we know what the primary cause of the 6s increase in multi container builds? Is it purely the docker startup overhead?

It appeared to be from the overhead of both starting up as well as tearing down the container. We didn't do a complete teardown of where in the container lifecycle we were losing time. Comparing the execution time of the binaries within the container vs the execution time of the entire container had a constant ∆ of roughly 1.5s. So running 1 container instead of 5 showed about a 6-7s improvement in the end to end pack build times.

matthewmcnew · 2020-01-23T03:01:00Z

text/0000-lifecycle-all.md

+
+### Usage
+
+`/cnb/lifecycle/all <image-name>`


Currently, pack prepends lifecycle output with the step of the container builds. Such as: ===> DETECTING.

Are we expecting the /lifecycle/all binary prepend the output or will pack still detect the current step in the build?

In this case, pack would have to be more clever when invoking /lifecycle/all to know which phase is running and this effort might be non trivial. I suspect the all binary might have to be better about logging which phase is currently being run.

As a stop gap we could add the phase to the lifecycle logs. I think the better longterm solution is the structured logging RFC I have long been threatening to submit. This should be a good forcing function.

+1 to structured logging

matthewmcnew · 2020-01-23T03:11:55Z

text/0000-lifecycle-all.md

+| `-gid`          | required | `CNB_GROUP_ID`         | -                         | GID of user's group in the stack's build and run images |
+| `-version`      | optional | -                      | false                     | show version |
+| `-skip-restore` | optional | -                      | false                     | do not restore metadata from previous image or data from cache
+| `-tag` (multiples allowed) | optional | -           |                           | additional tags to apply to exported image


What image would the lifecycle analyze for previous layers? Could we add a previous-image to separate the tags that we right to from the resulting location?

The argument provided as the <image name> would be the one that the lifecycle would analyze, no? -tag will be used to specify additional write locations.

It seems a bit confusing to me that tags are not the complete list of locations that the lifecycle will export to especially because doesn't have the tag name inside of it.

Additionally, at times it can be useful to explicitly call out the image input to the analyze step. For example: kpack could pass in the previous image's digest to the analyzer. You obviously wouldn't export to an image name with the digest.

@matthewmcnew I am not opposed to adding an optional flag for the previous image image, so long as it default to the export tag when not supplied.

The rationale behind the single positional argument is that an image tag is the only required input. Whereas every other flag configures the build process, the <image-name> is the primary thing the process acts on.

Given that we already enforce that all exported tags are tags on the same registry, what if we changed the value passed to -t to be a "tag tag" (in the <registry.com/repo/name:tag> sense). I am not sure exporting to multiple different repositories in the same registry is really a valid use case. Maybe that would make the usage clearer - only the positional argument gets to define the repository.

Love the optional previous image flag.

I fear using "tag tag" as the value passed to -t would be confusing because it would be differ from the "tag" terminology in docker and kpack.

Could we just reuse the multiple positional arguments like what is in exporter today for multiple tags?

A single positional argument and additional -tags seems more honest? The exporter currently treats the first of the positional arguments differently than the rest. In this proposed phase the positional argument will be used as the default value for the analyzer image. If one of the tags in special, 1 positional arg + flags seems to indicate that more clearly.

ekcasey · 2020-01-23T20:16:48Z

@jabrown85

should we consider /cnb/lifecycle/build or another name?

build seems too close to builder and therefore likely to be confusing.

/cnb/lifecycle/combo?

text/0000-lifecycle-all.md

jromero · 2020-01-23T20:42:25Z

text/0000-lifecycle-all.md

+[summary]: #summary
+
+### `/cnb/lifecycle/all`
+A new lifecycle binary with the name `lifecycle/all` will be included in each released lifecycle archive. Within a builder image, it will be found at the path `/cnb/lifecycle/all`. When invoked, it will run the following phases sequentially:


RE: name

Creating a thread to discuss the name of this new binary.

Previous comments:

If bin/develop gets built and get it's own binary at /cnb/lifecycle/develop should we consider /cnb/lifecycle/build or another name?
#46 (comment) - @jabrown85

build seems too close to builder and therefore likely to be confusing.
/cnb/lifecycle/combo?
#46 (comment) - @ekcasey

Not a fan of combo....

Also, FWIW, it already isn't all in that it doesn't include rebaser.

@jromero gonna start a thread just to shoot down every idea without adding a suggestion? ;)

lifecycle/make or lifecycle/constructor?

ooo, I think /lifecycle/make is my favorite so far (@ashwin-venkatesh agrees 💯 )

@ekcasey :D Wanted to start the thread but didn't have any good ideas myself.

Binary name suggestions:

/lifecycle/phases

Maybe we don't need a new binary at all and only provide a subcommand to the single lifecycle binary.

lifecycle/lifecycle build-app

lifecycle/lifecycle run-phases

I'm ok with lifecycle/make but it could be confused with make

jkutner · 2020-01-23T21:27:39Z

text/0000-lifecycle-all.md

+# Unresolved Questions
+[unresolved-questions]: #unresolved-questions
+
+- Is there a better name than `lifecycle/all`? It doesn't include `launcher` or `rebaser` functionality so it isn't truly `all`.


lifecycle/accio

lifecycle/alohomora

lifecycle/morsmordre (dark magic)

lifecycle/sectumsempra (very dark magic)

lifecycle/sectumsempra

lifecycle/silencio

lifecycle/ash_nazg to rule them all

nebhale · 2020-01-29T19:55:05Z

I move that this RFC be sent to FCP with a disposition of merge, closing 1700 PT, February 5, 2019.

ekcasey · 2020-01-29T21:05:14Z

text/0000-lifecycle-all.md

+# Summary
+[summary]: #summary
+
+### `/cnb/lifecycle/all`


Following suggestion in WG I am going to use this comment thread to conduct an informal poll to select the name for this binary. After this comment I will add comments each containing a single suggestion:

Please vote with a 👍

No more than 2 votes per person

Feel free to express dislike with a 👎 (full disclosure these may not factor into the counting)

To add a suggestion, add a comment containing only the suggestion

lifecycle/all

lifecycle/make

lifecycle/combo

lifecycle/phases

lifecycle/constructor

lifecycle/creator

/lifecycle/build

zmackie · 2020-02-06T19:23:38Z

@ekcasey Will make changes to codify lifecycle/creator

* name selected via poll #46 (comment) Signed-off-by: Emily Casey <[email protected]>

* summary * motivation * 1/2 of what-is-it Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

* adds context around security concerns * indicates the uid and gid are required Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

* typo fixes * docker build -t as prior art Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

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

* name selected via poll #46 (comment) Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

[resolves #46] Signed-off-by: Ben Hale <[email protected]>

ekcasey changed the title ~~RFC: /lifecycle/all~~ RFC: lifecycle/all Jan 22, 2020

ekcasey requested review from sclevine, hone, jkutner, nebhale and thisisnotashwin January 22, 2020 18:14

ekcasey added team/implementation team/platform labels Jan 22, 2020

thisisnotashwin approved these changes Jan 22, 2020

View reviewed changes

ekcasey force-pushed the lifecycle-all branch from 53202f2 to 56e9573 Compare January 22, 2020 18:43

thisisnotashwin approved these changes Jan 22, 2020

View reviewed changes

ekcasey mentioned this pull request Jan 22, 2020

RFC to isolate registry creds from builder images #43

Closed

zmackie added the roadmap/ci-cd label Jan 22, 2020

nebhale reviewed Jan 22, 2020

View reviewed changes

sclevine approved these changes Jan 22, 2020

View reviewed changes

nebhale approved these changes Jan 22, 2020

View reviewed changes

matthewmcnew reviewed Jan 23, 2020

View reviewed changes

jkutner reviewed Jan 23, 2020

View reviewed changes

text/0000-lifecycle-all.md Outdated Show resolved Hide resolved

jromero reviewed Jan 23, 2020

View reviewed changes

jkutner approved these changes Jan 23, 2020

View reviewed changes

jkutner reviewed Jan 23, 2020

View reviewed changes

nebhale added the Final Comment Period label Jan 29, 2020

ekcasey commented Jan 29, 2020

View reviewed changes

jromero approved these changes Jan 30, 2020

View reviewed changes

ekcasey mentioned this pull request Jan 31, 2020

Enable interception via shell per lifecycle phase execution buildpacks/pack#485

Closed

2 tasks

ekcasey mentioned this pull request Feb 3, 2020

Lifecycle/creator buildpacks/lifecycle#243

Merged

natalieparellano mentioned this pull request Feb 6, 2020

pack build could run faster buildpacks/pack#345

Closed

ekcasey added a commit that referenced this pull request Feb 6, 2020

/cnb/lifecycle/all -> /cnb/lifecycle/creator

5a31cbf

* name selected via poll #46 (comment) Signed-off-by: Emily Casey <[email protected]>

ekcasey requested a review from a team as a code owner February 6, 2020 20:35

ekcasey and others added 7 commits February 6, 2020 12:58

introduces /cnb/lifecycle/all

66a874a

* summary * motivation * 1/2 of what-is-it Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

lifecycle/all, remaining sections

5773ebc

* adds context around security concerns * indicates the uid and gid are required Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

incorporating PR review feedback

66da105

* typo fixes * docker build -t as prior art Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

-stack default value -> /cnb/stack.toml

d8ac56f

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

adds previous-image flag

33f24ca

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

/cnb/lifecycle/all -> /cnb/lifecycle/creator

92aee48

* name selected via poll #46 (comment) Signed-off-by: Emily Casey <[email protected]> Signed-off-by: Ben Hale <[email protected]>

RFC #26

528b1a1

[resolves #46] Signed-off-by: Ben Hale <[email protected]>

nebhale force-pushed the lifecycle-all branch from 5a31cbf to 528b1a1 Compare February 6, 2020 21:00

nebhale closed this in 528b1a1 Feb 6, 2020

nebhale merged commit 528b1a1 into master Feb 6, 2020

nebhale deleted the lifecycle-all branch February 6, 2020 21:01

jromero mentioned this pull request Jul 2, 2020

Document Trusted Builders buildpacks/docs#167

Closed

2 tasks

ekcasey removed the Final Comment Period label Oct 14, 2020


		Each of these phases will continue to be available individually as provided by the current lifecycle.

		### Platform API 0.3

RFC: lifecycle/all #46

RFC: lifecycle/all #46

Conversation

ekcasey commented Jan 22, 2020

jabrown85 commented Jan 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisnotashwin Jan 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thisisnotashwin Jan 23, 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

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

Choose a reason for hiding this comment

ekcasey commented Jan 23, 2020

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

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

dgageot Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

nebhale commented Jan 29, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmackie commented Feb 6, 2020

thisisnotashwin Jan 23, 2020 •

edited

Loading

thisisnotashwin Jan 23, 2020 •

edited

Loading

dgageot Jan 31, 2020 •

edited

Loading