Add mysql db access to productpage sample. by saumoh · Pull Request #473 · istio/istio

saumoh · 2017-07-17T19:05:57Z

The sample bookinfo app has productpage as one of the micro services. This patch optionally add's a mysql db access from the productpage microservice. This could be helpful in checking out the following use cases of a service mesh:

Envoy tcp proxy.
Bypassing Envoy for mysql db access
hybrid use case where the db is not part of k8s infrastructure.

I'd be happy to change this pr request to make it more useful for general consumption.
One option that was discussed was to create a rev. (v2) of productpage micro service which includes the db access and leave v1 version as is.

…hybrid/envoy-bypass use cases.

googlebot · 2017-07-17T19:06:01Z

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-07-17T19:06:04Z

Hi @saumoh. 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.

Details

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.

… service version embedded in the docker image.

googlebot · 2017-07-20T17:37:04Z

CLAs look good, thanks!

saumoh · 2017-07-20T17:43:40Z

@frankbu, @rshriram I've created another version of the productpage image and a mysql container that is pre-populated with the required db entry.
let me know if i should ping other folks as well or not.

rshriram

This is good start @saumoh . Thank you for diversifying the example with more use cases. We need e2e integration test (like the way we have for almost every task in Istio). Otherwise, these things go out of sync quickly.

The way the system is setup now, will only work in K8S. To showcase external access or hybrid deployment stuff, we need to be able to provide username, password and hostname associated with the mysql DB (so that people can point this to any hosted mysql or on-prem mysql and try it out).
I am not sure how that can be tested regularly though - but it certainly falls under the egress proxy test.

rshriram · 2017-07-20T21:36:32Z

samples/apps/bookinfo/src/productpage/Dockerfile

+EXPOSE 9080
+ARG service_version
+ENV SERVICE_VERSION ${service_version:-v1}
+CMD python productpage.py 9080 $MYSQLADDR


where is this variable coming from?

MYSQLADDR comes from the spec for the productpage Deployment. It can be an IP address or a fqdn. For service_version "v2" the productpage Deployment spec would have the following environment defined for the productpage container.

env: - name: MYSQLADDR value: "192.168.144.189"

rshriram · 2017-07-20T21:38:13Z

samples/apps/bookinfo/src/productpage/Dockerfile

+COPY requirements.txt ./
+RUN pip install --no-cache-dir -r requirements.txt
+
+COPY . .


move above requirements.txt line and remove the requierments.txt copy.

oops. will do.

rshriram · 2017-07-21T17:41:19Z

So, I found this in the IBM repos..
https://github.com/IBM/microservices-traffic-management-using-istio#7-modify-sample-application-to-use-the-external-database

This basically did almost what you are trying to do.. Ratings service is talking to mysql. But it has all the necessary code to talk to either an internal or hosted mysql (hostname, password, username, etc..).

I have spoken to @Animeshsing who created this demo. May be you guys could sync up and fix this PR accordingly?

To be clear, I am mainly looking for a simple journey... most of istio tasks run with existing bookinfo.

We add a new task that talks about integrating external services (people can choose whatever mysql db they want - in k8s or outside k8s) as long as they provide all the details. We can then use route rules to switch to new version, tinker with it.

The key requirement is that we need an integration test, that would run before any PR to the master branch.

saumoh · 2017-07-21T18:20:13Z

thanks for the ref. I am fine with leveraging the work done already. I am not a js guy so i'll lean on @Animeshsing to ensuring that it's going to work :-) Couple of suggestions/addition on the patch: - If we could get some defaults for the DB_USER/DB_PORT/DB_PASSWORD(s) that would be great. In case they are not passed as env. variables. DB_HOST is a mandatory env var that must be passed. - and if accessing the db is dependent on a service_version tag (v2: access the db, v1: no db access) If I have a patch with the above changes and the db schema and data used I can convert it all into something that is easily consumable. I am also happy to just adopt/copy the work done by @Animeshsing to the productpage ver proposed in this pr. @Animeshsing please let me know your preference. Irrespective of which ver we use I will write the integration tests to get it all into master. One question i have @rshriram is if it's ok to have the mysql/ docker image as part of this PR? In my opinion having it in there makes it easier to test against and gives everyone a reference to look at. of course folks may not use it and specify var's specific to their environment.

…

On Fri, Jul 21, 2017 at 10:41 AM, Shriram Rajagopalan < ***@***.***> wrote: So, I found this in the IBM repos.. https://github.com/IBM/microservices-traffic-management-using-istio#7- modify-sample-application-to-use-the-external-database This basically did almost what you are trying to do.. Ratings service is talking to mysql. But it has all the necessary code to talk to either an internal or hosted mysql (hostname, password, username, etc..). I have spoken to @Animeshsing who created this demo. May be you guys could sync up and fix this PR accordingly? To be clear, I am mainly looking for a simple journey... most of istio tasks run with existing bookinfo. We add a new task that talks about integrating external services (people can choose whatever mysql db they want - in k8s or outside k8s) as long as they provide all the details. We can then use route rules to switch to new version, tinker with it. The key requirement is that we need an integration test, that would run before any PR to the master branch. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#473 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AT2ksmWEWssr9_tb7f-VcU_28QVl7Uyoks5sQOLEgaJpZM4OaZnT> .

