Sub-team RFC for lifecycle package re-organization #217
Conversation
Signed-off-by: Natalie Arellano <narellano@vmware.com>
natalieparellano
added
team/implementation
scope/team
|
Maintainers, As you review this RFC please queue up issues to be created using the following commands: Issues(none) |
text/0000-lifecycle-pkg-refactor
Outdated
| │ ├── archive | ||
| │ ├── buildpack | ||
| │ │ ├── env | ||
| │ │ ├── launch |
There was a problem hiding this comment.
Actually nothing in buildpack imports this code. Maybe it should live at pkg/launch.
text/0000-lifecycle-pkg-refactor
Outdated
|
|
||
| * The structure of the lifecycle should mirror the spec as much as possible. However we should recognize that the spec is not perfect. Also, while there is a `lifecycle` package, there is no "lifecycle spec" - the responsibilities of the lifecycle are mixed between the platform and buildpack specs (see below). | ||
| * In general the `lifecycle` package should be more of an orchestrator - see [example](https://github.com/buildpacks/lifecycle/blob/b6803be364429689a818a5d0a09db42bd21b9995/analyzer.go#L52-L54). We should push as much logic as possible into services. | ||
| * CNB business logic should live inside the `buildpack`, `lifecycle`, and `platform` packages (and their nested packages) or their `internal` equivalents. All other packages that are direct children of `pkg` or `internal` should be CNB-free. |
There was a problem hiding this comment.
I think having some sort of rule like this will help us keep our packages well scoped.
text/0000-lifecycle-pkg-refactor
Outdated
| * The structure of the lifecycle should mirror the spec as much as possible. However we should recognize that the spec is not perfect. Also, while there is a `lifecycle` package, there is no "lifecycle spec" - the responsibilities of the lifecycle are mixed between the platform and buildpack specs (see below). | ||
| * In general the `lifecycle` package should be more of an orchestrator - see [example](https://github.com/buildpacks/lifecycle/blob/b6803be364429689a818a5d0a09db42bd21b9995/analyzer.go#L52-L54). We should push as much logic as possible into services. | ||
| * CNB business logic should live inside the `buildpack`, `lifecycle`, and `platform` packages (and their nested packages) or their `internal` equivalents. All other packages that are direct children of `pkg` or `internal` should be CNB-free. | ||
| * Logic that branches on api version should be handled by an internal service nested under the appropriate package - e.g., logic that branches on buildpack api should live in `internal/buildpack` - see [example](https://github.com/buildpacks/lifecycle/blob/b6803be364429689a818a5d0a09db42bd21b9995/buildpack/bom.go#L14-L23). |
There was a problem hiding this comment.
This to me feels like the most helpful organizing principle. Instead of having a bunch of if api.MustParse(platformAPI).AtLeast("XXX") sprinkled throughout the code, we can consolidate the branching logic in the constructor, by giving our orchestrator the right services for the api it implements.
text/0000-lifecycle-pkg-refactor
Outdated
| [alternatives]: #alternatives | ||
|
|
||
| - What other designs have been considered? Doing nothing, just keep adding code as best we can. | ||
| - Why is this proposal the best? Making it easier to add new code will allow us to deliver features faster. |
There was a problem hiding this comment.
Another benefit of having a well-organized lifecycle is that by adding good README files and docs comments, we can turn the lifecycle into an easy-to-consume repository of information for further understanding the CNB spec.
Signed-off-by: Natalie Arellano <narellano@vmware.com>
| - Launch | ||
| - Platform inputs and outputs (args, flags, env vars, mounts, exit codes) | ||
| - Registry auth | ||
| - Buildpacks directory layout |
There was a problem hiding this comment.
It has always sort of bothered me that the buildpack package contains DirBuildpackStore and I think this is why!
| │ ├── inputs # handles platform <-> lifecycle interface (args, flags) | ||
| │ ├── launch | ||
| │ │ └── inputs # handles platform <-> launcher interface (args, flags) |
There was a problem hiding this comment.
See also buildpacks/lifecycle#805
|
Closing this RFC as ideas have evolved somewhat since its inception, though as an instrument for thinking things through it was still useful. |
Readable