Istio Mesh LB/Gateway + RouteRule v1alpha2 #254

rshriram · 2017-11-22T04:47:24Z

This proposal suggests a new Gateway configuration resource that describes a set of ports that should be exposed bridging two networks. A Gateway can act as an ingress allowing external traffic into the mesh and it can act as an egress allow traffic in the mesh to exit. A Gateway can also be used to model a proxy that is entirely internal to the mesh and implemented either by sidecars or by a middle proxy. Behaviors for the ports like supported protocols, TLS, SNI etc are described by the resource.

Users can then use standard Istio RouteRules to control HTTP requests & TCP traffic entering a Gateway by binding the gateway to a route rule. The route rule will be applied at the gateway if and only if the rule’s destination (or one of its domain aliases) corresponds to one of the wildcard domains supported by the Gateway.

Note that this is not the same as the recent proposals for multiple zones that advocate for a new access model (direct addressing of services across platforms). These proposals target users who fully buy into Istio and are willing to modify their applications, while the proposal here targets the more immediate need - deployments that already exist.

Detailed discussion can be found in https://goo.gl/96UTKQ

frankbu · 2017-11-22T15:19:07Z

proxy/v1/config/gateway.proto

+// protocol to use, SNI configuration for the load balancer, etc.
+//
+// For example, the following gateway spec sets up Envoy to act as a load
+// balancer exposing port 80 (http), 443 (https), and port 2379 (TCP).


Also port 9080?

frankbu · 2017-11-22T15:26:44Z

proxy/v1/config/gateway.proto

+//         clientCABundle: client.ca-bundle
+//
+// The following route rule forwards http requests for
+// uk.bookinfo.com/reviews, eu.bookinfo.com/reviews to the reviews service.


Which port on the reviews service?

Requests to the gateway port 80 redirects to https, right?
Requests to the gateway port 443 go where?
Requests to the gateway port 9080 go where?

In the actual Bookinfo example, we want gateway port 80 to forward to reviews:9080, right? I don't see anything in the config that specifies that. Maybe it's just a matter of making the example consistent end-to-end?

Clarified.. I think the internal port is implicit. IOW, if reviews has only one HTTP port, it goes to that port, irrespective of that port. But if reviews has multiple HTTP ports, then the behavior is undefined. The undefined behavior is a bug and we need to fix. cc @ZackButcher @kyessenov

frankbu · 2017-11-22T15:28:07Z

proxy/v1/config/gateway.proto

+//             name: redis.prod
+//
+// By default, if there is no wildcard, HTTP requests for unknown domains
+// or requests that have no matching route rule will be responded with a


s/be responded with/respond with/

kyessenov · 2017-11-22T16:44:24Z

proxy/v1/config/gateway.proto

+//           route:
+//           - destination:
+//               port:
+//                 number: 7777


Does this redirect all ports (80, 443) to 7777/

Yep. 80 to 443. 443, 9080 to 7777

Can you please add source port selection then? This seems rather uncommon example to conflate all ports into one.

Why is this strange? The service port need not be the same as target port right? I agree that this example seems contrived, but it was there to illustrate the fact that you can have plaintext http ports in teh gateway.

kyessenov · 2017-11-22T16:45:07Z

proxy/v1/config/gateway.proto

+//       name: bookinfo-rule
+//     spec:
+//       destination:
+//         name: reviews.prod


Is this meant to uniquely identify a service, or just match addressing? E.g. if client addresses through reviews, does this rule kick in?

Good question. I think this should not apply if client simply says reviews.

That would be counter intuitive. DNS resolution is identical for reviews and reviews.prod under the same context, so the existing application may use either of those names. You would need to enumerate all choices here:

destination: - name: reviews - name: reviews.prod - name: reviews.prod.svc - name: reviews.prod.svc.cluster - name: reviews.prod.svc.cluster.local - name: uk.bookinfo.com - name: eu.bookinfo.com

Then the question still remains that reviews is actually different in another namespace:

destination: - name: reviews - name: reviews.qa