ldemailly · 2017-07-21T18:57:41Z

samples/apps/bookinfo/src/mysql/Dockerfile

+#   limitations under the License.
+
+FROM mysql:8
+ENV MYSQL_ROOT_PASSWORD "password"


any way to get a randomly generated password from env ?

ldemailly · 2017-07-21T18:58:09Z

/ok-to-test

animeshsingh · 2017-07-22T02:08:31Z

Thanks @rshriram for connecting. @saumoh will love to collaborate. Lets connect on Tuesday after between 2-5 PST - whatever tome works for you and we can discuss cc @AnthonyAmanse. Once we agree on a time i can send the invite

saumoh · 2017-07-24T17:26:36Z

@Animeshsing, @AnthonyAmanse great! Let me setup a meeting with you both for 2pm pst 7/25

…

On Fri, Jul 21, 2017 at 7:08 PM, Animesh Singh ***@***.***> wrote: Thanks @rshriram <https://github.com/rshriram> for connecting. @saumoh <https://github.com/saumoh> will love to collaborate. Lets connect on Tuesday after between 2-5 PST - whatever tome works for you and we can discuss cc @AnthonyAmanse <https://github.com/anthonyamanse> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#473 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AT2ksoO0uEXBDQMpx_Rsbt_Vc9DMKfKIks5sQVmigaJpZM4OaZnT> .

saumoh · 2017-07-25T16:14:52Z

@Animeshsing, @AnthonyAmanse let's use google hangouts for the discussion today at 2pm pdt https://plus.google.com/hangouts/_/tigera.io/saurabh?hceid=c2F1cmFiaEB0aWdlcmEuaW8.vf25qu0b75cljn24uleaqfc1co&authuser=0

…

On Mon, Jul 24, 2017 at 10:26 AM, Saurabh Mohan ***@***.***> wrote: @Animeshsing, @AnthonyAmanse great! Let me setup a meeting with you both for 2pm pst 7/25 On Fri, Jul 21, 2017 at 7:08 PM, Animesh Singh ***@***.***> wrote: > Thanks @rshriram <https://github.com/rshriram> for connecting. @saumoh > <https://github.com/saumoh> will love to collaborate. Lets connect on > Tuesday after between 2-5 PST - whatever tome works for you and we can > discuss cc @AnthonyAmanse <https://github.com/anthonyamanse> > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#473 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AT2ksoO0uEXBDQMpx_Rsbt_Vc9DMKfKIks5sQVmigaJpZM4OaZnT> > . >

* Enable Prow * Fix BUILD format * Add execution perm to the script * Fix shell script * Fix flag name * Use the right k8s cluster * Create the ~.kube dir * Linking instead of moving * Explicitly specify dir * Remove debug statements * Rename test_log_path to test_logs_path for consistency * Adds desc + failure conditions

…hybrid/envoy-bypass use cases.

istio-merge-robot · 2017-07-31T23:49:59Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:51:00Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:52:03Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:52:59Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:53:59Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:54:59Z

@saumoh PR needs rebase

istio-merge-robot · 2017-07-31T23:55:59Z

@saumoh PR needs rebase

yutongz · 2017-07-31T23:58:33Z

Yes, it's a bug about merge-bot, will fix it before reopen.

istio-merge-robot · 2017-08-01T00:29:40Z

@saumoh PR needs rebase

istio-merge-robot · 2017-08-01T00:30:40Z

@saumoh PR needs rebase

istio-merge-robot · 2017-08-01T01:19:39Z

@saumoh PR needs rebase

istio-merge-robot · 2017-08-01T01:20:40Z

@saumoh PR needs rebase

yutongz · 2017-08-01T01:43:00Z

Sorry for all the mess, so the problem is Mungegithub wants to add a label "needs-rebase" while there is no such label existing and merge-robot is so sad that he cannot create a label in this repo, so he keeps bugging. Since I can't create labels, can any one with access add it, merge-robot will appreciate it :)

saumoh · 2017-08-01T06:01:38Z

Closing this pull request. The plan is to work with @animeshsingh @AnthonyAmanse on a broader version of this.

@rshriram

Automatic merge from submit-queue Add mysql db to ratings service in bookinfo app. Tracked in #573 and discussed in #473, adding mysql db access to the ratings service. cc @rshriram @AnthonyAmanse @ldemailly

This PR will add a print out of version information to the log for istio mixer. This should ease debugging, etc., as the version info will come along with the log. Example: ```shell $ ./bazel-bin/cmd/server/mixs server Istio Mixer: version: unknown (build: 2017-03-29-84ea18c, status: Modified) Starting gRPC server on port 9091 ``` Former-commit-id: 7b4e4a37769b8132e9327333568767a013bef924

@rshriram

Automatic merge from submit-queue Add mysql db to ratings service in bookinfo app. Tracked in #573 and discussed in #473, adding mysql db access to the ratings service. cc @rshriram @AnthonyAmanse @ldemailly Former-commit-id: 7b5271d

