Add container and buildpack start commands to app status #936
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
lildann
force-pushed
the
container_start_cmd
branch
from
eb05df5
to
397e101
Compare
pkg/reconciler/app/reconciler.go
Outdated
|
|
||
| startCommands := app.Status.StartCommands | ||
|
|
||
| containerConfig, err := r.fetchContainerCommand(app) |
There was a problem hiding this comment.
Can we make sure this only happens when the image that will be running changes? (assuming the customer uses SHA based imaging rather than labels that don't change)
This code is on a hot path and will be executed each time a child object changes, several times through app startup, and also every 15ish minutes for every app which could lead to a lot of load on the container registry.
pkg/reconciler/app/reconciler.go
Outdated
|
|
||
| containerConfig, err := r.fetchContainerCommand(app) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
It is probably worth reflecting this error onto the app status instead so the developer can understand why the pull failed.
…status Signed-off-by: Oladipupo Ajayi <ajayidipo@ymail.com> Co-authored-by: Lilly Daniell <lilly.daniell@engineerbetter.com> Co-authored-by: Adrienne Galloway <adrienne.galloway@engineerbetter.com>
Signed-off-by: Oladipupo Ajayi <ajayidipo@ymail.com> Co-authored-by: Lilly Daniell <lilly.daniell@engineerbetter.com> Co-authored-by: Adrienne Galloway <adrienne.galloway@engineerbetter.com>
|
We had an issue when pushing an app on the
|
|
My guess is that this is coming from a missing or bad file in the lifecycle step: It gets the CF lifecycle binaries from the earlier steps, if the ko images were built incorrectly or the binaries were missing/had bad permissions you might see this. I'd double check the binaries in the local checked-out version to see if they've been modified. |
Alternatively, maybe the new jq command in this PR isn't available or correctly set up in the build image? |
|
The All feedback has been implemented and tested. |
| startCommands: | ||
| description: StartCommands defines the container and buildpack start commands for the App. | ||
| type: object | ||
| properties: |
There was a problem hiding this comment.
This will likely need to be updated with the two new properties
There was a problem hiding this comment.
The new properties have already been added
Proposed Changes