Consul related Dockerfiles and rule yamls #674

GregHanson · 2017-09-05T23:27:27Z

Release note:

<PLEASE PUT RELEASE-NOTE HERE, PUT NONE if NO RELEASE-NOTE>

Added consul with docker related Dockerfiles, build scripts, rule yamls, Consul readme, and docker-compose file for starting everything for that demo

frankbu · 2017-09-06T00:14:45Z

samples/apps/bookinfo/CONSUL.md

+## Bookinfo Demo
+
+The ingress controller is still under construction, rounting functionalities can be tested by curling a service container directly.
+


s/rounting/routing/

frankbu

I would suggest moving most of these files to a separate directory (e.g., samples/apps/bookinfo/consul), except the ones that can't be (like the Dockerfiles)

andraxylia · 2017-09-06T04:00:47Z

samples/apps/bookinfo/CONSUL.md

+When a service instance is brought up in docker, the [Registrator](http://gliderlabs.github.io/registrator/latest/)
+automatically registers the service in Consul.
+
+Noted that Istio pilot is running inside each app container so as to coordinate Envoy and the service mesh.


This is about pilot agent, isn't it? We refer to Istio pilot as the central pilot, taking care of discovery.

andraxylia · 2017-09-06T04:01:58Z

samples/apps/bookinfo/CONSUL.md

@@ -0,0 +1,76 @@
+# Consul Adapter for Istio on Docker


This is good write-up for a task, I hope you will move it to istio.io.

andraxylia · 2017-09-06T04:02:52Z

samples/apps/bookinfo/CONSUL.md

+
+Noted that Istio pilot is running inside each app container so as to coordinate Envoy and the service mesh.
+
+## Prequisites


Prerequisites.

andraxylia · 2017-09-06T04:04:32Z

samples/apps/bookinfo/CONSUL.md

+You can create basic routing rules using istioctl from the `/bookinfo` directory:
+
+```
+istioctl create -f consul-reviews-v1.yaml


Rules have been moved to /rules directory. It would be good to place them there from the beginning.

I suggest bookinfo/consul, or maybe bookinfo/consul/rules

@andraxylia not all rules have been moved to /rules. There are duplicates in both directories. the rules in /rules can be used by kubectl only, when I tried yesterday they did not work with istioctl

$ istioctl create -f ../rules/route-rule-all-v1.yaml Error: cannot parse proto message: unknown spec type

conversation yesterday in #pilot

Laurent Demailly [1:54 PM] afaik istioctl understand bookinfo/ and kubetcl understands bookinfo/rules/ [1:55] ie istioctl doesn't understand v2 kuat [1:55 PM] istioctl understands v2 spec but v1 meta

We need to do something about this, like moving the old rules under a directory v0.1 or similar. Having them at the top level would be very confusing. Of course, this can be done separate from your PR, but in your PR I would refer to the new rules and use kubectl, if istioctl has not been fixed to understand v2.

The rule files in bookinfo are obsolete v1 - they won't work with istioctl or kubectl. They will be removed as soon as we figure out how to fix broken links in blog. The rules in /rules are the new ones, currently only work with kubectl, until istioctl is changed to use the same format as kubectl.

Your rules in this PR seem to written using the partial-v2 istioctl-compatible format, which will work for now, but once istioctl is updated to support the proper new format, they will need to be changed.

So, I think you are OK to put your rules in /consul/rules and use istioctl to create them for now.

andraxylia · 2017-09-06T04:05:57Z

samples/apps/bookinfo/src/prepare_proxy.sh

@@ -0,0 +1,92 @@
+#!/bin/bash


Why is this being copied here? We have a version in pilot.

I agree. we need to reuse that one.

andraxylia · 2017-09-06T04:06:28Z

/lgtm

istio-merge-robot · 2017-09-06T04:07:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andraxylia

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~~ [andraxylia]

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

istio-merge-robot · 2017-09-06T14:40:46Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @GregHanson @andraxylia

rshriram

This is a very good start. I suggest creating a consul branch in istio/istio and pushing all this there. We can all refine this.. There is a lot of cleanups that can be done. I am worried about the need to manually update app images, for each proxy/pilot update - and would prefer to do this in automated fashion. Ideally, we should be able to use the debian package that @costinm has in the istio/proxy branch in order to install all the istio proxy stuff (including the iptables). We should just reuse the pilot discovery container as is, and override the kubeconfig path and registry via the Entrypoint. If its not created that way, lets fix that.

rshriram · 2017-09-06T00:16:26Z

samples/apps/bookinfo/CONSUL.md

+When a service instance is brought up in docker, the [Registrator](http://gliderlabs.github.io/registrator/latest/)
+automatically registers the service in Consul.
+
+Noted that Istio pilot is running inside each app container so as to coordinate Envoy and the service mesh.


Note that. Is this istio pilot agent ?

rshriram · 2017-09-06T00:17:09Z

release/create_release_archives.sh

@@ -95,7 +95,7 @@ function create_windows_archive() {

 pushd ${ROOT}
 ${CP} istio.VERSION LICENSE README.md CONTRIBUTING.md "${COMMON_FILES_DIR}"/
-find samples install -type f \( -name "*.yaml" -o -name "cleanup*" -o -name "README.md" \) \
+find samples install -type f \( -name "*.yaml" -o -name "cleanup*" -o -name "README.md" -o -name "CONSUL.md" \) \
  -exec ${CP} --parents {} "${COMMON_FILES_DIR}" \;


We shouldn't ship this. This goes in docs.

rshriram · 2017-09-06T14:11:43Z

samples/apps/bookinfo/src/prepare_proxy.sh

@@ -0,0 +1,92 @@
+#!/bin/bash


I agree. we need to reuse that one.

rshriram · 2017-09-06T14:13:41Z

samples/apps/discovery/Dockerfile

@@ -0,0 +1,8 @@
+FROM lyft/envoy:latest


why do we need envoy on the pilot agent?

GregHanson · 2017-09-06T14:49:54Z

moving PR to #681

pushed to consul branch in istio/istio so anyone can contribute to the source as needed

istio-testing · 2017-09-06T15:04:12Z

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

Test name	Commit	Details	Rerun command
prow/e2e-suite-rbac-no_auth.sh	`94bb9ae`	link	`/test e2e-suite-rbac-no_auth`
prow/e2e-suite-rbac-auth.sh	`94bb9ae`	link	`/test e2e-suite-rbac-auth`
prow/e2e-suite-no_rbac-auth.sh	`94bb9ae`	link	`/test e2e-suite-no_rbac-auth`
prow/e2e-suite-no_rbac-no_auth.sh	`94bb9ae`	link	`/test e2e-suite-no_rbac-no_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.

Automatic merge from submit-queue Consul related files for istio release **Release note**: ```release-note Examples to enable Istio run in docker environment by integrating Consul as a service registry. ``` moving #674 to this new PR so anyone can contribute to PR

…_go mention,...) (istio#675) * dev doc updates (working example, shorter vars, ./ in paths, bazel_to_go mention,...) Updates while going through the steps: Make the examples work (Fixes Issue istio#674 ) Shorter variables / no need to create the mixer directory, git clone does it Clearer and makes tab completion faster to use ./ in paths relative to current directory Mention of ./bin/bazel_to_go.py * Mention issue with kubeconfig and update info about string attribute * Hyperlink the file that needs edit Former-commit-id: 0d429685ab8f774e059398a9d73d777b1473a221

Automatic merge from submit-queue Consul related files for istio release **Release note**: ```release-note Examples to enable Istio run in docker environment by integrating Consul as a service registry. ``` moving #674 to this new PR so anyone can contribute to PR Former-commit-id: b83f3d7

…_go mention,...) (#675) * dev doc updates (working example, shorter vars, ./ in paths, bazel_to_go mention,...) Updates while going through the steps: Make the examples work (Fixes Issue #674 ) Shorter variables / no need to create the mixer directory, git clone does it Clearer and makes tab completion faster to use ./ in paths relative to current directory Mention of ./bin/bazel_to_go.py * Mention issue with kubeconfig and update info about string attribute * Hyperlink the file that needs edit Former-commit-id: 794d54668edbcddbcc67a55067e47afb031408e8

Automatic merge from submit-queue Consul related files for istio release **Release note**: ```release-note Examples to enable Istio run in docker environment by integrating Consul as a service registry. ``` moving istio#674 to this new PR so anyone can contribute to PR Former-commit-id: b83f3d7

* fix bug * bug fix in bug fix * minor

Automatic merge from submit-queue Consul related files for istio release **Release note**: ```release-note Examples to enable Istio run in docker environment by integrating Consul as a service registry. ``` moving #674 to this new PR so anyone can contribute to PR Former-commit-id: b83f3d7

* watch Istio resources changes in istio-operator * fix test error * Review Comments * Fix e2e test failure * Fix e2e test failure * Fix e2e failure * debug e2e failure * debug operator e2e * debug e2e * debug e2e * debug e2e * remove e2e debug change

GregHanson added 2 commits September 5, 2017 12:55

added consul with docker Dockerfiles and build scripts

465d8b5

added consul docker rule yamls and README

8a6e1d6

GregHanson added the area/environments label Sep 5, 2017

GregHanson added this to the Istio 0.2 milestone Sep 5, 2017

GregHanson requested review from frankbu and rshriram September 5, 2017 23:27

googlebot added the cla: yes label Sep 5, 2017

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

added CONSUL readme to release archiver script

b042d50

frankbu reviewed Sep 6, 2017

View reviewed changes

andraxylia reviewed Sep 6, 2017

View reviewed changes

istio-testing assigned andraxylia Sep 6, 2017

istio-testing added the lgtm label Sep 6, 2017

andraxylia approved these changes Sep 6, 2017

View reviewed changes

istio-merge-robot added the approved label Sep 6, 2017

code review comments, used prepare_proxy.sh in pilot/docker

94bb9ae

istio-merge-robot removed the lgtm label Sep 6, 2017

rshriram reviewed Sep 6, 2017

View reviewed changes

GregHanson mentioned this pull request Sep 6, 2017

Consul related files for istio release #681

Merged

GregHanson closed this Sep 6, 2017

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

bugfix: use seconds instead of ms (#674)

6ff20cf

* fix bug * bug fix in bug fix * minor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consul related Dockerfiles and rule yamls #674

Consul related Dockerfiles and rule yamls #674

GregHanson commented Sep 5, 2017 •

edited

Loading

frankbu Sep 6, 2017

frankbu left a comment

andraxylia Sep 6, 2017

andraxylia Sep 6, 2017

andraxylia Sep 6, 2017

andraxylia Sep 6, 2017 •

edited

Loading

frankbu Sep 6, 2017

GregHanson Sep 6, 2017

andraxylia Sep 6, 2017

frankbu Sep 6, 2017

andraxylia Sep 6, 2017

rshriram Sep 6, 2017

andraxylia commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

rshriram left a comment

rshriram Sep 6, 2017

rshriram Sep 6, 2017

rshriram Sep 6, 2017

rshriram Sep 6, 2017

GregHanson commented Sep 6, 2017

istio-testing commented Sep 6, 2017

		## Bookinfo Demo

		The ingress controller is still under construction, rounting functionalities can be tested by curling a service container directly.


		Noted that Istio pilot is running inside each app container so as to coordinate Envoy and the service mesh.

		## Prequisites

Consul related Dockerfiles and rule yamls #674

Consul related Dockerfiles and rule yamls #674

Conversation

GregHanson commented Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

frankbu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andraxylia Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andraxylia commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

rshriram left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregHanson commented Sep 6, 2017

istio-testing commented Sep 6, 2017

GregHanson commented Sep 5, 2017 •

edited

Loading

andraxylia Sep 6, 2017 •

edited

Loading