Add property machine.docker.local_node_host.external #2402

l0rd · 2016-09-09T15:45:24Z

What does this PR do?

Introduce a new property, machine.docker.local_node_host.external (along with the environment variable CHE_DOCKER_MACHINE_HOST_EXTERNAL), that allow to configure the external URL browsers need to use to connect with containers that are not directly pingable. This happens for example with Docker for Mac.

machine.docker.local_node_host.external=localhost

When this property is set the browser will use it to connect to the containers. Note that the wsmaster won't use that value to connect to the containers.

If this property is not set Che behaves as before.

This PR introduce a change to the API: Server class has a couple of new attributes: internalUrl and internalAddress. On the other there no particular change is needed client side and all clients will benefit of this change.

"servers": {
      "4401/tcp": {
                "url": "http://localhost:32906/wsagent/ext", 
                "internalUrl": "http://192.168.65.2:32906/wsagent/ext", 
                "address": "localhost:32906",
                "internalAddress": "192.168.65.2:32906",
                "path": "/wsagent/ext",
                "protocol": "http",
                "ref": "wsagent"
     }
}

This PR takes into account some feedbacks from PR #2004:

There are no changes on client side
Works on configurations where the wsagent and docker containers are on different hosts

I will not close PR #2004. I want to continue the work to integrate Traefik in the che-launcher there. But traefik/traefik#444 should be fixed first.

What issues does this PR fix or reference?

#1644

Previous behavior

Running Che on Docker for Mac required some networking hacking

New behavior

Remove the need for the networking hack when running Che on Docker for Mac and make running Che as simple as docker run -v /var/run/docker.sock:/var/run/docker.sock codenvy/che-launcher start on all platforms.

PR type

Minor change = no change to existing features or docs
Major change = changes existing features or docs

Minor change checklist

New API required?
API updated
Tests updated
Tests passed

codenvy-ci · 2016-09-09T15:49:02Z

Can one of the admins verify this patch?

TylerJewell · 2016-09-09T19:18:38Z

Nice work! Excited to have this get into master. I reviewed the changes for the che-launcher and they all look good.

TylerJewell · 2016-09-09T22:45:54Z

@garagatyi - this resolves a big inconvenience that users have with Docker for Mac. This eliminates a painful setup of the loopback alias on Macs. Your +1 here will be helpful.

garagatyi · 2016-09-10T21:23:36Z

In my opinion changing API for that workaround is not a good way to go. It will add pain sooner or later.
I've just figured out that we can try to play with IPTables in Che container.
For example we can try to redirect traffic inside of Che container from 32000-xxxxx to IP of xhyve machine. And let expose machines ports as localhost. In that case browser will be able to connect to machines on localhost and Che-server will use localhost also. But IPTables rules in che-server container will forward all traffic to another IP.
As I understand we can do that even after start of che-server, so we will be able to do that in che launcher and not change base che-server which works fine in all cases except che in container on mac.
Here is an example that I googled:
iptables -t nat -A OUTPUT -i lo -p tcp --dport 32000:65000 -j DNAT --to <IP OF XHYVE>:32000-65000
But I didn't try it and have no experience with IPTables, so not sure that I found correct (and complete) set of rules for out case.

l0rd · 2016-09-10T22:23:38Z

@garagatyi can you please be more specific? Where do you want to run this iptable command: OSX, xyve VM, che-server container, agent container? What do you mean when you say "let expose machines ports as localhost"?

garagatyi · 2016-09-11T05:14:22Z

Add iptables rules in che-server container.

Configure che-server in such a way that localhost is host in servers provided by Che API.
I think we just should not set CHE_DOCKER_MACHINE_HOST variable to achieve that.

So all traffic from che-server container to localhost with ports 32000-xxxxx will go through IPTables redirection to IP of xhyve where ports of all containers are available.

l0rd · 2016-09-11T20:26:55Z

Ok thanks for clarifying @garagatyi. I understand now. It's a smart solution :-) I will test that tomorrow.

l0rd · 2016-09-12T15:58:50Z

Bingo! I've struggled to apply the IP tables rules but I've finally found the following command that work if executed inside che-sever container!

