API-enabled BookInfo sample application #693

ErikWittern · 2017-09-07T17:53:15Z

This pull request adds a JSON API to the BookInfo application. Changed details, ratings, and reviews microservices now return JSON responses. The productpage microservice fetches JSON responses and either renders them in HTML productpage, or exposes them via API endpoints. Added a swagger.yaml file documenting the API. Exposed the API via ingress controller through changes in bookinfo.yaml.

The motivations behind this pull request are to more closely follow best practices of microservices communicating in a lean, machine-understandable data format, and to allow experiments with an API as discussed with @rshriram.

Updated bookinfo services to support proper REST responses (JSON format). You can now use swagger UI to access bookinfo API on your cluster.

…expose an API

…oints to return JSON

googlebot · 2017-09-07T17:53:19Z

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).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

istio-testing · 2017-09-07T17:53:24Z

Hi @ErikWittern. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

istio-merge-robot · 2017-09-07T17:56:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: rshriram

Assign the PR to them by writing /assign @rshriram in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

rshriram · 2017-09-07T18:12:50Z

/ok-to-test

ErikWittern · 2017-09-07T18:15:13Z

I signed it!

googlebot · 2017-09-07T18:15:17Z

CLAs look good, thanks!

frankbu

Overall, this looks great, but one problem is that when reviews-v1 is being used, the productpage now displays "no ratings available" messages, where previously it didn't show anything at all, and only showed the message in the case that the ratings service was failing (unavailable). Since we're running a version of the reviews service where the ratings feature doesn't even exists, we shouldn't be displaying the concept in the UI either.

@frankbu

…omment by @frankbu on pull request istio#693

ErikWittern · 2017-09-07T19:47:15Z

@frankbu Good point! I have committed a change in 04ee5fa to display nothing if no ratings are present, rather than displaying an error-message.

istio-merge-robot · 2017-09-08T02:25:45Z

@ErikWittern PR needs rebase

rshriram · 2017-09-08T14:16:50Z

Updated bookinfo app with proper REST style API and Swagger files

frankbu

LGTM

rshriram · 2017-09-08T15:06:12Z

All necessary tests pass again.. Phew! merging..

istio-testing · 2017-09-08T15:19:37Z

@ErikWittern: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/e2e-suite-no_rbac-no_auth.sh	`a6d9c5d`	link	`/test e2e-suite-no_rbac-no_auth`
prow/e2e-suite-rbac-auth.sh	`faae06e`	link	`/test e2e-suite-rbac-auth`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ldemailly · 2017-09-14T17:56:48Z

samples/apps/bookinfo/src/ratings/ratings.js


