Merge pull request #1347 from brendandburns/healthz

Fix a problem where if a minion went missing, we still thought the pod was running.

pkg/client/fake.go

@@ -37,6 +37,7 @@ type Fake struct {
 	Ctrl          api.ReplicationController
 	ServiceList   api.ServiceList
 	EndpointsList api.EndpointsList
+	Minions       api.MinionList
 	Err           error
 	Watch         watch.Interface
 }
@@ -144,5 +145,5 @@ func (c *Fake) ServerVersion() (*version.Info, error) {
 
 func (c *Fake) ListMinions() (*api.MinionList, error) {
 	c.Actions = append(c.Actions, FakeAction{Action: "list-minions", Value: nil})
-	return &api.MinionList{}, nil
+	return &c.Minions, nil
 }

pkg/master/master.go

@@ -119,6 +119,7 @@ func (m *Master) init(cloud cloudprovider.Interface, podInfoGetter client.PodInf
 			PodCache:      podCache,
 			PodInfoGetter: podInfoGetter,
 			Registry:      m.podRegistry,
+			Minions:       m.client,
 		}),
 		"replicationControllers": controller.NewREST(m.controllerRegistry, m.podRegistry),
 		"services":               service.NewREST(m.serviceRegistry, cloud, m.minionRegistry),

pkg/registry/pod/rest.go

@@ -44,6 +44,7 @@ type REST struct {
 	podInfoGetter client.PodInfoGetter
 	podPollPeriod time.Duration
 	registry      Registry
+	minions       client.MinionInterface
 }
 
 type RESTConfig struct {
@@ -51,6 +52,7 @@ type RESTConfig struct {
 	PodCache      client.PodInfoGetter
 	PodInfoGetter client.PodInfoGetter
 	Registry      Registry
+	Minions       client.MinionInterface
 }
 
 // NewREST returns a new REST.
@@ -61,6 +63,7 @@ func NewREST(config *RESTConfig) *REST {
 		podInfoGetter: config.PodInfoGetter,
 		podPollPeriod: time.Second * 10,
 		registry:      config.Registry,
+		minions:       config.Minions,
 	}
 }
 
@@ -101,7 +104,11 @@ func (rs *REST) Get(id string) (runtime.Object, error) {
 	}
 	if rs.podCache != nil || rs.podInfoGetter != nil {
 		rs.fillPodInfo(pod)
-		pod.CurrentState.Status = getPodStatus(pod)
+		status, err := getPodStatus(pod, rs.minions)
+		if err != nil {
+			return pod, err
+		}
+		pod.CurrentState.Status = status
 	}
 	pod.CurrentState.HostIP = getInstanceIP(rs.cloudProvider, pod.CurrentState.Host)
 	return pod, err
@@ -113,7 +120,11 @@ func (rs *REST) List(selector labels.Selector) (runtime.Object, error) {
 		for i := range pods.Items {
 			pod := &pods.Items[i]
 			rs.fillPodInfo(pod)
-			pod.CurrentState.Status = getPodStatus(pod)
+			status, err := getPodStatus(pod, rs.minions)
+			if err != nil {
+				return pod, err
+			}
+			pod.CurrentState.Status = status
 			pod.CurrentState.HostIP = getInstanceIP(rs.cloudProvider, pod.CurrentState.Host)
 		}
 	}
@@ -202,9 +213,24 @@ func getInstanceIP(cloud cloudprovider.Interface, host string) string {
 	return addr.String()
 }
 