Yes and this is what I was trying to avoid. I just discovered few more ambiguities here. Per @louiscryan 's PR,
if we infer the namespace from the namespace the gateway is running on, then we could get away with just reviews. That said, inheriting the top level destination into route.destination seems intuitive if and only if we are talking about just internal routing. From a gateway's perspective, it definitely needs some backend. i.e.

destination: domains: - uk.bookinfo.com http: route: destination: name: reviews.prod rewrite: uri: /foo

works but

destination: domains: - uk.bookinfo.com http: route: rewrite: uri: /foo

does not work, eventhough its a valid config. IF we decide to consider the above config as invalid, then internal route rules will become more verbose, i.e.

destination: name: reviews http: route: destination: name: reviews rewrite: uri: /foo

A similar complication arises when we start using destination.port.

destination: name: reviews domains: - uk.bookinfo.com port: - 6666 http: route: destination: reviews.prod

In the above rule, does 6666 apply to the gateway port or to the reviews service's port? If I want to write a rule1 against gateway port 9080 and a rule2 against gateway port 80, I need to make sure that I don't co mingle the rule with internal service names as the internal service might be using that port for some other purpose.

Thoughts? @ZackButcher / @louiscryan / @kyessenov ?
One option is to restrict the destination to have either name or domains. Another is to enforce the constraint that if destination has only domains, route.destination is mandatory. Along with these, we need to impose the constraint that port cannot be used when both name and domains are used in the destination.
This seems rather clunky.

To clarify.

I think the match should occur on 'addressing' but we can decide to be permissive (destination name contextualized into FQDN matches application provided name contextualized into FQDN) or strict (names must match explicitly). I strongly suspect that permissive is pretty much what folks want universally.

'Destinations' are always internal though this can cover public hosts & IPs.

Binding a route rule to a gateway with no default destination is likely to be a mistake but it might not be worthwhile to reject it for the marginal value such validation would provide.

A route rule with no destination at all is definitely invalid, regardless of whether it's bound to a gateway or not.

I don't get why destination port is a problem, its just a refinement on the internal destination selection.

kyessenov · 2017-11-22T16:47:49Z

proxy/v1/config/gateway.proto

+  // services using TLS with SNI. Standard DNS wildcard prefix syntax
+  // is permitted.
+  //
+  // RouteRules that are bound to a gateway must having a matching domain


What's the purpose of this constraints? Can we inherit all domains from the gateway?

Multiple route rules for the same gateway server?

Assume gateway wants multiple route rules. Assume some of those rules care about the host header. Why not add a HTTP match condition on URI then?

But what part of the gateway? It defines multiple virtual hosts on different ports.

kyessenov · 2017-11-22T16:50:46Z

proxy/v1/config/gateway.proto

