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

docs: Fix issues found in Queeny's test #507

Merged
merged 24 commits into from
May 24, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ebaa8ec
docs/dind: add instructions for XFS filesystems
AstroProfundis May 15, 2019
8d07ed2
docs: use `kubectl --watch` instead of `watch kubectl` in tutorials
AstroProfundis May 17, 2019
211012e
docs/aws: add detail instructions for requirements
AstroProfundis May 20, 2019
b4d4ffd
docs/dind: add notes for accessing TiDB cluster with MySQL client
AstroProfundis May 20, 2019
fe67dcd
docs: add sample output of terraform
AstroProfundis May 21, 2019
117dec3
Merge branch 'master' of github.com:pingcap/tidb-operator into fix-qu…
AstroProfundis May 21, 2019
5d2c19d
docs/aws: minor updates of words
AstroProfundis May 21, 2019
5eb0ddd
Update docs/local-dind-tutorial.md
AstroProfundis May 22, 2019
15d7ef2
docs: update instructions for macOS & update argument in sample
AstroProfundis May 22, 2019
471bab5
docs: Apply suggestions from code review
AstroProfundis May 22, 2019
e510d0e
docs/aws: fix synatax of intrucduction
AstroProfundis May 22, 2019
1f48c22
docs/aliyun: update instructions when deploying with terraform
AstroProfundis May 22, 2019
d04d708
docs/aws: adjust instructions to access DB and grafana & cleanup
AstroProfundis May 22, 2019
32fa5e9
Apply suggestions from code review
AstroProfundis May 23, 2019
7b46590
docs/dind: update contents to fix issues found in the test
AstroProfundis May 23, 2019
b7ff879
docs/aws: add an introduction sentense of terraform installation
AstroProfundis May 23, 2019
6bc6e06
docs/dind: pointing out xfs issue only applies to Linux users
AstroProfundis May 23, 2019
19a6b28
docs/dind: make port-forward instruction more clear
AstroProfundis May 23, 2019
3cd9826
Apply suggestions from code review
AstroProfundis May 23, 2019
e004170
docs/dind: make delete instructions more clear
AstroProfundis May 23, 2019
b38188e
docs/aws: update instructions of customizing params
AstroProfundis May 23, 2019
9e41760
docs/dind: clean up
AstroProfundis May 23, 2019
4e0c8e5
docs/aws: add examples and adjust order of sections
AstroProfundis May 24, 2019
faab010
Merge branch 'master' into fix-queeny-test-issues
tennix May 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions deploy/aws/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@
The easist way to install `aws-iam-authenticator` is to download the prebuilt binary:
AstroProfundis marked this conversation as resolved.
Show resolved Hide resolved

``` shell
# Download binary for Linux
curl -o aws-iam-authenticator https://amazon-eks.s3-us-west-2.amazonaws.com/1.12.7/2019-03-27/bin/linux/amd64/aws-iam-authenticator
AstroProfundis marked this conversation as resolved.
Show resolved Hide resolved

# Or, download binary for macOS
curl -o aws-iam-authenticator https://amazon-eks.s3-us-west-2.amazonaws.com/1.12.7/2019-03-27/bin/darwin/amd64/aws-iam-authenticator

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using the code below is simpler and more general

os=$(uname -s| tr '[:upper:]' '[:lower:]')
curl -o aws-iam-authenticator https://amazon-eks.s3-us-west-2.amazonaws.com/1.12.7/2019-03-27/bin/$os/amd64/aws-iam-authenticator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any mac for test, if you can confirm this works for macOS?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it works for macOS and Linux

Copy link
Member

Choose a reason for hiding this comment

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

@AstroProfundis The link you mentioned already has an elegant way to install aws-iam-authenticator.
For macOS, install with homebrew. For linux install with the curl to fetch the binary. It's inappropriate to provide the tricky script in the guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think homebrew is available to every macOS installation by default...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's not builtin on macOS. But almost all technical users install that. So I think provide homebrew command is reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This definitely can't pass Queeney's test, the "almost all technical users" is not fitting the target user groups.

Copy link
Contributor

@yikeke yikeke May 23, 2019

Choose a reason for hiding this comment

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

You can add a note in the document that before users install aws-iam-authenticator with homebrew, they need to make sure that they have installed homebrew first, then providing the homebrew way is totally acceptable for queeny's test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we're introducing new dependency and copying almost all major parts of AWS document, what's the point of avoiding out-linking again?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, you are right. So if the curl way does not require any new dependency for macOS, I suggest we introduce the curl way in our document then. If users prefer the homebrew way, they can still click the link and find the guide in the AWS document.

chmod +x ./aws-iam-authenticator
sudo mv ./aws-iam-authenticator /usr/local/bin/aws-iam-authenticator
```
Expand Down
2 changes: 1 addition & 1 deletion docs/local-dind-tutorial.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Before deploying a TiDB cluster to Kubernetes, make sure the following requireme
cat << EOF > /etc/systemd/system/docker.service.d/docker-storage.conf
[Service]
ExecStart=
ExecStart=/usr/bin/dockerd -g /data/docker -H fd:// --containerd=/run/containerd/containerd.sock
ExecStart=/usr/bin/dockerd --data-root /data/docker -H fd:// --containerd=/run/containerd/containerd.sock
EOF

# Restart docker daemon
Expand Down