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

update Docker startup steps #9216

Merged
merged 2 commits into from
Dec 31, 2020
Merged

update Docker startup steps #9216

merged 2 commits into from
Dec 31, 2020

Conversation

taroface
Copy link
Contributor

Fixes #9185.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@taroface
Copy link
Contributor Author

@jbowens I went as far back as 19.2 to make this fix, but let know if that wasn't necessary.

Also, these articles are used in our Linux tutorials (e.g. https://www.cockroachlabs.com/docs/v20.2/start-a-local-cluster-in-docker-linux) as well. Can the same updates apply to Linux, or should we break this out into a Mac-specific article?

@cockroach-teamcity
Copy link
Member

@jbowens
Copy link

jbowens commented Dec 18, 2020

Looks good! I think it's good idea to make the change all the way back to 19.2.

Can the same updates apply to Linux, or should we break this out into a Mac-specific article?

Yeah, that seems fine to recommend the same for Linux.

I think one more thing we should change is Step 6: Stop the cluster. Instead of telling them to rm -rf cockroach-data, we should tell them to remove the Docker volumes:

docker volume rm roachdb1 roachdb2 roachdb3

@taroface
Copy link
Contributor Author

TFTR!

@cockroach-teamcity
Copy link
Member

Copy link

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@mikeCRL mikeCRL left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @mikeCRL)

@jseldess
Copy link
Contributor

@taroface, I'll go ahead and merge this. If there's any work left, you can follow-up with another PR.

@jseldess jseldess merged commit 7d8ae50 into master Dec 31, 2020
@jseldess jseldess deleted the docker-start branch December 31, 2020 23:09
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.

Use docker volumes in docker instructions
5 participants