This repository has been archived by the owner on Dec 18, 2020. It is now read-only.
Removed hardcode from protokube logic. Fixes #15. #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prashima
requested review from
abrarshivani,
msterin,
luomiao and
SandeepPissay
|
This conflicts with Abrar's latest commit... sorry that you need to rebase again :) |
msterin
reviewed
Apr 11, 2017
| @@ -102,7 +102,7 @@ Use .build/dist/linux/amd64/kops if working on a linux machine, instead of mac. | |||
| **Notes** | |||
|
|
|||
| 1. ```clustername``` should end with **skydns.local**. Example: ```kubernetes.cluster.skydns.local```. | |||
| 2. ```zones``` should end with ```a```. Example: ```us-west-2a``` or as the above command sets ```--zones=${AWS_REGION}a```. | |||
| 2. For ```zones``` any string will do, for now. It's only getting used for the construction of names of various entities. But it's a mandatory argument. | |||
There was a problem hiding this comment.
if any strings will do, why not have a default ? Or, are you saying it will be mandatory in the future when the string will have meaning ?
There was a problem hiding this comment.
From kops (aws and gce) point of view default zone doesn't mean anything. I am hoping that we would be able to come up with some meaningful mapping for zone names. If not, later we can think about making it a default. But yes... for now any string will do.
prashima
force-pushed
the
vsphere-volume
branch
from
c7880e8 to
4633cd9
Compare
|
Rebased and tested my changes again. |
|
LGTM |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
Populating volume metadata based on actual zone name passed in 'kops cluster create' command.
Updated protokube code to use volume metadata and create volume and etcd cluster spec.
Testing:
Ran 'make ci'
Successfully deployed cluster with following command, please note the 'zone' argument-
kops create cluster --cloud=vsphere --name=v2c2.skydns.local --zones=vmw-zone --vsphere-server=10.160.236.59 --vsphere-datacenter=VSAN-DC --vsphere-resource-pool=VSAN-Cluster --vsphere-datastore=vsanDatastore --dns=private --vsphere-coredns-server=http://10.160.227.86:2379 --dns-zone=skydns.local --image=ubuntu_16_04 --node-count=2 --networking=flannel --yes
Checked protokube logs to make sure correct volume data is getting populated, along with etcd cluster information. Here is the log snippet-
I0411 18:18:00.874477 1 vsphere_volume.go:88] Found volumes: [{"ID":"01","LocalDevice":"/dev/sdb1","AttachedTo":"10.160.249.78","Mountpoint":"/mnt/master-01","Status":"attached","Info":{"Description":"main","EtcdClusters":[{"clusterKey":"main","nodeName":"e","nodeNames":["e"]}]}} {"ID":"02","LocalDevice":"/dev/sdc1","AttachedTo":"10.160.249.78","Mountpoint":"/mnt/master-02","Status":"attached","Info":{"Description":"events","EtcdClusters":[{"clusterKey":"events","nodeName":"e","nodeNames":["e"]}]}}]