Introduce "kpack" Droplets and Support Custom Start Commands#1917
Introduce "kpack" Droplets and Support Custom Start Commands#1917
Conversation
tcdowney
requested review from
cwlbraa,
jspawar,
matt-royal,
njbennett,
sannidhi and
sethboyles
|
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/175081086 The labels on this github issue will be updated when the story is started. |
cwlbraa
left a comment
cwlbraa
left a comment
There was a problem hiding this comment.
This looks good, but given what we've learned about the current state of PATCH droplet.process_types, the migration to turn docker droplets into kpack droplets and then use those seems likely to break apps that just happen to work currently.
| return buildpack_lifecycle_data if buildpack_lifecycle_data | ||
|
|
||
| DockerLifecycleDataModel.new | ||
| buildpack_lifecycle_data || kpack_lifecycle_data || DockerLifecycleDataModel.new |
There was a problem hiding this comment.
I wish CC Ruby used this syntax more often instead of the return x if x guards all over the place
tcdowney
force-pushed
the
introduce-kpack-droplets-175081086
branch
2 times, most recently
from
291b973 to
0a55501
Compare
- allows us to distinguish droplets that use a kpack-built container image versus a "regular" docker droplet - this allows us to prepend the CNG launcher command, `/cnb/lifecycle/launcher`, to any custom start commands that are specified - addresses cloudfoundry/capi-k8s-release#72 and cloudfoundry/cf-for-k8s#502 - add index to kpack_lifecycle_data.droplet_guid - this is important since droplets find their associated lifecycle through this column - a similar index exists on buildpack_lifecycle_data [#175081086](https://www.pivotaltracker.com/story/show/175081086) Authored-by: Tim Downey <tdowney@vmware.com>
- `@process.command` only covers commands set through the API - `@process.started_command` is what we use when scheduling LRPs on BOSH since it also includes detected start commands and commands from a `Procfile` - this worked before for the `web` process because the kpack-built image's `Entrypoint` knows the detected start command -- it did not work for multi-process apps, however [#175081086](https://www.pivotaltracker.com/story/show/175081086) Authored-by: Tim Downey <tdowney@vmware.com>
tcdowney
force-pushed
the
introduce-kpack-droplets-175081086
branch
from
0a55501 to
f05a752
Compare
|
The unit test failures are unrelated. It looks like these have been failing on Github Actions on other PRs (with unrelated changes) as well. :/ Started a Slack thread discussing this: |
| Sequel.migration do | ||
| up do | ||
| alter_table :kpack_lifecycle_data do | ||
| add_index :droplet_guid, name: :kpack_lifecycle_droplet_guid_index |
There was a problem hiding this comment.
This strikes me as an obvious improvement, but I've never thought about indices on lifecycle_data tables prior to now. It seems likely that we've missed some other indices on this table in the past lifecycle_data links itself to all sorts of other objects...
There was a problem hiding this comment.
I put a little blurb in the commit message about this one:
- add index to kpack_lifecycle_data.droplet_guid
- this is important since droplets find their associated lifecycle
through this column
- a similar index exists on buildpack_lifecycle_data
I think it's important if we want the droplet.kpack? / droplet.buildpack? type of methods to be performant.
|
|
||
| def to_hash | ||
| command = if @process.started_command.presence | ||
| ['/bin/sh', '-c', "#{CNB_LAUNCHER_PATH} #{@process.started_command}"] |
There was a problem hiding this comment.
🤔 is the bin/sh necessary here? it's safe to leave it in but it's kinda interesting to think about.
There was a problem hiding this comment.
Yeah, I thought that as well. I went with this cause it's what we were doing for docker droplets, but 🤷 .
https://github.com/cloudfoundry/cloud_controller_ng/pull/1417/files#diff-738bfbb749a5827ff834ca07b3594487e3f321066cf64d20ff4bdf7359137f4bR74
I can ask the Eirini folks. Slack thread: https://cloudfoundry.slack.com/archives/C8RU3BZ26/p1603391841149800
as of cloudfoundry/cloud_controller_ng#1917 apps staged using kpack will have "kpack" droplets instead of "docker" droplets [#175081086](https://www.pivotaltracker.com/story/show/175081086) Authored-by: Tim Downey <tdowney@vmware.com>
5 participants
This PR introduces the concept of "kpack" droplets by associating the existing
kpack_lifecycle_datafor a build with its droplet instead of creating "docker" droplets. It allows us to distinguish kpack-built droplets from regular "docker" droplets and means we can now support custom start commands by invoking them with the CNB launcer:/cnb/lifecycle/launcher.Additionally it includes a migration to convert existing kpack-built "docker" droplets into "kpack" droplets by associating their
kpack_lifecycle_datawith the "docker" droplet.This change addresses the following issues:
CAKE Story: https://www.pivotaltracker.com/story/show/175081086