From 0a4c1b5e204b6b3f5f1341b7a08cccaaa2685ff8 Mon Sep 17 00:00:00 2001 From: Filipe Brandenburger Date: Fri, 16 Jan 2015 16:38:09 -0800 Subject: [PATCH] Fix comparison of EtcdClient to nil so that it does not run into the pointer vs. interface issue This is a partial rollback of commit 6e6f465a3699ff ("Fix a crash for kubelet when without EtcdClient") in which we used the `reflect` module to inspect that the pointer stored inside the interface was `nil`, but as pointed out by @lavalamp, the correct solution is to make the function return the interface type, in which case a `return nil` will return the interface nil and not a nil pointer that turns into a non-nil value when coerced into an interface. For more details, see http://golang.org/doc/faq#nil_error and the discussion in PR #3356. Tested by installing a kubelet built from head with this patch into a containervm instance and confirming it did not crash on standalone.go. Confirmed that by only removing the `reflect.IsNil()` comparison but not changing the return type of `EtcdClientOrDie()` did indeed cause that same crash, so changing the return type does indeed fix the issue. Signed-off-by: Filipe Brandenburger --- pkg/kubelet/util.go | 3 ++- pkg/standalone/standalone.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubelet/util.go b/pkg/kubelet/util.go index cc6aa5834ce..7c88f150047 100644 --- a/pkg/kubelet/util.go +++ b/pkg/kubelet/util.go @@ -29,6 +29,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client/record" "github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth" "github.com/GoogleCloudPlatform/kubernetes/pkg/health" + "github.com/GoogleCloudPlatform/kubernetes/pkg/tools" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/coreos/go-etcd/etcd" "github.com/golang/glog" @@ -59,7 +60,7 @@ func InitHealthChecking(k *Kubelet) { } // TODO: move this into a pkg/tools/etcd_tools -func EtcdClientOrDie(etcdServerList util.StringList, etcdConfigFile string) *etcd.Client { +func EtcdClientOrDie(etcdServerList util.StringList, etcdConfigFile string) tools.EtcdClient { if len(etcdServerList) > 0 { return etcd.NewClient(etcdServerList) } else if etcdConfigFile != "" { diff --git a/pkg/standalone/standalone.go b/pkg/standalone/standalone.go index b9af29ac77a..96665b268a7 100644 --- a/pkg/standalone/standalone.go +++ b/pkg/standalone/standalone.go @@ -200,7 +200,6 @@ func makePodSourceConfig(kc *KubeletConfig) *config.PodConfig { glog.Infof("Adding manifest url: %v", kc.ManifestURL) config.NewSourceURL(kc.ManifestURL, kc.HttpCheckFrequency, cfg.Channel(kubelet.HTTPSource)) } - if kc.EtcdClient != nil { glog.Infof("Watching for etcd configs at %v", kc.EtcdClient.GetCluster()) config.NewSourceEtcd(config.EtcdKeyForHost(kc.Hostname), kc.EtcdClient, cfg.Channel(kubelet.EtcdSource))