-if (process.env.SERVICE_VERSION == "v2") {
-  if (process.env.DB_TYPE == "mysql") {


please restore this ASAP

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db

* Docker tag to be short-sha-date to ease release Updating version of jenkins library. * Using origin as remote name in Jenkins Former-commit-id: 895fd1aeadabe48e0fc598dac7bde97b1ced5c94

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 Former-commit-id: 2ffc8ed

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

* Docker tag to be short-sha-date to ease release Updating version of jenkins library. * Using origin as remote name in Jenkins Former-commit-id: 11845b1251fed4748530c0756d88bdcb1a701593

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request istio#693 Former-commit-id: 2ffc8ed

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request istio#693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

) * istioctl: Use cobra.Command::Example for documenting example usage * add missing word in comment

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 Former-commit-id: 2ffc8ed

@frankbu

* revised source code of microservices so they communicate in JSON and expose an API * make API accessible via ingress controller * added swagger definition of new API * removed HTML status pages of microservices; aligned their health endpoints to return JSON * do not display anything when ratings data is not present to address comment by @frankbu on pull request #693 * added back support for MySQL into ratings microservice * fixed trailing line * fixed space * fixed ratings data in mongo db * fixed ratings data in mysql db Former-commit-id: 584ab1b

This commit adds a TcpClusterRewrite proto to be used in the upcoming TCP cluster rewrite filter on istio/proxy. Signed-off-by: Venil Noronha <veniln@vmware.com>

Erik Wittern added 4 commits September 7, 2017 11:55

revised source code of microservices so they communicate in JSON and …

70c0de2

…expose an API

make API accessible via ingress controller

4a392f2

added swagger definition of new API

e977f06

removed HTML status pages of microservices; aligned their health endp…

37cb9cd

…oints to return JSON

googlebot added the cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. label Sep 7, 2017

istio-testing added the needs-ok-to-test label Sep 7, 2017

rshriram requested a review from frankbu September 7, 2017 17:55

rshriram self-assigned this Sep 7, 2017

istio-merge-robot added the release-note-label-needed label Sep 7, 2017

rshriram approved these changes Sep 7, 2017

View reviewed changes

Merge remote-tracking branch 'upstream/master'

4e65fb2

istio-testing removed the needs-ok-to-test label Sep 7, 2017

googlebot added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Sep 7, 2017

Merge branch 'master' into master

d05dffc

frankbu requested changes Sep 7, 2017

View reviewed changes

ErikWittern and others added 4 commits September 7, 2017 15:32

Merge branch 'master' into master

89f1678

Merge remote-tracking branch 'upstream/master'

ed5c802

do not display anything when ratings data is not present to address c…

04ee5fa

…omment by @frankbu on pull request istio#693

Merge branch 'master' of https://github.com/ErikWittern/istio

a6d9c5d

rshriram closed this Sep 7, 2017

rshriram added the needs-ok-to-test label Sep 7, 2017

rshriram reopened this Sep 8, 2017

istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 8, 2017

Merge branch 'master' into master

faae06e

istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 8, 2017

rshriram removed the needs-ok-to-test label Sep 8, 2017

rshriram added release-note and removed release-note-label-needed labels Sep 8, 2017

frankbu approved these changes Sep 8, 2017

View reviewed changes

rshriram merged commit 2ffc8ed into istio:master Sep 8, 2017

rshriram mentioned this pull request Sep 9, 2017

update bookinfo containers after code changes #705

Merged

ldemailly reviewed Sep 14, 2017

View reviewed changes

frankbu mentioned this pull request Oct 5, 2017

Bookinfo is not displaying ratings unavailable message #1034

Closed

mandarjog pushed a commit that referenced this pull request Oct 31, 2017

istioctl: Use cobra.Command::Example for documenting example usage (#693

9b9f0d6

) * istioctl: Use cobra.Command::Example for documenting example usage * add missing word in comment

howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020

Fix manifest diff output (istio#693)

a45cdc2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API-enabled BookInfo sample application #693

API-enabled BookInfo sample application #693

ErikWittern commented Sep 7, 2017 •

edited by rshriram

Loading

googlebot commented Sep 7, 2017

istio-testing commented Sep 7, 2017

istio-merge-robot commented Sep 7, 2017

rshriram commented Sep 7, 2017

ErikWittern commented Sep 7, 2017

googlebot commented Sep 7, 2017

frankbu left a comment

ErikWittern commented Sep 7, 2017

istio-merge-robot commented Sep 8, 2017

rshriram commented Sep 8, 2017

frankbu left a comment

rshriram commented Sep 8, 2017

istio-testing commented Sep 8, 2017

ldemailly Sep 14, 2017


		if (process.env.SERVICE_VERSION == "v2") {
		if (process.env.DB_TYPE == "mysql") {

API-enabled BookInfo sample application #693

API-enabled BookInfo sample application #693

Conversation

ErikWittern commented Sep 7, 2017 • edited by rshriram Loading

googlebot commented Sep 7, 2017

istio-testing commented Sep 7, 2017

istio-merge-robot commented Sep 7, 2017

rshriram commented Sep 7, 2017

ErikWittern commented Sep 7, 2017

googlebot commented Sep 7, 2017

frankbu left a comment

Choose a reason for hiding this comment

ErikWittern commented Sep 7, 2017

istio-merge-robot commented Sep 8, 2017

rshriram commented Sep 8, 2017

frankbu left a comment

Choose a reason for hiding this comment

rshriram commented Sep 8, 2017

istio-testing commented Sep 8, 2017

ldemailly Sep 14, 2017

Choose a reason for hiding this comment

ErikWittern commented Sep 7, 2017 •

edited by rshriram

Loading