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

Improve workflow in docs/google-kubernetes-tutorial.md #400

Merged
merged 12 commits into from
Apr 19, 2019

Conversation

cofyc
Copy link
Contributor

@cofyc cofyc commented Apr 16, 2019

  • remove kubectl create service account tiller --namespace kube-system because tiller service account yaml has been written into tiller-rbac.yaml
  • use absolute path $HOME/tidb-operator, so user can paste and run commands under any directory (bad idea, because the user may clone tidb-operator.git into another directory when they try the tutorial again)
  • update some notes about accessing grafana in browser
  • kubectl run ... is removed because it's not a reliable way to access kubernetes service (e.g. when resources are not deleted, user need to clean up manually)
  • port-forward svc/demo-tidb instead of demo-tidb-0
  • install helm in one command
  • Use Ctrl+C, better way to show keyboard keys (suggested by @lilin90)

Test link: https://console.cloud.google.com/cloudshell/open?tutorial=docs/google-kubernetes-tutorial.md&git_repo=https://github.com/cofyc/tidb-operator&cloudshell_git_branch=gke-tutorial

@cofyc cofyc force-pushed the gke-tutorial branch 2 times, most recently from 5e69c78 to 64ccbca Compare April 16, 2019 13:31
@cofyc cofyc requested review from gregwebs and tennix April 16, 2019 13:48
gregwebs
gregwebs previously approved these changes Apr 16, 2019
Copy link
Contributor

@gregwebs gregwebs left a comment

Choose a reason for hiding this comment

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

Thanks for going through this!

Co-Authored-By: cofyc <cofyc.jackson@gmail.com>
Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

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

LGTM. But I think we should add instructions to access Grafana for GKE tutorial in later PRs.

@cofyc
Copy link
Contributor Author

cofyc commented Apr 17, 2019

follow up issue: #404

@tennix
Copy link
Member

tennix commented Apr 17, 2019

FYI @lilin90

@tennix tennix requested a review from gregwebs April 17, 2019 13:38
@tennix tennix self-requested a review April 17, 2019 13:39
tennix
tennix previously approved these changes Apr 17, 2019
@cofyc
Copy link
Contributor Author

cofyc commented Apr 18, 2019

@gregwebs @lilin90 ask for another approval, thanks!

charts/tidb-cluster/templates/NOTES.txt Outdated Show resolved Hide resolved
docs/google-kubernetes-tutorial.md Outdated Show resolved Hide resolved
Co-Authored-By: cofyc <cofyc.jackson@gmail.com>
@cofyc
Copy link
Contributor Author

cofyc commented Apr 18, 2019

/run-e2e-tests

Copy link
Member

@lilin90 lilin90 left a comment

Choose a reason for hiding this comment

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

LGTM

@cofyc
Copy link
Contributor Author

cofyc commented Apr 18, 2019

@tennix Suggested by @lilin90, I replaced all Control + C with Ctrl+C. PTAL again.

@cofyc cofyc merged commit d953b0c into pingcap:master Apr 19, 2019
yahonda pushed a commit that referenced this pull request Dec 27, 2021
* operator offline installation

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>

* address comment

* address comment

* Apply suggestions from code review

Co-authored-by: Ran <huangran@pingcap.com>
Co-authored-by: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
Co-authored-by: ti-srebot <66930949+ti-srebot@users.noreply.github.com>
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.

5 participants