This PR will add a print out of version information to the log for istio mixer. This should ease debugging, etc., as the version info will come along with the log. Example: ```shell $ ./bazel-bin/cmd/server/mixs server Istio Mixer: version: unknown (build: 2017-03-29-84ea18c, status: Modified) Starting gRPC server on port 9091 ``` Former-commit-id: c926e35fdd6686b270eab3799ef6670af4cb3a96

@rshriram

Automatic merge from submit-queue Add mysql db to ratings service in bookinfo app. Tracked in istio#573 and discussed in istio#473, adding mysql db access to the ratings service. cc @rshriram @AnthonyAmanse @ldemailly Former-commit-id: 7b5271d

* Let MANAGER_HUB and MANAGER_TAG env vars override defaults in inject.go if present * Remove defaults in the binary in favor of env variables * Use test-specific hub instead of env var * Added comments on env var name constants * Format

@rshriram

Automatic merge from submit-queue Add mysql db to ratings service in bookinfo app. Tracked in #573 and discussed in #473, adding mysql db access to the ratings service. cc @rshriram @AnthonyAmanse @ldemailly Former-commit-id: 7b5271d

* Update to latest common-files * Address reviewer comments.

* Update readme for controller local debugging * Address comment

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>

) If an injected annotation value contained a double-quote character, the rendered istiod-injector-configmap YAML would be invalid. Unmarshalling the inject config would fail with: update error: failed to unmarshal injection template: error converting YAML to JSON: yaml: line 10: did not find expected key

saumohos added 2 commits July 17, 2017 09:58

Add mysql db access to productpage so as to be able to show non-html/…

8d83080

…hybrid/envoy-bypass use cases.

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

d647b3f

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 Jul 17, 2017

istio-testing added the needs-ok-to-test label Jul 17, 2017

saumohos added 3 commits July 19, 2017 13:01

Gen ver2 of productpage and create rev of mysql db service.

4c308f5

Mysql service docker image with db initialized.

fa4d829

Productpage base use of mysql off the environment variable setting of…

3576f71

… service version embedded in the docker image.

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 Jul 20, 2017

Merge branch 'master' into master

c8bbbeb

rshriram suggested changes Jul 20, 2017

View reviewed changes

Merge branch 'master' into master

a17cfdc

ldemailly reviewed Jul 21, 2017

View reviewed changes

istio-testing removed the needs-ok-to-test label Jul 21, 2017

Merge branch 'master' into master

57c56b1

saumoh and others added 5 commits July 26, 2017 22:19

Merge branch 'master' into master

c0b5d60

Add mysql db access to productpage so as to be able to show non-html/…

6bf7282

…hybrid/envoy-bypass use cases.

Gen ver2 of productpage and create rev of mysql db service.

ecccffd

Mysql service docker image with db initialized.

9de59ff

saumoh closed this Aug 1, 2017

This was referenced Aug 17, 2017

Bookinfo app add mysql db access to ratings service #573

Closed

Add mysql db to ratings service in bookinfo app. #574

Merged

kyessenov pushed a commit to kyessenov/istio that referenced this pull request Aug 13, 2018

Update mixerclient with latest (istio#473)

9666dcd

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

Update to latest common-files (istio#473)

9f46b09

* Update to latest common-files * Address reviewer comments.

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

Update readme for controller local debugging (istio#473)

fd93371

* Update readme for controller local debugging * Address comment

danehans pushed a commit to danehans/istio that referenced this pull request Nov 2, 2021

Copy Wasm binaries from upstream. (istio#473)

053f270

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>

Conversation

saumoh commented Jul 17, 2017

Uh oh!

googlebot commented Jul 17, 2017

Uh oh!

istio-testing commented Jul 17, 2017

Uh oh!

googlebot commented Jul 20, 2017

Uh oh!

saumoh commented Jul 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rshriram left a comment

Choose a reason for hiding this comment

Uh oh!

rshriram Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

saumoh Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

rshriram Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

saumoh Jul 20, 2017

Choose a reason for hiding this comment

Uh oh!

rshriram commented Jul 21, 2017

Uh oh!

saumoh commented Jul 21, 2017 via email

Uh oh!

ldemailly Jul 21, 2017

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Jul 21, 2017

Uh oh!

animeshsingh commented Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saumoh commented Jul 24, 2017 via email

Uh oh!

saumoh commented Jul 25, 2017 via email

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Jul 31, 2017

Uh oh!

yutongz commented Jul 31, 2017

Uh oh!

istio-merge-robot commented Aug 1, 2017

Uh oh!

istio-merge-robot commented Aug 1, 2017

Uh oh!

istio-merge-robot commented Aug 1, 2017

Uh oh!

istio-merge-robot commented Aug 1, 2017

Uh oh!

yutongz commented Aug 1, 2017

Uh oh!

saumoh commented Aug 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

saumoh commented Jul 20, 2017 •

edited

Loading

animeshsingh commented Jul 22, 2017 •

edited

Loading