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

fix local dind env #440

Merged
merged 2 commits into from
May 5, 2019
Merged

fix local dind env #440

merged 2 commits into from
May 5, 2019

Conversation

weekface
Copy link
Contributor

What problem does this PR solve?

  • add --skip-refresh to helm init
  • fix dind document
  • rm manifests/local-dind/pv-create.sh manifests/local-dind/pv-hosts.sh
    and hack/dind-run-operators.sh

What is changed and how it works?

Check List

Tests

  • No code

Code changes

  • Has documents change

Side effects

Related changes

  • Need to update the documentation

* fix dind document
* rm manifests/local-dind/pv-create.sh manifests/local-dind/pv-hosts.sh
and hack/dind-run-operators.sh
@@ -2401,7 +2401,7 @@ function dind::run_tiller {
if [[ $? -eq 0 ]];then
helm_version=$(helm version -c --template '{{.Client.SemVer}}')
if [[ -n ${KUBE_REPO_PREFIX} ]];then
helm init --tiller-image ${KUBE_REPO_PREFIX}/tiller:${helm_version}
helm init --tiller-image ${KUBE_REPO_PREFIX}/tiller:${helm_version} --skip-refresh
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you want to add this parameter ? Just want to skip downloading the stable repo's index.yaml file ? refer this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, just to skip the download of the index.yaml file on https://kubernetes-charts.storage.googleapis.com. It is blocked that you understand.

onlymellb
onlymellb previously approved these changes Apr 30, 2019
Copy link
Contributor

@onlymellb onlymellb left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1,8 +0,0 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

It is useful for me to have a bash script rather than doing a copy and paste from the tutorial.

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 script is out of date, it will include ./manifests/local-dind/setup.sh -> ./manifests/local-dind/pv-hosts.sh -> ./manifests/local-dind/pv-create.sh.

but the pv create is replaced by manifests/local-dind/dind-cluster-v1.12.sh.

and many users use pv-create.sh in their production setup, this is dangerous.

So we may better use this document for local dind setup instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the script is out of date. Can we update it to use the commands in that document?

manifests/local-dind/dind-cluster-v1.12.sh up
kubectl apply -f manifests/crd.yaml
helm install charts/tidb-operator --name=tidb-operator --namespace=tidb-admin
helm install charts/tidb-cluster --name=demo --namespace=tidb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course

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

@weekface
Copy link
Contributor Author

weekface commented May 5, 2019

/run-e2e-tests

@weekface
Copy link
Contributor Author

weekface commented May 5, 2019

@onlymellb @gregwebs ptal

@tennix tennix merged commit 6598b4d into pingcap:master May 5, 2019
@@ -48,8 +48,7 @@ NAME AGE
tidbclusters.pingcap.com 1m

$ # Install TiDB Operator into Kubernetes
$ helm install charts/tidb-operator --name=tidb-operator --namespace=tidb-admin

$ helm install charts/tidb-operator --name=tidb-operator --namespace=tidb-admin --set scheduler.kubeSchedulerImageName=mirantis/hypokube --set scheduler.kubeSchedulerImageTag=final
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not change it in values.yaml directly?

Copy link
Member

Choose a reason for hiding this comment

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

This is a tutorial, it's easier for users to copy/paste.

@weekface weekface deleted the fix-local-dind branch June 21, 2019 08:42
yahonda pushed a commit that referenced this pull request Dec 27, 2021
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.

6 participants