Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjustments for our wonderful Windows users #759

Merged
merged 3 commits into from
Mar 11, 2016
Merged

Adjustments for our wonderful Windows users #759

merged 3 commits into from
Mar 11, 2016

Conversation

etoews
Copy link
Contributor

@etoews etoews commented Mar 11, 2016

No description provided.

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@carolynvs @zack-shoylev Can you please give this a look?

@gpdowning I think you'll be interested in tracking this as well.

FYI, I made these changes as this tutorial advises WIndows users to use the Docker Quickstart Terminal.

@carolynvs
Copy link
Contributor

I am reviewing now, some of my comments are for the original, not just the changes. Trying to figure out how to leave comments on something other than the diff... 😄

@carolynvs
Copy link
Contributor

When I run docker-compose up I get the following error:

$ docker-compose up
ERROR: Validation failed in file 'C:\Users\caro8994\carina-examples\python-web-app\docker-compose-common.yml',
reason(s):
services.app.networks contains an invalid type, it should be an array

My docker-compose version is 1.6.0, build cdb920a. I'm updating now to see if that makes a difference.

@carolynvs
Copy link
Contributor

Heh, updating to the latest version of toolbox broke everything... This may take a bit longer to fix and finish reviewing!

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@carolynvs This tweet fixed the Compose version issue 😄

@carolynvs
Copy link
Contributor

I've run into the dreaded docker/machine#1954 which means that I can't test this for another 6 hours or so... I'm having another go at troubleshooting that problem while I'm waiting. 😄

@carolynvs
Copy link
Contributor

The instructions for https://github.com/everett-toews/getcarina.com/blob/py-dev-win/_getting-started/2016-03-08-python-web-app-dev.md#initialize-the-environment-1 are not what we recommend.

Can we point them instead to this? Create and connect to a cluster. In most of the tutorials that link is the first step, without any additional text.

@carolynvs
Copy link
Contributor

For https://github.com/everett-toews/getcarina.com/blob/py-dev-win/_getting-started/2016-03-08-python-web-app-dev.md#build-the-images, I tested the interactive docker login command with both the QuickStart Terminal (aka git bash) and powershell, both worked just fine. So you can remove the "Note: On Windows, specify all of the information immediately for the docker login command. For example, docker login -u -p -e email@example.com" caveat.

@carolynvs
Copy link
Contributor

I wasn't able to run through the tutorial due to unrelated problems but I've commented on anything that made my "windows sense" tingle.

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

Can we point them instead to this? Create and connect to a cluster. In most of the tutorials that link is the first step, without any additional text.

I think this boils down to...do we need to add something like the following to Create and connect to a cluster

If you've installed the Docker Toolbox on Windows and are using the Quickstart Terminal, run the following commands:

$ cd Downloads/mycluster
$ source docker.env
$ export DOCKER_CERT_PATH=C:\\Users\\everett\\Downloads\\mycluster

because you have to setup things a bit differently when using the Docker Toolbox on Windows and the Quickstart Terminal.

What do you think?

@carolynvs
Copy link
Contributor

ick, okay yeah I remember this now...

How about we have them run this if they are on windows?

export DOCKER_CERT_PATH=$(echo $DOCKER_CERT_PATH | sed 's/\///1' | sed 's/\//:\//1')

We just need to get /c/ changed to c:/ and windows doesn't care about the remaining slashes, just the drive (which may not necessarily be C:\).

$ source docker.env
$ echo $DOCKER_CERT_PATH
/c/users/carolynvs/downloads/mycluster
$ export DOCKER_CERT_PATH=$(echo $DOCKER_CERT_PATH | sed 's/\///1' | sed 's/\//:\//1')
$ echo $DOCKER_CERT_PATH
c:/users/carolynvs/downloads/mycluster
$ docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
e51aea54d165        carina/consul       "/bin/consul agent -b"   28 hours ago        Up 28 hours                             af34ce2c-87a2-47c6-8022-4919ae9f552d-n1/carina-svcd

That works for me, how about I update Create and Connect with that and remove the caveat from this one?

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

I can make the update as part of this PR since this is what motivated it. I'll remove the caveat here.

@carolynvs
Copy link
Contributor

👍

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@carolynvs Give it one last look please. For the sed commands I changed the delimiters to # because I get confused looking at too many \\/\/\\/\/\/\/\\/// in a row. o_O

@zack-shoylev
Copy link
Contributor

I ran into some issues:

https://gist.github.com/zack-shoylev/0c5693956ad350377db5

After following the steps, at the step where I open the browser, I get a 502 Bad Gateway; the application log complains about not being able to connect upstream.

Also had to use DockerToolbox-1.10.3 ; the one I had installed I had to nuke and replace.

But I am not sure if this is necessarily windows related.

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@zack-shoylev Ya. That's not a Windows issue.

Did you alter the app at all? Why did you have to use --force-recreate?

@zack-shoylev
Copy link
Contributor

No, I haven't altered the app. The first time it ran I stopped it and reset everything completely, so I thought it maybe didn't reset properly.

I can try to delete everything, reclone, and retry again

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

A docker-compose down should get you a clean slate.

@zack-shoylev
Copy link
Contributor

Already tried that, but no luck.

@carolynvs
Copy link
Contributor

@everett-toews Nice! I didn't know about changing the deliminator. Just tried it out and reviewed the updated files. Looks good!

@zack-shoylev
Copy link
Contributor

I am guessing maybe this is the problem, but will have to check.

@zack-shoylev
Copy link
Contributor

Yes, that was the problem. Mounting doesn't work from any random directory.

@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@zack-shoylev So what exactly do we need to recommend to Windows users to avoid that problem?

@zack-shoylev
Copy link
Contributor

You need to clone carina-examples in a directory under C:\Users[Your username]

```bash
$ eval $(carina env mycluster)
```
1. Open a new terminal and initialize as per the instructions for the [Quickstart Termainal]({{ site.baseurl }}/docs/tutorials/create-connect-cluster/#quickstart-terminal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo: Terminal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Termainal" is a perfectly cromulent word.

@carolynvs
Copy link
Contributor

As I understand it, that is a quirk of VirtualBox, not Windows. So mac/linux users should be working in their home directory as well.

etoews added a commit that referenced this pull request Mar 11, 2016
Adjustments for our wonderful Windows users
@etoews etoews merged commit e4ed6ad into getcarina:master Mar 11, 2016
@etoews
Copy link
Contributor Author

etoews commented Mar 11, 2016

@carolynvs @zack-shoylev Thanks for the review!

@etoews etoews deleted the py-dev-win branch June 15, 2016 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants