Skip to content
This repository has been archived by the owner on Feb 26, 2019. It is now read-only.

Dockerfile deis deployment of deis-ui with grunt-connect-proxy and etcd autodiscovery #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ianblenke
Copy link

No description provided.

@bacongobbler
Copy link
Collaborator

There's a lot going on in this PR. I'm not a maintainer of this project, but this seems to infer a lot about Deis' internals, especially WRT inferring that etcd is at 172.17.42.1:4001 by default (a lot of different cloud providers use different internal addresses). Even I'm 😕 on all of the changes going on in here. It's a lot to take in.

Would it be too much to separate out the different changes in this PR (adding a new DEIS_CONTROLLER_FQDN envvar, LIVERELOAD, Dockerfile, proxy settings) into separate PRs along with a well-described justification such that each change can be given the focus it needs?

@ianblenke
Copy link
Author

@bacongobbler, I appreciate your feedback, and I would totally love to spend more time helping with this project, which I fully understand isn't a OpDemand project nor one that you helped build.

However, and I'm going to put it nicely, I spent a solid 12 hours slamming my head up against really horrible grunt/connect/node.js documentation in order to solve the key problem with this project as it stands, and that is that until this PR it was impossible for me to deploy this into my deis clusters.

This uses someting called "livereload" which normally runs on TCP port 35729 and runs its own WebSocket service that is entirely separate from the "connect" web service that runs on port 9000. In order to use with deis, grunt-connect-proxy needed to be grafted in, and let me tell you, that was no small feat.

Note: I don't run much node.js. I've written a few smaller node.js apps, but I'm not an expert in it by any means. This means it's still a pretty monumental undertaking for me to jump into some foreign node.js codebase and do something like graft a proxy into it to make it use a single port.

Heck, the second biggest problem I ran into is that grunt-connect 0.1.11 broke HTTPS proxying. No joke. HTTPS proxying is I had to freeze this thing to 0.1.10 to use HTTPS to my deis controller. Issue #70 in grunt-connect-proxy has my comments for posterity on the matter.

The first problem I ran into is the, again, horrible node.js/grunt/connect documentation, and a community of folks who seem to be mostly cutting and pasting code snippets from a very small number of folks who actually know how this works internally. I had to dig into the source to figure it out.

So, I spent a full day on this. It is deployed. It is obviously a work in progress, but I think jumbojett has a really nice idea here, and I think very idiomatic (such as they are) node.js is being employed. I want to encourage jumbojett to continue this effort, and the best way I could think of was to spend an additional couple of hours iterating and whittling down my PR to the bare minimum required to allow deis-ui to use one port, as is the current deis limitation.

To be honest, I'm using port 8443 here mostly because I have my own ELB setup with TCP and proxyProtocol fowarded to hacked deis-router instances that merrily proxy through to that same TCP port 9000 listener being used by deis (as it is, and can be, the only EXPORT in the Dockerfile). Only by the magic of detecting UPGRADE for proxying WebSockets is this thing merrily working with livereload. It probably should be 443, to be honest.

Then again, this all may be lack of sleep on my part, and I might just be all gripey because of the hair and sleep I've lost last night trying to figure out the right magic to allow both HTTP and WebSockets proxying in node.js, using nothing but sheer force of will and determination.

So, yes, I agree, there is room for improvement. This PR could be so much more. But without this, it simply isn't deployable yet. Granted, it may not be ready to be deployed by most folks yet, but more eyes on a project can only help it.

Keep up the great work @jumbojett! And know that someone else is using your project now :)

@jumbojett
Copy link
Owner

This is a great discussion.

@ianblenke - I agree the Node.js realm is daunting at first. I will point out that the end result is a build of static html and javascript files. Grunt and livereload are meant for development purposes. I'd like to include a dist in the future so developers can avoid dealing with the build process. I think there's a lot of good ideas in you pull request though.

@bacongobbler - I'm adding you as a collaborator. Understandably @ianblenke seems short of time and I'd like help cherry-picking this pull request.

@ianblenke
Copy link
Author

Thanks @jumbojett!

Running deis-ui as pure static content would be great. The big challenge seems to be related to CORS, as without the proxy I was getting a consistent error with Authorization against the deis controller URL. After proxying things, this problem abated.

Is this meant to be deployed as the static content served up by nginx by default on a deis-controller?

@Joshua-Anderson
Copy link
Collaborator

The CORS issues is due to the fact that deis headers need to be whitelisted, I have a PR for that open in deis/deis#3253 which will probably be part of the next release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants