Buildpack sample edits #222
Conversation
samodell
commented
Jul 20, 2018
samodell
commented
- Edits nodejs and dotnet buildpack samples
google-prow-robot
added
approved
size/M
| @@ -1,14 +1,14 @@ | |||
| # Buildpack Sample App | |||
|
|
|||
| A sample app that demonstrates usage of Cloud Foundry buildpacks on Knative Serving, | |||
| A sample app that demonstrates using Cloud Foundry buildpacks on Knative Serving, | |||
There was a problem hiding this comment.
Can we link to Cloud Foundry here? Not everyone may know what that is.
| @@ -22,11 +22,13 @@ First, install the Buildpack build template from that repo: | |||
| kubectl apply -f buildpack.yaml | |||
There was a problem hiding this comment.
Where do I get this file from? Maybe call out to save it from the [Buildpack build template] link?
| REPO="gcr.io/<your-project-here>" | ||
| # Replace <your-project-here> with your own registry | ||
| export REPO="gcr.io/<your-project-here>" | ||
|
|
||
| perl -pi -e "s@DOCKER_REPO_OVERRIDE@$REPO@g" serving/samples/buildpack-app-dotnet/sample.yaml |
There was a problem hiding this comment.
I have no idea what this command does. Can we replace it with the instructions to update the yaml file we use elsewhere?
There was a problem hiding this comment.
I wasn't 100% sure either, but I was worried that this was an important part of the sample. It looks like @mattmoor added this in this PR: knative/serving#851
Matt, what does perl -pi -e "s@DOCKER_REPO_OVERRIDE@$REPO@g" serving/samples/buildpack-app-dotnet/sample.yaml do? Do we need that command for this sample to work?
| REPO="gcr.io/<your-project-here>" | ||
| # Replace <your-project-here> with your own registry | ||
| export REPO="gcr.io/<your-project-here>" | ||
|
|
||
| perl -pi -e "s@DOCKER_REPO_OVERRIDE@$REPO@g" serving/samples/buildpack-app-dotnet/sample.yaml | ||
|
|
||
| # Create the Kubernetes resources |
There was a problem hiding this comment.
Our other samples use the URL relative to the project folder, not the repo. Not sure what the best practice is, but we should be consistent.
| @@ -1,20 +1,20 @@ | |||
| # Buildpack Sample Function | |||
There was a problem hiding this comment.
I think my comments above apply here too, so I won't repeat them.
…ion (#222) * Added port-forwarding information to telemetry documentation to prevent exposing Prometheus and Grafana publicly. * Removed public IP option for Prometheus UI (this is mainly used for debugging and can be done using port forwarding) * Changed the order of installation of Prometheus in the build file. Previous ordering was causing an error on a brand new cluster and required retrying the step to succeed. I still need to root cause the issue but checking this in as a mitigation/workaround for the time being.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rgregg, samodell The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