-func getPodStatus(pod *api.Pod) api.PodStatus {
+func getPodStatus(pod *api.Pod, minions client.MinionInterface) (api.PodStatus, error) {
 	if pod.CurrentState.Info == nil || pod.CurrentState.Host == "" {
-		return api.PodWaiting
+		return api.PodWaiting, nil
+	}
+	res, err := minions.ListMinions()
+	if err != nil {
+		glog.Errorf("Error listing minions: %v", err)
+		return "", err
+	}
+	found := false
+	for _, minion := range res.Items {
+		if minion.ID == pod.CurrentState.Host {
+			found = true
+			break
+		}
+	}
+	if !found {
+		return api.PodTerminated, nil
 	}
 	running := 0
 	stopped := 0
@@ -222,13 +248,13 @@ func getPodStatus(pod *api.Pod) api.PodStatus {
 	}
 	switch {
 	case running > 0 && stopped == 0 && unknown == 0:
-		return api.PodRunning
+		return api.PodRunning, nil
 	case running == 0 && stopped > 0 && unknown == 0:
-		return api.PodTerminated
+		return api.PodTerminated, nil
 	case running == 0 && stopped == 0 && unknown > 0:
-		return api.PodWaiting
+		return api.PodWaiting, nil
 	default:
-		return api.PodWaiting
+		return api.PodWaiting, nil
 	}
 }
 

pkg/registry/pod/rest_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/fake"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest"
@@ -257,6 +258,15 @@ func TestGetPodCloud(t *testing.T) {
 }
 
 func TestMakePodStatus(t *testing.T) {
+	fakeClient := client.Fake{
+		Minions: api.MinionList{
+			Items: []api.Minion{
+				{
+					JSONBase: api.JSONBase{ID: "machine"},
+				},
+			},
+		},
+	}
 	desiredState := api.PodState{
 		Manifest: api.ContainerManifest{
 			Version: "v1beta1",
@@ -269,12 +279,6 @@ func TestMakePodStatus(t *testing.T) {
 	currentState := api.PodState{
 		Host: "machine",
 	}
-	pod := &api.Pod{DesiredState: desiredState, CurrentState: currentState}
-	status := getPodStatus(pod)
-	if status != api.PodWaiting {
-		t.Errorf("Expected 'Waiting', got '%s'", status)
-	}
-
 	runningState := docker.Container{
 		State: docker.State{
 			Running: true,
@@ -286,67 +290,103 @@ func TestMakePodStatus(t *testing.T) {
 		},
 	}
 
-	// All running.
-	pod = &api.Pod{
-		DesiredState: desiredState,
-		CurrentState: api.PodState{
-			Info: map[string]docker.Container{
-				"containerA": runningState,
-				"containerB": runningState,
+	tests := []struct {
+		pod    *api.Pod
+		status api.PodStatus
+		test   string
+	}{
+		{&api.Pod{DesiredState: desiredState, CurrentState: currentState}, api.PodWaiting, "waiting"},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": runningState,
+						"containerB": runningState,
+					},
+					Host: "machine",
+				},
 			},
-			Host: "machine",
+			api.PodRunning,
+			"all running",
+		},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": runningState,
+						"containerB": runningState,
+					},
+					Host: "machine-two",
+				},
+			},
+			api.PodTerminated,
+			"all running but minion is missing",
+		},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": stoppedState,
+						"containerB": stoppedState,
+					},
+					Host: "machine",
+				},
+			},
+			api.PodTerminated,
+			"all stopped",
+		},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": stoppedState,
+						"containerB": stoppedState,
+					},
+					Host: "machine-two",
+				},
+			},
+			api.PodTerminated,
+			"all stopped but minion missing",
+		},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": runningState,
+						"containerB": stoppedState,
+					},
+					Host: "machine",
+				},
+			},
+			api.PodWaiting,
+			"mixed state #1",
+		},
+		{
+			&api.Pod{
+				DesiredState: desiredState,
+				CurrentState: api.PodState{
+					Info: map[string]docker.Container{
+						"containerA": runningState,
+					},
+					Host: "machine",
+				},
+			},
+			api.PodWaiting,
+			"mixed state #2",
 		},
 	}
-	status = getPodStatus(pod)
-	if status != api.PodRunning {
-		t.Errorf("Expected 'Running', got '%s'", status)
-	}
-
-	// All stopped.
-	pod = &api.Pod{
-		DesiredState: desiredState,
-		CurrentState: api.PodState{
-			Info: map[string]docker.Container{
-				"containerA": stoppedState,
-				"containerB": stoppedState,
-			},
-			Host: "machine",
-		},
-	}
-	status = getPodStatus(pod)
-	if status != api.PodTerminated {
-		t.Errorf("Expected 'Terminated', got '%s'", status)
-	}
-
-	// Mixed state.
-	pod = &api.Pod{
-		DesiredState: desiredState,
-		CurrentState: api.PodState{
-			Info: map[string]docker.Container{
-				"containerA": runningState,
-				"containerB": stoppedState,
-			},
-			Host: "machine",
-		},
-	}
-	status = getPodStatus(pod)
-	if status != api.PodWaiting {
-		t.Errorf("Expected 'Waiting', got '%s'", status)
-	}
-
-	// Mixed state.
-	pod = &api.Pod{
-		DesiredState: desiredState,
-		CurrentState: api.PodState{
-			Info: map[string]docker.Container{
-				"containerA": runningState,
-			},
-			Host: "machine",
-		},
-	}
-	status = getPodStatus(pod)
-	if status != api.PodWaiting {
-		t.Errorf("Expected 'Waiting', got '%s'", status)
+	for _, test := range tests {
+		if status, err := getPodStatus(test.pod, &fakeClient); status != test.status {
+			t.Errorf("In test %s, expected %v, got %v", test.test, test.status, status)
+			if err != nil {
+				t.Errorf("In test %s, unexpected error: %v", test.test, err)
+			}
+		}
 	}
 }