Merge pull request #986 from adamreese/fix/validate-running-tiller

fix(cmd): ensure tiller is running for connection

cmd/helm/tunnel.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/labels"
 
 	"k8s.io/helm/pkg/kube"
@@ -29,30 +30,43 @@ import (
 var tunnel *kube.Tunnel
 
 func newTillerPortForwarder(namespace string) (*kube.Tunnel, error) {
-	podName, err := getTillerPodName(namespace)
+	kc := kube.New(nil)
+	client, err := kc.Client()
+	if err != nil {
+		return nil, err
+	}
+
+	podName, err := getTillerPodName(client, namespace)
 	if err != nil {
 		return nil, err
 	}
-	// FIXME use a constain that is accessible on init
 	const tillerPort = 44134
-	return kube.New(nil).ForwardPort(namespace, podName, tillerPort)
+	return kc.ForwardPort(namespace, podName, tillerPort)
 }
 
-func getTillerPodName(namespace string) (string, error) {
-	client, err := kube.New(nil).Client()
+func getTillerPodName(client unversioned.PodsNamespacer, namespace string) (string, error) {
+	// TODO use a const for labels
+	selector := labels.Set{"app": "helm", "name": "tiller"}.AsSelector()
+	pod, err := getFirstRunningPod(client, namespace, selector)
 	if err != nil {
 		return "", err
 	}
+	return pod.ObjectMeta.GetName(), nil
+}
 
-	// TODO use a const for labels
-	selector := labels.Set{"app": "helm", "name": "tiller"}.AsSelector()
+func getFirstRunningPod(client unversioned.PodsNamespacer, namespace string, selector labels.Selector) (*api.Pod, error) {
 	options := api.ListOptions{LabelSelector: selector}
 	pods, err := client.Pods(namespace).List(options)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 	if len(pods.Items) < 1 {
-		return "", fmt.Errorf("I could not find tiller")
+		return nil, fmt.Errorf("could not find tiller")
 	}
-	return pods.Items[0].ObjectMeta.GetName(), nil
+	for _, p := range pods.Items {
+		if api.IsPodReady(&p) {
+			return &p, nil
+		}
+	}
+	return nil, fmt.Errorf("could not find a ready pod")
 }

cmd/helm/tunnel_test.go

@@ -0,0 +1,91 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+func mockTillerPod() api.Pod {
+	return api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "orca",
+			Namespace: api.NamespaceDefault,
+			Labels:    map[string]string{"app": "helm", "name": "tiller"},
+		},
+		Status: api.PodStatus{
+			Phase: api.PodRunning,
+			Conditions: []api.PodCondition{
+				{
+					Status: api.ConditionTrue,
+					Type:   api.PodReady,
+				},
+			},
+		},
+	}
+}
+
+func mockTillerPodPending() api.Pod {
+	p := mockTillerPod()
+	p.Name = "blue"
+	p.Status.Conditions[0].Status = api.ConditionFalse
+	return p
+}
+
+func TestGetFirstPod(t *testing.T) {
+	tests := []struct {
+		name     string
+		pods     []api.Pod
+		expected string
+		err      bool
+	}{
+		{
+			name:     "with a ready pod",
+			pods:     []api.Pod{mockTillerPod()},
+			expected: "orca",
+		},
+		{
+			name: "without a ready pod",
+			pods: []api.Pod{mockTillerPodPending()},
+			err:  true,
+		},
+		{
+			name: "without a pod",
+			pods: []api.Pod{},
+			err:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		client := &testclient.Fake{}
+		client.PrependReactor("list", "pods", func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
+			return true, &api.PodList{Items: tt.pods}, nil
+		})
+
+		name, err := getTillerPodName(client, api.NamespaceDefault)
+		if (err != nil) != tt.err {
+			t.Errorf("%q. expected error: %v, got %v", tt.name, tt.err, err)
+		}
+		if name != tt.expected {
+			t.Errorf("%q. expected %q, got %q", tt.name, tt.expected, name)
+		}
+	}
+}