Allow etcd config file to be passed to apiserver, kubelet, and proxy #1464
Conversation
|
Question on the CLA: this patch (unlike my earlier one) was done while wearing my Box Employee hat. I've signed an individual CA for my previous contributions, and it sounds like @ghodss has submitted a corporate CLA for Box. Should I get a separate corporate CLA signed or is there some procedure for getting added to the existing one? |
|
Regarding CLA: I do not see a corp CLA for Box, just an individual CLA for Sam, and another for you, though not with a box email address. Let me poke legal. |
|
Our legal team says we need a corp CLA if you're doing work on Box's dime. It's probably the right thing to do anyway, given Sam's work. |
| @@ -103,6 +105,15 @@ func initCloudProvider(name string, configFilePath string) cloudprovider.Interfa | |||
| return cloud | |||
| } | |||
|
|
|||
| func newEtcd(etcdConfigFile string, etcdServerList util.StringList) (helper tools.EtcdHelper, err error) { | |||
| if etcdConfigFile == "" { | |||
| helper, err = master.NewEtcdHelper(etcdServerList, *storageVersion) | |||
There was a problem hiding this comment.
idomatically I think you can get away without naming the returns and saying:
if etcdConfigFile = "" {
return master.NewEtcdHelper(etcdServerList, *storageVersion)
}
return master.NewConfiguredEtcdHelper(etcdConfigFile, *storageVersion)
Though I would probably flip it to check for != ""
|
Can you explain the motivation for this? Why do we need it? |
|
@thockin The motivation is to allow setting certificates and securing the kubernetes <-> etcd communication |
|
I've passed along the CLA to our legal team, I'll get back to you when it's ready and/or if we have any questions. |
|
Great! On Fri, Sep 26, 2014 at 3:37 PM, Haney Maxwell notifications@github.com
|
| @@ -141,10 +141,19 @@ func main() { | |||
| } | |||
|
|
|||
| // define etcd config source and initialize etcd client | |||
| var etcdClient tools.EtcdClient | |||
| var etcdClient *etcd.Client | |||
There was a problem hiding this comment.
Why change this? Should still be our internal interface.
There was a problem hiding this comment.
Hey, the internal interface doesn't expose .GetCluster (used below), which is why I changed it. What's your preference:
var etcdClient *etcd.Client- Change the internal interface to include .GetCluster
- Change the logging below
There was a problem hiding this comment.
It's fine as is - just wanted to double check.
|
Looks good to me, can you squash and let me know when you have CLA? Then I'll do final signoff and merge. |
|
Will do, unfortunately still waiting on CLA On Wed, Oct 1, 2014 at 8:48 AM, Clayton Coleman notifications@github.com
|
hmrm
force-pushed
the
add-etcd-config
branch
2 times, most recently
from
7b8a57f
to
2187772
Compare
|
@smarterclayton CLA in! Rebased to address the merge conflicts; probably worth a quick look since the rebase involved a few logic changes. |
|
CLA hasn't found its way onto our list just yet. Sometimes that takes a day or two for the corporate ones. |
|
Hmmm, the build seems to have succeeded on 1.3 but failed to compile something in on 1.2. I'm getting an error locally when I try building with 1.2:
but it seems unrelated to the changes I'm making. |
|
CLA confirmed! I restarted travis, too. |
|
@lavalamp Thanks! Looks like Travis is passing; I'm ready to merge if you are! |
Allow etcd config file to be passed to apiserver, kubelet, and proxy
|
Thanks for the change! |
UPSTREAM: <carry>: update rebase doc
This is meant to fix #1338 .
I'm a relative golang newbie, so let me know where I'm going wrong here.