apk update
apk install iptables
sysctl -w net.ipv4.conf.eth0.route_localnet=1
iptables -t nat -A OUTPUT -p tcp -d localhost -o lo --dport 32000:65000 -j DNAT --to ${XHYVE_VM_IP}:32000-65000
iptables -t nat -A POSTROUTING -o eth0 -m addrtype --src-type LOCAL --dst-type UNICAST -j MASQUERADE

l0rd · 2016-09-12T16:19:49Z

I've created a new PR to address the Docker for Mac problem: eclipse-che/che-dockerfiles#12

@garagatyi I will let you choose which one of these 2 PR is more suitable but let me explain why I believe we should keep this one and close eclipse-che/che-dockerfiles#12. Apart some concerns with the iptables solution I also think that changing the Server class, adding an internal url attribute, is not just a patch to solve the Docker for Mac issue but a solution for a recurring problem.

In the current model there is only one address to reach the wsagent server. That same address is used by the browser and the wsmaster. In the diagram I've called this address "Post Office". Depending on the platform the "Post Office" can be docker0, the IP of the docker host or just localhost. However in most scenarios wsmaster and wsagent are close and there is a shorter path to reach the wsagent (the red path in the diagram). Most important this red path avoid some scary mountains that can be tricky to cross (firewalls, routers, IPtables etc...). That's the case with Docker for Mac but that's also the case with Ubuntu (#2030) or Kubernetes or Openshift etc...This PR is to add the address of this red path to the attributes of the Server class.

TylerJewell · 2016-09-12T16:44:23Z

I tend to agree with Mario that having the internal sokution is more optimal for the longer term. And that we can continue to layer in the iptables solution where necessary.

Great graphic :)

l0rd · 2016-09-14T16:52:29Z

I've updated the PR. I've added a new ServerProperties class where I moved servers path, internalUrl and internalAddress fields. That's the new Server API:

"servers": {
    "4401/tcp": {
        "ref": "wsagent",
        "protocol": "http",
        "address": "localhost:32929",
        "url": "http://localhost:32929/wsagent/ext",
        "port": "32929",
        "properties": {
            "internalUrl": "http://192.168.65.2:32929/wsagent/ext",
            "internalAddress": "192.168.65.2:32929",
            "path": "/wsagent/ext"
        }
    }
}

@garagatyi please review

benoitf · 2016-09-14T16:55:57Z

I like it also
I would say that timeframe for updating json format as we're in process for 5.x release and still in milestone,is the right time. After it would be too late

TylerJewell · 2016-09-14T17:28:51Z

I am ok with this approach. I want to target this for M2 - solving this problem simplifies a lot of scenarios for us with Mac.

garagatyi · 2016-09-15T07:22:58Z

...he-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/ServerProperties.java

+     * Internal address of the server in form <b>host:port</b>
+     * Used by wsmaster to communicate with the server
+     */
+    String getInternalAddress();


Looks like all fields in that object are nullable.

garagatyi · 2016-09-15T07:26:43Z

...he-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/ServerProperties.java

