From 79ff06ffa1176eb0c855f7a3fea88898f604afaf 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 | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/util.go b/pkg/kubelet/util.go index 767108c9bf5..079acc04658 100644 --- a/pkg/kubelet/util.go +++ b/pkg/kubelet/util.go @@ -25,6 +25,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/client/record" "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" @@ -55,7 +56,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 b3d7218dcb3..14594a562e8 100644 --- a/pkg/standalone/standalone.go +++ b/pkg/standalone/standalone.go @@ -20,7 +20,6 @@ import ( "fmt" "net" "net/http" - "reflect" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -229,11 +228,11 @@ 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 && !reflect.ValueOf(kc.EtcdClient).IsNil() { + 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)) } - if kc.KubeClient != nil && !reflect.ValueOf(kc.KubeClient).IsNil() { + if kc.KubeClient != nil { glog.Infof("Watching apiserver") config.NewSourceApiserver(kc.KubeClient, kc.Hostname, cfg.Channel(kubelet.ApiserverSource)) }