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

Add kubernetes api server config #2603

Merged
merged 1 commit into from
Mar 3, 2017
Merged

Add kubernetes api server config #2603

merged 1 commit into from
Mar 3, 2017

Conversation

davygeek
Copy link
Contributor

Add Kuberentes api address config for KUBECTL
The default value is 127.0.0.1:8080
When the Kuberentes api server is not local, We can easily access the api by edit KUBERENETAPI's value

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@davygeek
Copy link
Contributor Author

@googlebot I signed it!

# Kuberentes api address for $KUBECTL
# The default value is 127.0.0.1:8080
# When the Kuberentes api server is not local, We can easily access the api by edit KUBERENETAPI's value
KUBERENETAPI=${KUBERENETAPI:-'127.0.0.1:8080'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix the comments to say Kubernetes instead of Kuberentes? Also can you name this var KUBERNETES_API_SERVER to be clearer?

One thing that might help make life simpler in case we need to add any other flags in the future is to create a var in env.sh called KUBECTL_OPTIONS, which would just be '--namespace=$VITESS_NAME --server=$KUBERNETES_API_SERVER' and use that instead in the other scripts. That's optional for this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thompsonja ok, thanks. I'll fix it

@@ -18,7 +23,7 @@ VTCTLD_PORT=${VTCTLD_PORT:-30001}

# Get the ExternalIP of any node.
get_node_ip() {
$KUBECTL get -o template --template '{{range (index .items 0).status.addresses}}{{if eq .type "ExternalIP" "LegacyHostIP"}}{{.address}}{{end}}{{end}}' nodes --namespace=$VITESS_NAME
$KUBECTL --server=$KUBERENETAPI get -o template --template '{{range (index .items 0).status.addresses}}{{if eq .type "ExternalIP" "LegacyHostIP"}}{{.address}}{{end}}{{end}}' nodes --namespace=$VITESS_NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extra space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thompsonja I'll remove it, thanks

@@ -13,10 +13,10 @@ cells=`echo $CELLS | tr ',' ' '`
# Delete replication controllers
for cell in 'global' $cells; do
echo "Stopping etcd replicationcontroller for $cell cell..."
$KUBECTL delete replicationcontroller etcd-$cell --namespace=$VITESS_NAME
$KUBECTL --server=$KUBERENETAPI delete replicationcontroller etcd-$cell --namespace=$VITESS_NAME
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extra space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thompsonja I'll remove it, thanks

@michael-berlin
Copy link
Contributor

re: the CLA check problem. Check the email address which you use in your commits e.g. view it here: https://patch-diff.githubusercontent.com/raw/youtube/vitess/pull/2603.patch

I think this email must also be the primary email address of your GitHub account. If there's an inconsistency, you may have to change one of it.

@davygeek
Copy link
Contributor Author

davygeek commented Mar 1, 2017

@googlebot I signed it!

@davygeek
Copy link
Contributor Author

davygeek commented Mar 1, 2017

I signed it!

@davygeek
Copy link
Contributor Author

davygeek commented Mar 1, 2017

@michael-berlin the patch email address is "From: davygeek davygeek@163.com" and GitHub account 's email is "davygeek@163.com Primary Public". Is that right? Thank you

@davygeek
Copy link
Contributor Author

davygeek commented Mar 2, 2017

@acharis @michael-berlin the patch email address is "From: davygeek davygeek@163.com" and GitHub account 's email is "davygeek@163.com Primary Public". Is that right? Thank you

@michael-berlin
Copy link
Contributor

I signed it!

@michael-berlin
Copy link
Contributor

@davygeek Can you please try to trigger the googlebot again? It doesn't seem to work when I do it :)

re: the review itself. Let's wait for Josh to approve the change. Thanks! :)

@davygeek
Copy link
Contributor Author

davygeek commented Mar 2, 2017

I signed it!

@davygeek
Copy link
Contributor Author

davygeek commented Mar 2, 2017

@michael-berlin thanks, I trigger the googlebot again, but the cla still "cal:no". I'm very confused. Please help to check the problem, I can't find the cause. Thank you very much.

my config base info as follow:
/// local git config
$ git config --global user.name
davygeek
$ git config --global user.email
davygeek@163.com

// github account info:
davygeek@163.com Primary Public

// cla data
Name: davygeek
GitHub Account Name: davygeek@163.com

@davygeek
Copy link
Contributor Author

davygeek commented Mar 2, 2017

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Mar 2, 2017
@davygeek
Copy link
Contributor Author

davygeek commented Mar 2, 2017

@michael-berlin Thanks very much, The reason is my google develop's account is not match with github account. I used davygeek@163.com as google develop's account the cla is ok.

@thompsonja
Copy link
Contributor

thompsonja commented Mar 2, 2017

LGTM, thanks :)

Approved with PullApprove

@thompsonja thompsonja merged commit 104bee0 into vitessio:master Mar 3, 2017
frouioui pushed a commit to planetscale/vitess that referenced this pull request Mar 26, 2024
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.

4 participants