+    String getPath();
+
+    /**
+     * Internal address of the server in form <b>host:port</b>


Line break by entering 'enter' key doesn't work in javadocs. Please add dot and  or  

garagatyi · 2016-09-15T07:27:01Z

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/Server.java

@@ -25,25 +25,35 @@
    String getRef();

    /**
-     * Address of the server in form <b>host:port</b>
+     * External address of the server in form <b>host:port</b>


Line break by entering 'enter' key doesn't work in javadocs. Please add dot and  or  

garagatyi · 2016-09-15T07:28:23Z

...e-core-ide-api/src/main/java/org/eclipse/che/ide/api/machine/DevMachineServerProperties.java

+import org.eclipse.che.api.core.model.machine.ServerProperties;
+
+/**
+ * Describe development machine server instance.


Line break by entering 'enter' key doesn't work in javadocs. Please add  or  

garagatyi · 2016-09-15T07:33:59Z

...r-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfo.java

    private final Map<String, ServerConfImpl> serversConf;

    @Inject
    public DockerInstanceRuntimeInfo(@Assisted ContainerInfo containerInfo,
-                                     @Assisted String containerHost,
+                                     @Assisted String containerExternalHostname,


Not named assisted injection doesn't work when several arguments have the same class

Also please check what will happen if someone provide null value as one of these parameters. I believe nullable annotation should be declared somewhere.

I've changed to named assisted injection and checked if containerExternalHostname is null and set it to containerHost in that case.

garagatyi · 2016-09-15T08:00:40Z

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/Server.java

     */
    String getAddress();

    /**
+     * Port of the server, e.g. 8080


I'm not sure should this port contain 8080 or 8080/tcp.

And it is external port. Please explain that in javadocs.

It's 8080, the protocol is not there:

"port": "32929"

garagatyi · 2016-09-15T08:03:20Z

ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/machine/DevMachineServer.java

+        public String getPort() { return port; }
+
+        @Override
+        public ServerProperties getProperties() { return properties; };


Please add equals/hashcode/toString methods

garagatyi · 2016-09-15T08:03:36Z

...e-core-ide-api/src/main/java/org/eclipse/che/ide/api/machine/DevMachineServerProperties.java

+    public String getInternalUrl() {
+        return internalUrl;
+    }
+}


Please add equals/hashcode/toString methods

garagatyi · 2016-09-15T08:07:01Z

...e-core-ide-api/src/main/java/org/eclipse/che/ide/api/machine/DevMachineServerProperties.java

+    private final String internalAddress;
+    private final String internalUrl;
+
+    public DevMachineServerProperties(ServerProperties dto) {


Argument doesn't have to be DTO object. Please rename parameter to not confuse developers.

garagatyi · 2016-09-15T08:08:04Z

...e-core-ide-api/src/main/java/org/eclipse/che/ide/api/machine/DevMachineServerProperties.java

+
+/**
+ * Describe development machine server instance.
+ * {@link ServerProperties}


Curly brackets are used when link is inlined into description. When it is used as a separate tag it has the same syntax as @author

garagatyi · 2016-09-15T08:10:07Z

...r-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfo.java

    private final Map<String, ServerConfImpl> serversConf;

    @Inject
    public DockerInstanceRuntimeInfo(@Assisted ContainerInfo containerInfo,
-                                     @Assisted String containerHost,
+                                     @Assisted String containerExternalHostname,


Also please check what will happen if someone provide null value as one of these parameters. I believe nullable annotation should be declared somewhere.

garagatyi

Overall solution is good. Code requires some cleanups and fixes.

garagatyi · 2016-09-15T08:24:42Z

...r-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfo.java

                                                     null,
-                                                     null));
+                                                     externalPort,
+                                                     new ServerPropertiesImpl(null, internalHostname + ":" + externalPort, null)));


Please ensure that null value won't be treated as string "null"

After fixup externalHostname, internalHostname and externalPorts can never be null.

garagatyi · 2016-09-15T08:44:40Z

...rc/main/java/org/eclipse/che/plugin/docker/machine/local/LocalDockerInstanceRuntimeInfo.java

-              dockerNodeHost != null ? dockerNodeHost : (System.getenv(CHE_DOCKER_MACHINE_HOST) != null ?
-                                                         System.getenv(CHE_DOCKER_MACHINE_HOST) :
-                                                         containerHost),
+              dockerNodeExternalHostname != null ? dockerNodeInternalHostname : (


I believe that new policy of environment variables vs property usage in Che is that environment variable has priority over property.

Fixed. Env variables are now evaluated first.

garagatyi · 2016-09-15T08:50:18Z

...chine/src/test/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfoTest.java

+                                                       "32103",
+                                                       new ServerPropertiesImpl("some/path4",
+                                                           CONTAINER_HOST + ":32103",
+                                                           null)));


Please add tests for all new use cases, e.g. internal/external addresses are configired with properties, env variables, both null, both not-null, some is null other not.

Added a couple of tests

garagatyi · 2016-09-15T08:54:26Z

...he/ide/extension/machine/client/perspective/widgets/machine/appliance/server/ServerTest.java

+        verify(descriptor).getProperties();
+        verify(descriptor2).getInternalUrl();
+    }
+
 }


Please add trailing new line at the end of file

garagatyi · 2016-09-15T08:55:13Z

...ster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/DtoConverter.java

@@ -10,24 +10,8 @@
 *******************************************************************************/
 package org.eclipse.che.api.machine.server;

-import org.eclipse.che.api.core.model.machine.MachineLimits;


