Envoy-based ingress controller #71

zcahana · 2017-02-02T13:21:57Z

This is a first-pass on Envoy-based ingress controller (addressing #6).
I'm PR'ing this while still WIP to start gathering feedback and set this in the right direction.

What we have so far:

Ingress controller running in the context of the manager (via manager ingress command).
Extending the platform/kube/Controller to sync ingress resources.
Adding ingress watcher that reconfigures Envoy based on the given ingresses.
Envoy clusters made up of single upstream (service's DNS name).

What's still missing:

Represent ingress rules in the Istio model, so that proxy/envoy isn't coupled with k8s objects.
Support for path regex in ingress (under the limitations of what Envoy supports)
Constructs envoy clusters using SDS + pod endpoints.
Update ingress status with the proxy's "public" IP address (node IP?).
TLS termination.
Testing.

googlebot · 2017-02-02T13:21:59Z

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 you signed the CLA as a corporation, please let us know the company's name.

rshriram · 2017-02-02T14:04:43Z

@zcahana could you please sign the cla?

zcahana · 2017-02-02T14:21:00Z

@rshriram I applied to join the ibm-contributors group, waiting for it to get approved.

googlebot · 2017-02-02T14:21:03Z

CLAs look good, thanks!

rshriram

@zcahana thanks a lot for the comments in the code. See my review comments. Also for first pass, we can skip SSL/TLS. We don't have support for that in the manager yet (i.e., pulling it out of kube secrets, etc.).

With regard to the TODOs

Represent ingress rules in the Istio model, so that proxy/envoy isn't coupled with k8s objects.
- This is a WIP. See PR [wip] Proxy cfg validation & route generation #68
Support for path regex in ingress (under the limitations of what Envoy supports)
- We can skip this. Envoy doesn't support it yet. A TODO should suffice.
Constructs envoy clusters using SDS + pod endpoints.
- Yes to this. And CDS as well.
Update ingress status with the proxy's "public" IP address (node IP?).
- Depends on the provider isn't it? local cluster, it could be nodeIP. In GCE, I have seem something like a load balancer.
TLS termination.
- Skip for now. May be take in the configs(?). Needs TLS config support in envoy config generation (pulling secrets out of k8s. Long term, a different solution is needed, but thats long term).
Testing
- yes. And in this case, we probably need an end to end integration test with the go version of bookinfo example in istio/istio, on top of other unit tests, etc.

One more comment (a future todo): refactor config generation code such that its reused between both internal proxy and ingress as well.

rshriram · 2017-02-02T14:26:35Z

cmd/manager/main.go

+		"Discovery service DNS address")
+	ingressCommand.PersistentFlags().IntVarP(&flags.proxy.AdminPort, "admin_port", "a", 5000,
+		"Envoy admin port")
+	ingressCommand.PersistentFlags().StringVarP(&flags.proxy.BinaryPath, "envoy_path", "b", "/envoy_esp",


Envoy's admin port is of little use to end user. It is a readonly port. I don't even think this needs to be exposed.
Could you also add a TODO here saying that the hardcoded use of envoy must be changed to something more generic? e.g.,
``//TODO: Change to proxy_type: envoy|nginx|haproxy, and then make the rest (SDS, etc) as sub flags```

rshriram · 2017-02-02T14:29:11Z

model/BUILD

@@ -4,6 +4,7 @@ go_library(
    name = "go_default_library",
    srcs = [
        "controller.go",
+        "ingress.go",
        "registry.go",


Please remove ingress from the model. Model is platform agnostic representation of resources. Mesos may not even have ingress.

So, we need something in the model (RoutingRule?) that the envoy ingress watcher can consume, and the kube ingresses converted to. Is there already any work in that direction?

No, there is no model object for the ingress. I think it is likely going to be a superset of k8s ingress resource, perhaps, even reusing RoutingRule. Can we create a custom struct for Ingress that is just a duplicate of k8s ingress for now? I'm not happy with the dependency on k8s here.

+1. Model package is completely platform independent.

rshriram · 2017-02-02T14:29:38Z

model/controller.go

@@ -34,6 +36,9 @@ type Controller interface {
 	// for a service.
 	AppendInstanceHandler(f func(*ServiceInstance, Event)) error

+	// AppendIngressHandler notifies about changes to ingress resources
+	AppendIngressHandler(f func(*v1beta1.Ingress, Event)) error
+


May be add a type to the Config struct to detect if its an ingress or not. This way, ingress becomes kube specific.

I'm not sure I understand how the Config is designed to be worked with. Any reference or explanation can help.

Config is untyped, since when controller was designed when we did not have any config resources defined. There are two options here - you can define a proto for Ingress and reuse AppendConfigHandler with a cast to your ingress proto-generated struct. I'm also OK with having a dedicated ingress handler for now assuming we get the config ball rolling later.

Wont the ingress config be the same or a superset of the ProxyConfig, with some additional stuff, that literally configures Envoy? i.e., the meshConfig object in code is the thing that needs to be generalized and surfaced right?

The ingress config seems to be a subset of the model's RouteRule (that's what you mean by ProxyConfig, right)? So, we can have the controller convert ingresses to such rules, and have the proxy watcher subscribe to these.

Yes. Please take a look at the Generate function in proxy/envoy/config.go. You can provide an empty array for upstreams and rules. It should give you an envoy configuration for services as hostnames and / as the default prefix. Along with SDS functionality. In next iteration (or if you want to hack the utility functions), you could generate Envoy route entries under a virtual host with specific path or prefixes.

Bottom line. There is opportunity for code reuse. The config.go takes care of generating all the right listener blocks (with use_orig_dst, etc).

rshriram · 2017-02-02T14:30:08Z

model/ingress.go

+type IngressRegistry interface {
+	// ListIngresses all ingress rules currently defined.
+	ListIngresses() []*v1beta1.Ingress
+}


Please (re)move was you see fit.

If Ingress becomes a subset of Configs, this will go away. We can treat this as an adaptor interface from a config registry to an ingress registry.

rshriram · 2017-02-02T14:31:31Z

proxy/envoy/ingress.go

+type ingressWatcher struct {
+	agent   Agent
+	mesh    *MeshConfig
+	ingress model.IngressRegistry


one favor: could you add a place holder or a TODO, so that #32 can be incorporated here easily as well?

Not sure I understand that comment...

I think the idea is to merge ingress watcher and sidecar (+config) watcher since they are almost same. We can do that later, to parallelize work for now.

I'd suggest to postpone merging the two watchers until we get a clearer picture of how much logic they share. At least for now, they are generating quite a different envoy configuration. The sidecar watcher shouldn't care / handle ingress-based configurations, whereas the ingress watcher shouldn't allow non-ingressed traffic.

Sounds good to me. We should share code for config generation but the controllers themselves should be independent of one another. Let's see how it works out and extract proper module interfaces later.

rshriram · 2017-02-02T14:33:20Z

proxy/envoy/ingress.go

+	time.Sleep(256 * time.Millisecond)
+}
+
+func (w *ingressWatcher) generateConfig() (*Config, error) {


is there a way to de-duplicate this functionality by moving code out of proxy/envoy/watcher.go(?) that generates Envoy configuration, and reusing it in both places? After all, that code just expects a list of services, pods, and rules. in the absence of rules, it behaves just like a user-space kube proxy (except operating on per-pod basis). Whatever is missing will not be configured.

On second thought, lets add a TODO here as a reminder that we need to refactor this code. We could do that in a later PR.
Kind of similar to the IdentityProvider stuff in A8, in principle. What do you think?

Yes, there's some de-duplication and code reuse that can be done. Will be refactored in next iterations on this.

rshriram · 2017-02-02T14:37:40Z

proxy/envoy/ingress.go

+		}
+
+		// TODO: Validate no duplicate path/routes within this virtualhost
+		// (e.g., due to 2 default backends defined in 2 ingresses in different namespaces)


silly question: is ingress common to all namespaces? or specific to a namespace? IF latter, we don't need this right?

Ingresses are defined within namespaces, but are normally realized by a single namespace-neutral ingress controller, which serves incoming traffic to all namespaces. This TODO refers to the need to make sure there are no collision between ingresses in different namespaces, which might yield invalid envoy configuration (e.g., a virtualhost with 2 similar routes, or 2 prefix-similar routes, etc).

rshriram · 2017-02-02T14:39:06Z

proxy/envoy/ingress.go

+	listeners = append(listeners, Listener{
+		Port:           80,
+		BindToPort:     true,
+		UseOriginalDst: true,


Set to false please. This does not apply to ingress. Its only for internal envoy instances.

rshriram · 2017-02-02T14:46:53Z

proxy/envoy/ingress.go

+				Type:             "strict_dns", // TODO: Change to SDS
+				LbType:           DefaultLbType,
+				ConnectTimeoutMs: DefaultTimeoutMs,
+				Hosts:            clusterHosts(namespace, backend),


don't we have to set the cluster "features": "http2", since we would have Envoy backends? If this is for istio only, then by default (and for first cut) I think we should set to features to "http2", since all http communication happens over Envoy to Envoy. If not specified, Envoy falls back to http/1.1, nullifying a lot of benefits that we get out of http/2.

I think this is Istio-only for now. Will set "http2".

I'll try to refactor current config to make it more re-usable. It handles things like enabling http2.

rshriram · 2017-02-02T14:48:11Z

proxy/envoy/ingress.go

+		Listeners: listeners,
+		ClusterManager: ClusterManager{
+			Clusters: clusters,
+		},


please set the "local_cluster_name": "ingress". Local cluster name is needed for a variety of features in envoy (CDS, tracing, possibly RDS, etc.).

Reading here, it seems that local_cluster_name is only needed for enabling zone aware routing. This is probably out of the scope of this PR.

kyessenov

Looks like you are making good progress.
Regarding testing: I have set-up a basic e2e infrastructure for in-cluster routing in test/integration. To test ingress, you probably want to deploy ingress proxies, then add ingress resource, and send a request directly to ingress proxies to check that it gets routed to both ingress and sidecar proxy by checking access log for the request ID.

kyessenov · 2017-02-02T20:00:22Z

cmd/manager/main.go

+		"Envoy binary location")
+	ingressCommand.PersistentFlags().StringVarP(&flags.proxy.ConfigPath, "config_path", "e", "/etc/envoy",
+		"Envoy config root location")
+	rootCmd.AddCommand(ingressCommand)


If you do proxyCmd.AddCommand(ingressComand) then you inherit all flags from proxy command, and make ingress a subcommand.

Yes, but should ingress be a subcommand of proxy? In essence, these are two different modes of operation (in-cluster service sidecar proxy vs. edge ingress proxy).

Most (all?) of the commonalities stem from the fact that both commands manage envoy, so maybe both should be a child command of an "envoy" command?

Yeah, good point. I think we need a shared root, and then run "sidecar", "ingress", and "egress" as a subcommand. Actually, the flag parameter need to be turned into ProxyMeshConfig in the proto, but that's for later.

kyessenov · 2017-02-02T20:02:00Z

model/BUILD

@@ -4,6 +4,7 @@ go_library(
    name = "go_default_library",
    srcs = [
        "controller.go",
+        "ingress.go",
        "registry.go",


No, there is no model object for the ingress. I think it is likely going to be a superset of k8s ingress resource, perhaps, even reusing RoutingRule. Can we create a custom struct for Ingress that is just a duplicate of k8s ingress for now? I'm not happy with the dependency on k8s here.

kyessenov · 2017-02-02T20:03:45Z

model/controller.go

@@ -34,6 +36,9 @@ type Controller interface {
 	// for a service.
 	AppendInstanceHandler(f func(*ServiceInstance, Event)) error

+	// AppendIngressHandler notifies about changes to ingress resources
+	AppendIngressHandler(f func(*v1beta1.Ingress, Event)) error
+


Config is untyped, since when controller was designed when we did not have any config resources defined. There are two options here - you can define a proto for Ingress and reuse AppendConfigHandler with a cast to your ingress proto-generated struct. I'm also OK with having a dedicated ingress handler for now assuming we get the config ball rolling later.

kyessenov · 2017-02-02T20:04:42Z

model/ingress.go

+type IngressRegistry interface {
+	// ListIngresses all ingress rules currently defined.
+	ListIngresses() []*v1beta1.Ingress
+}


If Ingress becomes a subset of Configs, this will go away. We can treat this as an adaptor interface from a config registry to an ingress registry.

kyessenov · 2017-02-02T20:05:27Z

platform/kube/controller.go

+// AppendIngressHandler notifies about changes to ingress resources
+func (c *Controller) AppendIngressHandler(f func(*v1beta1.Ingress, model.Event)) error {
+	c.ingresses.handler.append(func(obj interface{}, event model.Event) error {
+		f(obj.(*v1beta1.Ingress), event)


This is where you would need to convert k8s ingress to something platform-neutral.

kyessenov · 2017-02-02T20:16:21Z

proxy/envoy/ingress.go

+type ingressWatcher struct {
+	agent   Agent
+	mesh    *MeshConfig
+	ingress model.IngressRegistry


I think the idea is to merge ingress watcher and sidecar (+config) watcher since they are almost same. We can do that later, to parallelize work for now.

kyessenov · 2017-02-02T21:34:52Z

proxy/envoy/ingress.go

+	return []Host{
+		{
+			URL: fmt.Sprintf("tcp://%s.%s.svc.cluster.local:%d",
+				backend.ServiceName, namespace, backend.ServicePort.IntValue()),


Can we use SDS here? The service key for the cluster includes the port name, so you would need to fetch model.Service by hostname first, then find the port. That would work for both named and int ports.

kyessenov · 2017-02-02T22:22:00Z

proxy/envoy/ingress.go

+	clustersMap := make(map[string]Cluster)
+
+	// Function to add a rule for a particular host, path and backend.
+	// This will later be invoked for each ingress rule, as well as ingress default backend


We need to provide a service for a 404 backend. The service can be implemented by Envoy or default-http-backend from kubernetes.

This functionality is already exposed by k8s' ingresses, which allows the user to specify a default backend (service) for requests that don't match any ingress rule. This is implemented as an Envoy VirtualHost with a wildcard domain ("*"). If the user doesn't specify a default backend in its ingresses, then envoy we don't produce such wildcard VirtualHost, and fallback to Envoy's own 404.

kyessenov · 2017-02-02T22:23:06Z

proxy/envoy/ingress.go

+				Type:             "strict_dns", // TODO: Change to SDS
+				LbType:           DefaultLbType,
+				ConnectTimeoutMs: DefaultTimeoutMs,
+				Hosts:            clusterHosts(namespace, backend),


I'll try to refactor current config to make it more re-usable. It handles things like enabling http2.

kyessenov · 2017-02-02T22:25:06Z

proxy/envoy/ingress.go

+	}
+
+	// Iterate on all ingresses, and create corresponding rule
+	for _, ingress := range w.ingress.ListIngresses() {


Please make the order of rules or ingresses canonical. Otherwise, the watcher will keep reloading Envoy due to some permutation in the config file.

kyessenov · 2017-02-02T22:31:07Z

More comments for the TODOs:

Constructs envoy clusters using SDS + pod endpoints.
Yes, we should avoid using kube DNS since we have SDS. Don't worry about CDS for now, it's easy to refactor for that later.
Update ingress status with the proxy's "public" IP address (node IP?).
GCE LB ingress does this because it manages firewall rules and IP allocation. NGINX ingress does not AFAIK and you have to create a separate service of type LoadBalancerIP pointing to NGINX deployment. Let's do what NGINX ingress does.
TLS termination.
Don't worry about it. We will do one big swoop to enable TLS termination.
Testing.
Take a look at test/integration/driver.go. I think it has all the pieces to reuse.

zcahana · 2017-02-13T14:27:24Z

I've rewritten the ingress controller implementation such that Ingress API objects are converted to Istio model rules (currently, same config.RouteRule, but with a different kind - IngressRule), which are then transformed to Envoy configuration. There's still only a little re-use of existing Envoy configuration code. This can be refactored later when the package stabilizes and the common parts become clearer.

Note that due to the major changes that already went to master, I've rebased my branch and restructured the commits so that they're more reviewable. The commits from the first round are thus gone :).

zcahana · 2017-02-14T10:34:24Z

Jenkins' presubmit build quickly fails with the following error (link):

+ go get ./...
/var/jenkins_home/jobs/manager/jobs/presubmit/workspace/src/main@tmp/durable-3d3f8166/script.sh: 2: /var/jenkins_home/jobs/manager/jobs/presubmit/workspace/src/main@tmp/durable-3d3f8166/script.sh: go: not found
[Pipeline] }
[Pipeline] // dir
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline
ERROR: script returned exit code 127
Finished: FAILURE

Not sure what's causing this... any chance it's some recent changes to istio-testing repo? @sebastienvas

sebastienvas · 2017-02-15T05:39:31Z

Please rebase your PR.

istio-testing · 2017-02-15T08:29:52Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T08:34:02Z

Jenkins job manager/presubmit failed

istio-testing · 2017-02-15T12:03:42Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T12:09:20Z

Jenkins job manager/presubmit passed

zcahana · 2017-02-15T12:12:16Z

thanks @sebastienvas , build now passes.

rshriram

LGTM for first pass. Can you add a test script please? Don't bother with driver.go. If you could add this ingress spec to istio/istio#8 and verify that basic calls to bookinfo work (no need for automation. There are some bugs in the automation framework), then I am fine with this pr.

rshriram · 2017-02-15T14:02:35Z

platform/kube/conversion.go

+				// to the proxy configuration generator. This can be improved by using
+				// a dedicated model object for IngressRule (instead of reusing RouteRule),
+				// which exposes the necessary target port field within the "Route" field.
+				Tags: map[string]string{


I don't think we do that anymore.

I mean refer to ports in the route rule

This is necessary for envoy routes which are generated from ingresses - we need to route traffic to the cluster corresponding with the service:port defined by the user in the ingress.

Actually, looking further into this, cluster names include the port name, and not the port number.
So, what happens when there's no port name specified?

That can only happen if there is only one port, and the upstream cluster becomes just the service name ("a.svc....") or service-tag combination ("a.svc...::env=prod")

Can you add a TODO item here? This needs to be in the next iteration of routing rules, we've been assuming rules apply per port, but ingress needs an explicit port. Moreover, this is a port name, so this has some implication for the route rule model.

This seems to be captured in #151.

rshriram

Feel free to merge when you deem it fit. As it doesn't conflict with existing functionality. I would rather have the code merged sooner instead of rebasing again and again.

zcahana · 2017-02-15T14:34:58Z

There's at least one bugfix on the way, and potentially another one. I'm currently running/debugging this locally.

istio-testing · 2017-02-15T14:49:42Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T14:56:03Z

Jenkins job manager/presubmit passed

istio-testing · 2017-02-15T15:33:02Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T15:42:18Z

Jenkins job manager/presubmit passed

istio-testing · 2017-02-15T17:58:03Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T18:05:19Z

Jenkins job manager/presubmit passed

istio-testing · 2017-02-15T18:14:53Z

Jenkins job manager/presubmit started

istio-testing · 2017-02-15T18:28:30Z

Jenkins job manager/presubmit passed

kyessenov

LGTM
Feel free to merge when you feel we can have an example ingress working. We can revisit the route rule model which seems to be causing issues for you.

kyessenov · 2017-02-15T18:07:32Z

platform/kube/controller.go

@@ -298,6 +399,22 @@ func (c *Controller) List(kind, namespace string) (map[model.Key]proto.Message,
 	return out, errs
 }

+func (c *Controller) listIngresses(kind, namespace string) (map[model.Key]proto.Message, error) {


nit: not needed

What is it that's not needed here? The listIngresses method gets called from Controller.List(kind, namespace) according to the actual kind. In practice, this is being used by the envoy's ingress watcher.

Oops my bad, I was referring to "kind" parameter.

Well, you're right that the "kind" parameter isn't really needed here. I left it for uniformity with the other listX methods.

kyessenov · 2017-02-15T19:35:03Z

platform/kube/conversion.go

+				// to the proxy configuration generator. This can be improved by using
+				// a dedicated model object for IngressRule (instead of reusing RouteRule),
+				// which exposes the necessary target port field within the "Route" field.
+				Tags: map[string]string{


Can you add a TODO item here? This needs to be in the next iteration of routing rules, we've been assuming rules apply per port, but ingress needs an explicit port. Moreover, this is a port name, so this has some implication for the route rule model.

istio-testing · 2017-02-16T05:37:46Z

Jenkins job manager/presubmit passed

istio-testing · 2017-02-16T13:09:35Z

Jenkins job manager/presubmit passed

istio-testing · 2017-02-16T15:00:13Z

Jenkins job manager/presubmit passed

zcahana · 2017-02-16T15:07:12Z

With the latest round of commits, I've verified that basic ingress function works (i.e., ingress controller forwards ingress traffic to correct backend service according to defined routes). In practice, the backend service's reverse proxy is still blocking the request from hitting the actual service (due to #150), so this is still not usable end-to-end, but probably solid enough for first-pass --> merging.

elevran · 2017-02-23T13:40:15Z

Together with #177 - fixes #6

zcahana requested review from rshriram, kyessenov, ijsnellf and GregHanson February 2, 2017 13:24

rshriram changed the title ~~Envoy-based ingress controller~~ [WIP] Envoy-based ingress controller Feb 2, 2017

rshriram reviewed Feb 2, 2017

View reviewed changes

kyessenov reviewed Feb 2, 2017

View reviewed changes

rshriram mentioned this pull request Feb 6, 2017

Kubernetes Ingress Controller envoyproxy/envoy#311

Closed

zcahana force-pushed the ingress_controller branch from e564de2 to 28039ff Compare February 13, 2017 14:16

zcahana force-pushed the ingress_controller branch 2 times, most recently from 8175098 to 42ffc8b Compare February 14, 2017 10:01

zcahana force-pushed the ingress_controller branch from 42ffc8b to 21b459c Compare February 14, 2017 12:43

zcahana added 4 commits February 15, 2017 09:30

incorporate IngressRule config type into model

deb1027

convert k8s ingresses to IngressRule config

9a2eb7b

add ingress watcher for envoy configuration

a886476

add a 'manager ingress' command to launch the ingress controller

67c131b

zcahana force-pushed the ingress_controller branch from 21b459c to 67c131b Compare February 15, 2017 08:28

lint fixes

6eeb2c5

rshriram reviewed Feb 15, 2017

View reviewed changes

rshriram approved these changes Feb 15, 2017

View reviewed changes

make sure rule.Match.Http map gets created

756d80f

kyessenov approved these changes Feb 15, 2017

View reviewed changes

ijsnellf approved these changes Feb 15, 2017

View reviewed changes

use port name for cluster name from ingress rule

73d9384

Merge branch 'master' into ingress_controller

627a170

zcahana changed the title ~~[WIP] Envoy-based ingress controller~~ Envoy-based ingress controller Feb 16, 2017

zcahana merged commit 194066a into istio:master Feb 16, 2017

zcahana deleted the ingress_controller branch February 16, 2017 15:07

zcahana mentioned this pull request Feb 16, 2017

Generate envoy virtual host entry for pod IP and port #150

Closed

elevran mentioned this pull request Feb 23, 2017

Implement Ingress Controller for Envoy #6

Closed

elevran mentioned this pull request Mar 22, 2017

Need Envoy based ingress controller for kubernetes amalgam8/amalgam8#483

Closed

Envoy-based ingress controller #71

Envoy-based ingress controller #71

Conversation

zcahana commented Feb 2, 2017 • edited Loading

googlebot commented Feb 2, 2017

rshriram commented Feb 2, 2017

zcahana commented Feb 2, 2017

googlebot commented Feb 2, 2017

rshriram left a comment • 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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyessenov 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyessenov commented Feb 2, 2017

zcahana commented Feb 13, 2017

zcahana commented Feb 14, 2017

sebastienvas commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

zcahana commented Feb 15, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram left a comment

Choose a reason for hiding this comment

zcahana commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

istio-testing commented Feb 15, 2017

zcahana commented Feb 2, 2017 •

edited

Loading

rshriram left a comment •

edited

Loading