-
Notifications
You must be signed in to change notification settings - Fork 58
Update Knative version Readme #708
base: knative
Are you sure you want to change the base?
Conversation
@@ -90,6 +92,10 @@ Installing Dispatch depends on having a Kubernetes cluster with the Knative comp | |||
|
|||
4. Deploy via helm chart (if helm is not installed and initialized, do that first): | |||
```bash | |||
cd charts/dispatch | |||
helm dependency update | |||
helm init |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably add a new optional step above for helm init
and also mentions that tiller requires cluster admin privileges (refer the legacy installation steps)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: helm dep up
instead of the verbose command
README.md
Outdated
``` | ||
|
||
2. Build and publish a dispatch image: | ||
```bash | ||
PUSH_IMAGES=1 make images | ||
``` | ||
> **NOTE**: You may need `docker login` before push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit : You may need to docker login
before push
README.md
Outdated
@@ -90,6 +92,10 @@ Installing Dispatch depends on having a Kubernetes cluster with the Knative comp | |||
|
|||
4. Deploy via helm chart (if helm is not installed and initialized, do that first): | |||
```bash | |||
cd charts/dispatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to cd
helm dep up ./charts/dispatch/
README.md
Outdated
export CLUSTER_NAME=dispatch-knative | ||
gcloud container clusters create -m n1-standard-4 --cluster-version ${K8S_VERSION} ${CLUSTER_NAME} | ||
gcloud container clusters get-credentials ${CLUSTER_NAME} | ||
gcloud container clusters create -m n1-standard-4 --cluster-version ${K8S_VERSION} ${CLUSTER_NAME} --zone us-west1-c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make the zone an env var too?
README.md
Outdated
```bash | ||
cat << EOF > config.json | ||
{ | ||
"current": "${RELEASE_NAME}", | ||
"contexts": { | ||
"${RELEASE_NAME}": { | ||
"host": "$(kubectl -n ${DISPATCH_NAMESPACE} get service ${RELEASE_NAME}-nginx-ingress-controller -o json | jq -r .status.loadBalancer.ingress[].ip)", | ||
"host": "$(kubectl -n ${DISPATCH_NAMESPACE} get service ${RELEASE_NAME}-nginx-ingress-controller -o json | jq -r ".status.loadBalancer.ingress[].ip")", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be single quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both works, then single quotes seems better
No description provided.