Wildcard imports are forbidden in Che

Sorry for forgetting that. I've fixed it.

garagatyi · 2016-09-15T08:57:11Z

...core-api-machine/src/main/java/org/eclipse/che/api/machine/server/model/impl/ServerImpl.java

    }

    public ServerImpl(Server server) {
-        this(server.getRef(), server.getProtocol(), server.getAddress(), server.getPath(), server.getUrl());
+        this(server.getRef(), server.getProtocol(), server.getAddress(), server.getUrl(), server.getPort(), (ServerPropertiesImpl) server.getProperties());


It would be better to avoid casting here as it prevents class cast exception. Please rewrite first constructor instead.

garagatyi · 2016-09-15T08:59:10Z

...e-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceServiceTest.java

@@ -24,14 +24,7 @@
 import org.eclipse.che.api.core.rest.shared.dto.ServiceError;
 import org.eclipse.che.api.environment.server.MachineProcessManager;
 import org.eclipse.che.api.environment.server.MachineServiceLinksInjector;
-import org.eclipse.che.api.machine.server.model.impl.CommandImpl;


Please do not use wildcard imports

garagatyi · 2016-09-15T09:00:40Z

.../extension/machine/client/perspective/widgets/machine/appliance/server/ServerProperties.java

+    @Override
+    public String getInternalUrl() {
+        return descriptor.getInternalUrl();
+    }


Please add equals/hashcode/toString methods

This class still needs equals/hashcode/toString methods

Class https://github.com/eclipse/che/blob/master/plugins/plugin-machine/che-plugin-machine-ext-client/src/main/java/org/eclipse/che/ide/extension/machine/client/perspective/widgets/machine/appliance/server/Server.java had no equals method. Should I add equals/hashcode/toString to existing classes too?

At least add it in classes added by you.

I've added for both.

garagatyi · 2016-09-15T15:10:53Z

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/Server.java

+     * This address is used by the browser to communicate with the server and
+     * corresponds to the internal address if not explicitly set.
+     * Can be set using property machine.docker.local_node_host.external or
+     * environment variable CHE_DOCKER_MACHINE_HOST_EXTERNAL
     */
    String getAddress();


Now this method returns both host and port of the server. Are you going to change the behavior in this PR?

No I'm not:

"address": "localhost:32929"

Is that ok?

garagatyi · 2016-09-15T20:45:15Z

Looks like I'm not fully understand new server object. I'll take a look again tomorrow and ask you if something is not clear.

l0rd · 2016-09-16T16:24:55Z

@garagatyi I've commited the fixes you have suggested:

Javadoc improved
Removed port field in Server class
Annotated all fields in ServerProperties as nullable
Overwritten methods equals, hashcode and toString for classes DevMachineServer and DevMachineServerProperties
Fixed unnamed assisted injections
Added some unit tests
Restored the license header (I'll submit a distinct PR for that)
Environment variables have now higher priority then properties

garagatyi · 2016-09-19T06:17:09Z

.../extension/machine/client/perspective/widgets/machine/appliance/server/ServerProperties.java

+    @Override
+    public String getInternalUrl() {
+        return descriptor.getInternalUrl();
+    }


This class still needs equals/hashcode/toString methods

garagatyi · 2016-09-19T06:17:28Z

...he/ide/extension/machine/client/perspective/widgets/machine/appliance/server/ServerTest.java

+        verify(descriptor).getProperties();
+        verify(descriptor2).getInternalUrl();
+    }
+
 }


Please add trailing new line at the end of file

garagatyi · 2016-09-19T06:18:25Z

...ster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/DtoConverter.java

@@ -10,24 +10,8 @@
 *******************************************************************************/
 package org.eclipse.che.api.machine.server;

-import org.eclipse.che.api.core.model.machine.MachineLimits;


Wildcard imports are forbidden in Che

garagatyi · 2016-09-19T06:19:22Z

...e-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceServiceTest.java

@@ -24,14 +24,7 @@
 import org.eclipse.che.api.core.rest.shared.dto.ServiceError;
 import org.eclipse.che.api.environment.server.MachineProcessManager;
 import org.eclipse.che.api.environment.server.MachineServiceLinksInjector;
