Fix some imports and added some changes to K8S deployment #7
Conversation
deploy/kubernetes/deployment.yml
Outdated
| spec: | ||
| containers: | ||
| - image: sample/bookstore-service-broker:0.0.1.BUILD-SNAPSHOT | ||
| - image: eu.gcr.io/cf-sandbox-ohughes/bookstore-service-broker:0.0.1.BUILD-SNAPSHOT |
There was a problem hiding this comment.
This change should not be committed
| To test the Service Instance API, the URL can be constructed from the binding credentials; | ||
|
|
||
| --- | ||
| export SI_URI=$(kubectl get secret bookstore-binding -n test -o json | jq -r '.data.uri' | base64 --decode) |
There was a problem hiding this comment.
If we're going to suggest this command, we'll need to add jq as an additional pre-requisite at the top of the document. We shouldn't assume everyone will have that installed.
deploy/kubernetes/deployment.yml
Outdated
| metadata: | ||
| name: bookstore-broker | ||
| namespace: bookstore-broker | ||
| namespace: service-catalog |
There was a problem hiding this comment.
Why the namespace change? That ripples thru a lot of files and doc references. The namespace service-catalog might be confused with the namespace that the actual Service Catalog extension is deployed to, or potentially clash with other service broker sample deployments. It would be better if the name were specific to bookstore.
deploy/kubernetes/service-broker.yml
Outdated
| run: bookstore-broker | ||
| spec: | ||
| url: http://bookstore-broker.bookstore-broker.svc.cluster.local | ||
| url: http://35.197.249.208 |
There was a problem hiding this comment.
Will the DNS name not work with LoadBalanced? This IP address won't be valid for all deployments, will it?
There was a problem hiding this comment.
Yes using the internal DNS works for me
deploy/kubernetes/README.adoc
Outdated
|
|
||
| ---- | ||
| $ kubectl create -f deploy/kubernetes/service-broker-secret.yml | ||
| $ kubectl create secret generic -n bookstore-broker bookstore-broker-secret --from-literal=username='admin' --from-literal=password='supersecret' |
There was a problem hiding this comment.
I like the consistency of creating all artifacts from files, but I'm OK with creating with CLI options instead. This commit should remove deploy/kubernetes/service-broker-secret.yml if it's not going to be used though.
deploy/kubernetes/README.adoc
Outdated
|
|
||
| ---- | ||
| $ curl http://192.168.1.235:32248/v2/catalog -u admin:supersecret | ||
| $ curl http://35.197.249.208/v2/catalog -u admin:supersecret |
There was a problem hiding this comment.
Will this IP address be valid for all deployments?
There was a problem hiding this comment.
No but I can't see a way of making an IP deterministic without creating and external DNS record
deploy/kubernetes/README.adoc
Outdated
| ---- | ||
|
|
||
| Note the value in the `EXTERNAL-IP` column of the node whose `NAME` matches the node for the `bookstore-broker` pod. | ||
| Note the value in the `LoadBalancer Ingress` column and the `Port` column below. |
There was a problem hiding this comment.
There's no output below this with the specified columns.
deploy/kubernetes/README.adoc
Outdated
| ---- | ||
| $ kubectl describe service bookstore-broker -n bookstore-broker | ||
| Name: bookstore-broker | ||
| $ kubectl describe service bookstore-broker -n bookstore-brokerName: bookstore-broker |
There was a problem hiding this comment.
Need a line break before Name: bookstore-broker
|
@ojhughes I had to fix the imports to get another change in. You should be able to rebase and remove those changes. |
Ensure internal DNS and correct NS used Use substituted external IP Advise on JQ prerequisite
2 participants
No description provided.