+message TLSConfig {
+  // REQUIRED: A set of hostnames associated with the server
+  // certificate. For wildcard hostname, use *
+  repeated string domains = 1;


How do we associate a TLS cert with a server listener? Based on domain suffix match? Since this is already a reference to a cert, should we put that reference into the server instead?

Can’t the same cert be applicable across servers?

Normally, one domain name = one cert. But you can have a wildcard cert for multiple listeners. I don't understand the benefit of the extra indirection listener -> TLS config -> certs. Why not just listener (server) -> certs.

I simplified it.. PTAL

As I'm sitting down and writing code to generate listeners from Gateways, having the TLS config embedded in the server is way more convenient. I like this improvement.

kyessenov · 2017-11-22T16:53:31Z

proxy/v1/config/gateway.proto

+//       destination:
+//         name: reviews.prod
+//         domains:
+//           - uk.bookinfo.com


It seems like this is capturing two names reviews.prod.svc.cluster.local and eu.bookinfo.com. What if we make destination an array and match on EITHER of name field values?

That is not a bad idea. The only thing is to interpolate namespace. Also destination needs to be renamed to hosts for clarity. There are too many destinations.

Destination is a mixer attribute namespace. We want consistent vocabulary. I'm fine either way, but then mixer needs to adopt host as the primary attribute space.

Destinations are names inside of the mesh, domains are names outside of the mesh. I think the distinction is important if we're talking about gateways bridging the external and internal worlds. If we conflate internal and external names I think it'll wind up more confusing than it needs to be (do I write a RouteRule for destination reviews.prod.svc.cluster.local, or eu.bookinfo.com? Are both valid?).

What this is actually doing is not capturing two names; it's capturing that Host: eu.bookinfo.com (an external name) routes to the service reviews.prod (an internal name).

kyessenov · 2017-11-22T16:53:41Z

proxy/v1/config/gateway.proto

+//           - uk.bookinfo.com
+//           - eu.bookinfo.com
+//       gateway:
+//         - my-gateway #apply at my-gateway as well as reviews.prod internally


What does it mean to apply a gateway in a rule?

That the rule selects itself to apply at the gateway. I am conflicted about this. @louiscryan seemed to think that this option was better than gateway selecting the rules. It seemed fine until I started describing the model.

Is this source matching? What about the original source match clause?

No, all this is doing is instructing Pilot to include these route rules in the config of ingress proxies exposing Gateway "my-gateway". From there, we use the domain to map from the external name to a destination with an internal name.

Here's how IMO things should work:
Gateway a exposes domains eu.bookinfo.com and us.bookinfo.com. Gateway b exposes us.bookinfo.com.

Then we define a RouteRule like:

metadata: name: route-reviews spec: destination: name: reviews.prod domains: - eu.bookinfo.com - us.bookinfo.com gateways: - a ...

A request with Host: us.bookinfo.com that hits Gateway a will be routed according to the rule route-reviews. The same request hitting Gateway b results in a 404 because Gateway b has no routes for us.bookinfo.com; route-reviews only applies to Gateway a. A request inside of the mesh with a destination.service == reviews.prod has destination selected according the route-reviews as normal, the Gateway portion doesn't come in to play.

louiscryan · 2017-11-27T21:59:06Z

proxy/v1/config/gateway.proto

+//       name: my-gateway
+//     spec:
+//       servers:
+//         http:


Why declare protocol at this level? To facilitate some API pattern.

Most properties at the gateway level are common among protocols unlike routing rules.

That was a leftover from old version.. removing..

rshriram · 2017-11-28T21:10:38Z

@louiscryan / @ZackButcher / @kyessenov / @frankbu I have moved the documentation to a separate temp file, as it was really talking about the routing rules, and not about the gateway. It seems that we are all in agreement about the gateway spec itself. So in the interest of moving this forward, do you guys have any concerns about the gateway spec? If not, we should merge and then iterate on the concerns for routing in subsequent PRs for routing..

kyessenov · 2017-11-28T21:18:23Z

routing/v1alpha2/gateway.proto.withcomments

@@ -0,0 +1,268 @@
+// Copyright 2017 Istio Authors


redundant file?

Its intentional.. I just wanted to retain the comments, so that I don't forget the comments. But this won't be merged in, until the routing is refined..

kyessenov · 2017-11-28T21:19:27Z

routing/v1alpha2/gateway.proto

+//
+message Gateway {
+  // REQUIRED: A list of server specifications.
+  repeated Server servers = 1;


What is the reason to not make "server" by the top-level config unit?
Logically, the configuration state is subdivided by domains, but it's three levels nested in the Gateway.

Don't you want to refer to the gateway as a whole by its name, in the routing rule ?

Are we assigning gateways to edge proxies? If all proxies run all gateways, then I want to bind a rule to a domain, not a gateway.

Don't we want to assign gateways to proxies? For e.g., I won't want to expose the set of servers exposed by my internal gateway to the external gateway.

Yup, I think we'll have use cases for tying gateways to specific sets of proxies.

What's the model for binding gateways to proxies? Can we bind rules to those proxies instead of gateways?

kyessenov · 2017-11-28T21:20:57Z

I'm not sure what to make of it. It's unclear how traffic should be routed from the gateway without any destination or routing rule support.

louiscryan · 2017-12-11T23:30:01Z

routing/v1alpha2/route_rule.proto

+// *Source* - A downstream client calling a service.
+//
+// *Host* - The address used by a client when attempting to connect to a
+// *service.


louiscryan · 2017-12-11T23:52:18Z

LGTM

kyessenov · 2017-12-11T23:59:46Z

routing/v1alpha2/gateway.proto

+//       name: bookinfo-rule
+//     spec:
+//       hosts:
+//       - reviews.prod


How does this work for non-namespaced components (e.g. cloud-provided ingress controllers)?

I'm not quite sure if you're asking about? The gateway in totality or just namespaced vs non-namespaced domains ?

There seems to be some implicit DNS context depending where the rule is applied. Since some k8s components run out of cluster, we want to make sure the rule can apply there too.

kyessenov · 2017-12-12T00:00:26Z

routing/v1alpha2/gateway.proto

+// with a wildcard host and the desired backend. For example, the following
+// wildcard routing rule routes all traffic to homepage.prod by default.
+//
+//     metadata:


missing kind and version

kyessenov · 2017-12-12T00:00:53Z

routing/v1alpha2/gateway.proto

+//     spec:
+//       servers:
+//       - port:
+//           number: 80


out of date example

ZackButcher

LGTM. We should rationalize how we describe ports across our config, but IMO that doesn't block this PR.

ZackButcher · 2017-12-11T23:34:37Z

routing/v1alpha2/route_rule.proto

+
+  // The names of gateways that should apply these routes. A single route
+  // rule could be used for sources inside the mesh as well as one or more
+  // gateways. This selection condition imposed by this field is


ZackButcher · 2017-12-11T23:56:20Z

routing/v1alpha2/gateway.proto

+//
+message Server {
+  // Port describes the properties of a specific port of a service.
+  message Port {


We really need to rationalize our usage of port, the current egress rule's port definition is a subset of this.

api/routing/v1alpha1/egress_rule.proto

Lines 47 to 53 in cda3855

message Port {

// A valid non-negative integer port number.

int32 port = 1;

// The protocol to communicate with the external services.

// MUST BE one of HTTP|HTTPS|GRPC|HTTP2.

string protocol = 2;

}

Not blocking, but in a follow on we should extract this port definition out and share it across rules.

I think at most we need two definitions: a source port definition (here) and a destination port definition (used in egress, and in a few other places in this PR, typically in the form of a oneof).

I tried doing this earlier. It’s not clean. Egress rule port is a spec. Same as gateway. But route rule port is a match. So some fields like protocol don’t make sense.

ZackButcher · 2017-12-11T23:57:41Z

routing/v1alpha2/route_rule.proto

+  // collide with numbers. If there are multiple ports on a service with
+  // the same protocol the names should be of the form <protocol-name>-<DNS
+  // label>.
+  oneof destination_port {


Same as above, why can't we just extract out and re-use a single Port message. Having this many ways to specify the same concept is going to cause user confusion and inconsistencies in our implementations.

kyessenov · 2017-12-12T00:04:15Z

routing/v1alpha2/route_rule.proto

+//               version: v1
+//           weight: 100
+//
+// A host name can be defined by only one RouteRule.  A single route rule


So in this example, which reviews service does this rule apply to, if there are multiple namespaces with reviews service?

local namespace of the pod where this rule is being applied.

I think the comment needs some refinement for sure but its fair to start with the requirement that the host name is unique and relax it later.

If a name is unqualified we can try to qualify it in context (i.e namespace) before enforcing the uniqueness rule

kyessenov · 2017-12-12T00:06:14Z

routing/v1alpha2/route_rule.proto

+  // referring to "reviews" would be resolved to the FQDN
+  // "reviews.default.svc.cluster.local". However, a different name such as
+  // "reviews.sales" would be treated as a FQDN during virtual host
+  // matching.  In Consul, a plain service name would be resolved to the


I don't understand this explanation. Do you mean I have to use reviews.sales.svc.cluster.local as FQDN? What happens if I move my deployment to a different cluster suffix?

kyessenov · 2017-12-12T00:07:04Z

routing/v1alpha2/route_rule.proto

+  // is possible to define a route rule that applies to both the gateway
+  // and a specific internal workload by specifying gateways as well as the
+  // source match condition in HTTP/TCP routes.
+  repeated string gateways = 2;


Are gateways namespaced? All Istio config resources are currently namespaced

Didn't think of it. We could I guess?

Yes.

That probably begs the question of how to refer to a gateway in another namespace but I think that can be in the string if necessary. E.g. "namespace/gateway", if we need to disambiguate.

kyessenov · 2017-12-12T00:08:59Z

routing/v1alpha2/route_rule.proto

+// RouteRule for usage examples.
+message Destination {
+  // REQUIRED: Service name from the service registry. The name can be a
+  // short name, a fully qualified domain name or an IP address. If short


Define three fields for each type of naming?

With one of?

Don't think that would yield any usability benefit.

kyessenov · 2017-12-12T00:12:37Z

By the way, did you check if k8s allows multiple versions per CRD? Last time I checked, it was only one version supported. That means we need to define conversion function from alpha1 to alpha2.

louiscryan · 2017-12-12T04:24:16Z

routing/v1alpha2/gateway.proto

@@ -93,39 +93,42 @@ package istio.routing.v1alpha2;
 //               user: dev-123
 //         route:
 //         - destination:
-//             port: 7777
+//             port:


I don't think the oneof will yield that output

louiscryan · 2017-12-12T04:25:38Z

routing/v1alpha2/route_rule.proto

@@ -721,3 +713,13 @@ message HTTPFaultInjection {
  }
 }

+// PortSelector specifies the name or number of a port to be used for


Why this type as opposed to the inline oneof ? It yields funky YAML

@ZackButcher had this comment that we have the same one off everywhere.. repetition and chance of mistakes, etc.

The yaml should look like:

port: name: foo

and

port: number: 7

Where the "port" in those snippets is the name of the PortSelector field in the parent message. This is exactly what we want, I think.

rshriram · 2017-12-12T17:30:22Z

On Mon, Dec 11, 2017 at 9:08 PM, Sven Mawson ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In routing/v1alpha2/gateway.proto <#254 (comment)>: > + // Port describes the properties of a specific port of a service. + message Port { + // A valid non-negative integer port number. + int32 number = 1; + + // The protocol to communicate with the external services. + // MUST BE one of HTTP|HTTPS|GRPC|HTTP2|MONGO|REDIS|TCP. + string protocol = 2; + + // Name assigned to the port. + string name = 3; + } + + // REQUIRED: The Port on which Envoy should listen for incoming + // connections + Port port = 1; The benefit is sharing of domains between ports on a server, and depending on your perspective a more natural model (a server is a set of domains, ports are opened for the server, ports have TLS settings). The general principle is that this is a form of data normalization, our goal is to minimize the amount of information users have to tell us in order to get their job done, and especially minimize how often they repeat themselves (DRY). Requiring the same set of domains be repeated multiple times vs. allowing multiple ports per server increases the amount of configuration needed for no clear gain. As the set of information that ends up repeated grows, users get increasingly annoyed, and eventually they write their own config layer on top that generates our layer. I've seen this more times than I'd like to admit (since it was my layer making them repeat themselves). I can't find this comment on GH.. Are you saying that instead of

organizing by ports, we should organize by domains? I am not opposed to this. I didn't have data on what is the more common format. I thought that something like a port 80 with 10s of domains and an odd man out on port 9080, is a more common thing. On the general comment, I totally agree. That was the reason I introduced some iterator style functionality into the route rule that allows people to specify multiple matches, multiple sources, etc. There is still a bit more left to do there, but as with other things in routing, I would like to drive it by use cases.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#254 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH0qdyBcCTPt5wJ9kzPBfJpiQJ6dMFquks5s_eAbgaJpZM4Qm21p> .

shalako · 2017-12-12T19:40:12Z

routing/v1alpha2/gateway.proto

+//         tls:
+//           mode: simple #enables HTTPS on this port
+//           serverCert: server.crt
+//           clientCABundle: client.ca-bundle


Does this refer to CAs Envoy will use to validate trust of client certificates?

shalako · 2017-12-12T19:55:42Z

routing/v1alpha2/gateway.proto

+
+// Server describes the properties of the proxy on a given load balancer port.
+// For example,
+//     metadata:


kind and version

shalako · 2017-12-12T20:22:58Z

routing/v1alpha2/gateway.proto

+  // is permitted.
+  //
+  // RouteRules that are bound to a gateway must having a matching domain
+  // in their default destination. Specifically one of the route rule


How is the default destination determined? Is it one of, or the first domain listed in the array of hosts for the RouteRule?

shalako · 2017-12-12T20:23:59Z

routing/v1alpha2/gateway.proto

+  // services using TLS with SNI. Standard DNS wildcard prefix syntax
+  // is permitted.
+  //
+  // RouteRules that are bound to a gateway must having a matching domain


What happens if a RouteRule is "bound" to a gateway but doesn't have a matching domain? How does an operator discover a misconfiguration?

aaronshurley · 2017-12-13T20:05:31Z

Evaluating for CloudFoundry Usage:

It's a bit early in our development stages to have full confidence on how this change will affect our implementation. It doesn't raise any immediate concerns and will potentially simplify our configurations, so we approve. Our expectations for our early work will probably be to statically configure the Envoy listeners instead of using the LDS from Pilot (still using RDS, CDS, and SDS). If we do use LDS in the future, it will certainly be nice to configure listeners separately from route rules, which the gateway resource enables.

Gateway spec

f229176

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 22, 2017

istio deleted a comment from istio-testing Nov 22, 2017

This was referenced Nov 22, 2017

Proposal: add ingress exposure as part of the routing rule #192

Closed

[WIP] Gateway (standalone proxy) configuration #227

Closed

frankbu reviewed Nov 22, 2017

View reviewed changes

Shriram Rajagopalan added 3 commits November 22, 2017 11:23

feedback

477e091

pb.go

12d5503

other pb.go

8be0394

kyessenov reviewed Nov 22, 2017

View reviewed changes

louiscryan reviewed Nov 27, 2017

View reviewed changes

Shriram Rajagopalan added 8 commits November 27, 2017 21:10

updates

eb7b9db

temp del

151643f

more del

12cecd4

Merge branch 'master' into gateway

408c610

v1alpha2

8afc752

Merge branch 'master' into gateway

5e00253

gateway.pb.go

e3b9f42

move docs to a different temp file

fe5aa0d

kyessenov reviewed Nov 28, 2017

View reviewed changes

nits

4ab77c7

frankbu approved these changes Dec 11, 2017

View reviewed changes

louiscryan reviewed Dec 11, 2017

View reviewed changes

louiscryan approved these changes Dec 11, 2017

View reviewed changes

kyessenov reviewed Dec 11, 2017

View reviewed changes

kyessenov reviewed Dec 12, 2017

View reviewed changes

ZackButcher approved these changes Dec 12, 2017

View reviewed changes

kyessenov reviewed Dec 12, 2017

View reviewed changes

Shriram Rajagopalan added 3 commits December 11, 2017 21:56

nits

9889cf8

nits

003bba2

nits

9713339

rshriram merged commit 7eada33 into master Dec 12, 2017

louiscryan reviewed Dec 12, 2017

View reviewed changes

shalako reviewed Dec 12, 2017

View reviewed changes

rshriram deleted the gateway branch January 5, 2018 04:24

This was referenced Jan 26, 2018

Gateway for Cloud Foundry edge ingress routing istio/istio#2867

Closed

Gateway mode should support Cloud Foundry ingress routing istio/istio#2812

Closed

incfly pushed a commit to incfly/api that referenced this pull request Jun 13, 2018

change platform to environment (istio#254)

29b4b04

	message Port {
	// A valid non-negative integer port number.
	int32 port = 1;
	// The protocol to communicate with the external services.
	// MUST BE one of HTTP\|HTTPS\|GRPC\|HTTP2.
	string protocol = 2;
	}

Istio Mesh LB/Gateway + RouteRule v1alpha2 #254

Istio Mesh LB/Gateway + RouteRule v1alpha2 #254

Uh oh!

Conversation

rshriram commented Nov 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZackButcher Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rshriram commented Nov 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyessenov Nov 22, 2017 •

edited

Loading

ZackButcher Nov 27, 2017 •

edited

Loading

kyessenov Nov 28, 2017 •

edited

Loading