-import org.eclipse.che.api.machine.server.model.impl.CommandImpl;


Please do not use wildcard imports

garagatyi · 2016-09-19T06:22:01Z

...lugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstance.java

@@ -145,6 +145,7 @@ public MachineRuntimeInfoImpl getRuntime() {
            try {
                final ContainerInfo containerInfo = docker.inspectContainer(container);
                machineRuntime = new MachineRuntimeInfoImpl(dockerMachineFactory.createMetadata(containerInfo,
+                                                                                                null,


For me it looks like this argument is always null. Did I miss something?

Feel free to add more details to your questions if that was not what you meant. I think you are wondering how Server.url is ever set if this parameter is always null. It should be set using property machine.docker.local_node_host.external but it looks as it's always null.

Property machine.docker.local_node_host.external is injected in LocalDockerModule objects that are binded to DockerInstanceRuntimeInfo in LocalDockerModule.

Therefore method DockerInstance.getRuntime() is not used by the API to get the machine runtime. That explains why Server.url is correctly returned by the API.

Let me explain how I understand what is happening with these changes. Please correct me if I'm mistaken.
DockerInstance always pass null as external host. DockerInstanceRuntimeInfo doesn't change that. LocalDockerInstanceRuntimeInfo uses assisted injection, so this null is passed here too.
So I don't see how another than null value can be accepted by DockerInstanceRuntimeInfo.

As discussed on Slack I've cleaned-up LocalDockerInstanceRuntimeInfo class and made it easier to read.

l0rd · 2016-09-19T08:24:47Z

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/machine/Server.java

@@ -36,14 +42,15 @@
    String getProtocol();

    /**
-     * Path to access the server.
+     * Url of the server, e.g.&nbsp;http://localhost:8080


I think you mean "regular space". If I add a regular space and I try to generate the Javadoc I have the following result:

If I use   instead the sentence is not broken:

OK, interesting

benoitf · 2016-09-16T16:28:05Z

...e-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceServiceTest.java

-import org.eclipse.che.api.machine.server.model.impl.MachineSourceImpl;
-import org.eclipse.che.api.machine.server.model.impl.ServerImpl;
-import org.eclipse.che.api.machine.server.model.impl.SnapshotImpl;
+import org.eclipse.che.api.machine.server.model.impl.*;


still need to remove wildcard import

I think I alrady fixed that: https://github.com/l0rd/che/blob/a35ccae37ab7fdecf958beb02acbf0e8177c8a3b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/WorkspaceServiceTest.java

You were probably looking at the outdated version?

yep it was pending request (I forgot to validate it)

garagatyi · 2016-09-19T13:57:56Z

...lugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstance.java

@@ -145,6 +145,7 @@ public MachineRuntimeInfoImpl getRuntime() {
            try {
                final ContainerInfo containerInfo = docker.inspectContainer(container);
                machineRuntime = new MachineRuntimeInfoImpl(dockerMachineFactory.createMetadata(containerInfo,
+                                                                                                null,


Let me explain how I understand what is happening with these changes. Please correct me if I'm mistaken.
DockerInstance always pass null as external host. DockerInstanceRuntimeInfo doesn't change that. LocalDockerInstanceRuntimeInfo uses assisted injection, so this null is passed here too.
So I don't see how another than null value can be accepted by DockerInstanceRuntimeInfo.

garagatyi · 2016-09-21T08:43:38Z

@musienko-maxim Please do a QA cycle for this PR

garagatyi · 2016-09-21T08:46:00Z

ci-build

codenvy-ci · 2016-09-21T10:02:28Z

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/452/

garagatyi · 2016-09-22T14:55:37Z

QA team has approved this PR. We will merge it when M3 is released

This property allows communications beetween browser and containers that are on different networks (eg. Docker for Mac or NAT) Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

l0rd mentioned this pull request Sep 9, 2016

Set variable CHE_DOCKER_MACHINE_HOST_EXTERNAL on Docker for Mac and Windows eclipse-che/che-dockerfiles#11

Merged

garagatyi mentioned this pull request Sep 12, 2016

[WIP] Replace client side HTTP requests using the hostname that is in the address bar #2004

Closed

l0rd mentioned this pull request Sep 12, 2016

Add iptables rules when running on Docker for Mac eclipse-che/che-dockerfiles#12

Closed

TylerJewell added this to the 5.0.0-M2 milestone Sep 14, 2016

TylerJewell added the kind/enhancement A feature request - must adhere to the feature request template. label Sep 14, 2016

garagatyi reviewed Sep 15, 2016

View reviewed changes

garagatyi suggested changes Sep 15, 2016

View reviewed changes

garagatyi reviewed Sep 15, 2016

View reviewed changes

garagatyi suggested changes Sep 19, 2016

View reviewed changes

l0rd commented Sep 19, 2016

View reviewed changes

l0rd force-pushed the ws-url-fix-reloaded branch from 6dafbc5 to a35ccae Compare September 19, 2016 08:49

benoitf reviewed Sep 19, 2016

View reviewed changes

benoitf approved these changes Sep 19, 2016

View reviewed changes

l0rd force-pushed the ws-url-fix-reloaded branch from a35ccae to f6dfd7b Compare September 19, 2016 13:52

garagatyi suggested changes Sep 19, 2016

View reviewed changes

l0rd force-pushed the ws-url-fix-reloaded branch from f6dfd7b to f549903 Compare September 19, 2016 23:07

riuvshin modified the milestones: 5.0.0-M3, 5.0.0-M2 Sep 20, 2016

bmicklea modified the milestones: 5.0.0-M3, 5.0.0-M4 Sep 20, 2016

garagatyi approved these changes Sep 21, 2016

View reviewed changes

garagatyi added the status/pending-merge label Sep 22, 2016

l0rd force-pushed the ws-url-fix-reloaded branch from f549903 to 9f1d920 Compare September 22, 2016 16:55

l0rd mentioned this pull request Sep 24, 2016

Running workspace results in IOException: Docker image build failed #1766

Closed

riuvshin modified the milestones: 5.0.0-M5, 5.0.0-M4 Sep 27, 2016

bmicklea mentioned this pull request Sep 27, 2016

5.0.0-M5 Milestone Plan #2359

Closed

56 tasks

Add property machine.docker.local_node_host.external.

c2ad879

This property allows communications beetween browser and containers that are on different networks (eg. Docker for Mac or NAT) Signed-off-by: Mario Loriedo <mloriedo@redhat.com>

l0rd force-pushed the ws-url-fix-reloaded branch from 9f1d920 to c2ad879 Compare September 28, 2016 08:54

l0rd merged commit dabb749 into eclipse-che:master Sep 28, 2016

benoitf removed the status/pending-merge label Sep 28, 2016

l0rd mentioned this pull request Oct 12, 2016

Che server should connect to ws-agent on internal URL #2030

Closed

Add property machine.docker.local_node_host.external #2402

Add property machine.docker.local_node_host.external #2402

Conversation

l0rd commented Sep 9, 2016

What does this PR do?

What issues does this PR fix or reference?

Previous behavior

New behavior

PR type

Minor change checklist

codenvy-ci commented Sep 9, 2016

TylerJewell commented Sep 9, 2016

TylerJewell commented Sep 9, 2016

garagatyi commented Sep 10, 2016

l0rd commented Sep 10, 2016

garagatyi commented Sep 11, 2016

l0rd commented Sep 11, 2016

l0rd commented Sep 12, 2016

l0rd commented Sep 12, 2016

TylerJewell commented Sep 12, 2016

l0rd commented Sep 14, 2016

benoitf commented Sep 14, 2016

TylerJewell commented Sep 14, 2016

garagatyi Sep 15, 2016 • edited Loading

Choose a reason for hiding this comment

garagatyi Sep 15, 2016 • edited Loading

Choose a reason for hiding this comment

garagatyi Sep 15, 2016 • edited Loading

Choose a reason for hiding this comment

garagatyi Sep 15, 2016 • 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

garagatyi 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

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

garagatyi commented Sep 15, 2016

l0rd commented Sep 16, 2016

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

garagatyi Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

garagatyi Sep 15, 2016 •

edited

Loading

garagatyi Sep 15, 2016 •

edited

Loading

garagatyi Sep 15, 2016 •

edited

Loading

garagatyi Sep 15, 2016 •

edited

Loading

garagatyi Sep 19, 2016 •

edited

Loading