Merge pull request #5631 from dchen1107/podstatus

Remove PodIP from ContainerStatus

pkg/api/types.go

@@ -457,9 +457,6 @@ type ContainerStatus struct {
 	// Note that this is calculated from dead containers.  But those containers are subject to
 	// garbage collection.  This value will get capped at 5 by GC.
 	RestartCount int `json:"restartCount"`
-	// TODO(dchen1107): Deprecated this soon once we pull entire PodStatus from node,
-	// not just PodInfo. Now we need this to remove docker.Container from API
-	PodIP string `json:"podIP,omitempty"`
 	// TODO(dchen1107): Need to decide how to represent this in v1beta3
 	Image       string `json:"image"`
 	ImageID     string `json:"imageID" description:"ID of the container's image"`

pkg/api/v1beta1/types.go

@@ -437,9 +437,6 @@ type ContainerStatus struct {
 	// Note that this is calculated from dead containers.  But those containers are subject to
 	// garbage collection.  This value will get capped at 5 by GC.
 	RestartCount int `json:"restartCount" description:"the number of times the container has been restarted, currently based on the number of dead containers that have not yet been removed"`
-	// TODO(dchen1107): Deprecated this soon once we pull entire PodStatus from node,
-	// not just PodInfo. Now we need this to remove docker.Container from API
-	PodIP string `json:"podIP,omitempty" description:"pod's IP address"`
 	// TODO(dchen1107): Need to decide how to reprensent this in v1beta3
 	Image       string `json:"image" description:"image of the container"`
 	ImageID     string `json:"imageID" description:"ID of the container's image"`

pkg/api/v1beta2/types.go

@@ -434,9 +434,6 @@ type ContainerStatus struct {
 	// Note that this is calculated from dead containers.  But those containers are subject to
 	// garbage collection.  This value will get capped at 5 by GC.
 	RestartCount int `json:"restartCount" description:"the number of times the container has been restarted, currently based on the number of dead containers that have not yet been removed"`
-	// TODO(dchen1107): Deprecated this soon once we pull entire PodStatus from node,
-	// not just PodInfo. Now we need this to remove docker.Container from API
-	PodIP string `json:"podIP,omitempty" description:"pod's IP address"`
 	// TODO(dchen1107): Need to decide how to reprensent this in v1beta3
 	Image       string `json:"image" description:"image of the container"`
 	ImageID     string `json:"imageID" description:"ID of the container's image"`

pkg/api/v1beta3/types.go

@@ -468,9 +468,6 @@ type ContainerStatus struct {
 	// Note that this is calculated from dead containers.  But those containers are subject to
 	// garbage collection.  This value will get capped at 5 by GC.
 	RestartCount int `json:"restartCount" description:"the number of times the container has been restarted, currently based on the number of dead containers that have not yet been removed"`
-	// The IP of the Pod
-	// PodIP is deprecated and will be removed from v1beta3 once it becomes possible for the Kubelet to report PodStatus.
-	PodIP string `json:"podIP,omitempty" description:"pod's IP address"`
 	// TODO(dchen1107): Which image the container is running with?
 	// The image the container is running
 	Image       string `json:"image" description:"image of the container"`

pkg/kubelet/dockertools/docker.go

@@ -528,19 +528,29 @@ var (
 	ErrContainerCannotRun = errors.New("Container cannot run")
 )
 
-func inspectContainer(client DockerInterface, dockerID, containerName, tPath string) (*api.ContainerStatus, error) {
+// Internal information kept for containers from inspection
+type containerStatusResult struct {
+	status api.ContainerStatus
+	ip     string
+	err    error
+}
+
+func inspectContainer(client DockerInterface, dockerID, containerName, tPath string) *containerStatusResult {
+	result := containerStatusResult{api.ContainerStatus{}, "", nil}
+
 	inspectResult, err := client.InspectContainer(dockerID)
 
 	if err != nil {
-		return nil, err
+		result.err = err
+		return &result
 	}
 	if inspectResult == nil {
 		// Why did we not get an error?
-		return &api.ContainerStatus{}, nil
+		return &result
 	}
 
 	glog.V(3).Infof("Container inspect result: %+v", *inspectResult)
-	containerStatus := api.ContainerStatus{
+	result.status = api.ContainerStatus{
 		Image:       inspectResult.Config.Image,
 		ImageID:     DockerPrefix + inspectResult.Image,
 		ContainerID: DockerPrefix + dockerID,
@@ -548,11 +558,11 @@ func inspectContainer(client DockerInterface, dockerID, containerName, tPath str
 
 	waiting := true
 	if inspectResult.State.Running {
-		containerStatus.State.Running = &api.ContainerStateRunning{
+		result.status.State.Running = &api.ContainerStateRunning{
 			StartedAt: util.NewTime(inspectResult.State.StartedAt),
 		}
 		if containerName == PodInfraContainerName && inspectResult.NetworkSettings != nil {
-			containerStatus.PodIP = inspectResult.NetworkSettings.IPAddress
+			result.ip = inspectResult.NetworkSettings.IPAddress
 		}
 		waiting = false
 	} else if !inspectResult.State.FinishedAt.IsZero() {
@@ -565,7 +575,7 @@ func inspectContainer(client DockerInterface, dockerID, containerName, tPath str
 		} else {
 			reason = inspectResult.State.Error
 		}
-		containerStatus.State.Termination = &api.ContainerStateTerminated{
+		result.status.State.Termination = &api.ContainerStateTerminated{
 			ExitCode:   inspectResult.State.ExitCode,
 			Reason:     reason,
 			StartedAt:  util.NewTime(inspectResult.State.StartedAt),
@@ -578,7 +588,7 @@ func inspectContainer(client DockerInterface, dockerID, containerName, tPath str
 				if err != nil {
 					glog.Errorf("Error on reading termination-log %s: %v", path, err)
 				} else {
-					containerStatus.State.Termination.Message = string(data)
+					result.status.State.Termination.Message = string(data)
 				}
 			}
 		}
@@ -589,18 +599,20 @@ func inspectContainer(client DockerInterface, dockerID, containerName, tPath str
 		// TODO(dchen1107): Separate issue docker/docker#8294 was filed
 		// TODO(dchen1107): Need to figure out why we are still waiting
 		// Check any issue to run container
-		containerStatus.State.Waiting = &api.ContainerStateWaiting{
+		result.status.State.Waiting = &api.ContainerStateWaiting{
 			Reason: ErrContainerCannotRun.Error(),
 		}
 	}
 
-	return &containerStatus, nil
+	return &result
 }
 
-// GetDockerPodInfo returns docker info for all containers in the pod/manifest and
+// GetDockerPodStatus returns docker related status for all containers in the pod/manifest and
 // infrastructure container
-func GetDockerPodInfo(client DockerInterface, manifest api.PodSpec, podFullName string, uid types.UID) (api.PodInfo, error) {
+func GetDockerPodStatus(client DockerInterface, manifest api.PodSpec, podFullName string, uid types.UID) (*api.PodStatus, error) {
-	info := api.PodInfo{}
+	var podStatus api.PodStatus
+	podStatus.Info = api.PodInfo{}
+
 	expectedContainers := make(map[string]api.Container)
 	for _, container := range manifest.Containers {
 		expectedContainers[container.Name] = container
@@ -635,34 +647,35 @@ func GetDockerPodInfo(client DockerInterface, manifest api.PodSpec, podFullName
 			terminationMessagePath = c.TerminationMessagePath
 		}
 		// We assume docker return us a list of containers in time order
-		if containerStatus, found := info[dockerContainerName]; found {
+		if containerStatus, found := podStatus.Info[dockerContainerName]; found {
 			containerStatus.RestartCount += 1
-			info[dockerContainerName] = containerStatus
+			podStatus.Info[dockerContainerName] = containerStatus
 			continue
 		}
 
-		containerStatus, err := inspectContainer(client, value.ID, dockerContainerName, terminationMessagePath)
+		result := inspectContainer(client, value.ID, dockerContainerName, terminationMessagePath)
-		if err != nil {
+		if result.err != nil {
 			return nil, err
 		}
-		info[dockerContainerName] = *containerStatus
+		// Add user container information
+		if dockerContainerName == PodInfraContainerName {
+			// Found network container
+			podStatus.PodIP = result.ip
+		} else {
+			podStatus.Info[dockerContainerName] = result.status
+		}
 	}
 
-	if len(info) == 0 {
+	if len(podStatus.Info) == 0 && podStatus.PodIP == "" {
 		return nil, ErrNoContainersInPod
 	}
 
-	// First make sure we are not missing pod infra container
+	// Not all containers expected are created, check if there are
-	if _, found := info[PodInfraContainerName]; !found {
+	// image related issues
-		return nil, ErrNoPodInfraContainerInPod
+	if len(podStatus.Info) < len(manifest.Containers) {
-	}
-
-	if len(info) < (len(manifest.Containers) + 1) {
 		var containerStatus api.ContainerStatus
-		// Not all containers expected are created, check if there are
-		// image related issues
 		for _, container := range manifest.Containers {
-			if _, found := info[container.Name]; found {
+			if _, found := podStatus.Info[container.Name]; found {
 				continue
 			}
 
@@ -684,11 +697,11 @@ func GetDockerPodInfo(client DockerInterface, manifest api.PodSpec, podFullName
 				}
 			}
 
-			info[container.Name] = containerStatus
+			podStatus.Info[container.Name] = containerStatus
 		}
 	}
 
-	return info, nil
+	return &podStatus, nil
 }
 
 const containerNamePrefix = "k8s"

pkg/kubelet/handlers.go

@@ -24,7 +24,6 @@ import (
 	"strconv"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/golang/glog"
@@ -77,11 +76,10 @@ func (h *httpActionHandler) Run(podFullName string, uid types.UID, container *ap
 			glog.Errorf("Unable to get pod info, event handlers may be invalid.")
 			return err
 		}
-		netInfo, found := status.Info[dockertools.PodInfraContainerName]
+		if status.PodIP == "" {
-		if !found {
 			return fmt.Errorf("failed to find networking container: %v", status)
 		}
-		host = netInfo.PodIP
+		host = status.PodIP
 	}
 	var port int
 	if handler.HTTPGet.Port.Kind == util.IntstrString && len(handler.HTTPGet.Port.StrVal) == 0 {

pkg/kubelet/kubelet.go

@@ -2015,44 +2015,40 @@ func (kl *Kubelet) GetPodStatus(podFullName string, uid types.UID) (api.PodStatu
 
 func (kl *Kubelet) generatePodStatus(podFullName string, uid types.UID) (api.PodStatus, error) {
 	glog.V(3).Infof("Generating status for %s", podFullName)
-	var podStatus api.PodStatus
+
 	spec, found := kl.GetPodByFullName(podFullName)
 	if !found {
-		return podStatus, fmt.Errorf("Couldn't find spec for pod %s", podFullName)
+		return api.PodStatus{}, fmt.Errorf("Couldn't find spec for pod %s", podFullName)
 	}
 
-	info, err := dockertools.GetDockerPodInfo(kl.dockerClient, *spec, podFullName, uid)
+	podStatus, err := dockertools.GetDockerPodStatus(kl.dockerClient, *spec, podFullName, uid)
 
 	if err != nil {
 		// Error handling
 		glog.Infof("Query docker container info for pod %s failed with error (%v)", podFullName, err)
 		if strings.Contains(err.Error(), "resource temporarily unavailable") {
 			// Leave upstream layer to decide what to do
-			return podStatus, err
+			return api.PodStatus{}, err
 		} else {
-			podStatus.Phase = api.PodPending
+			pendingStatus := api.PodStatus{
-			podStatus.Message = fmt.Sprintf("Query docker container info failed with error (%v)", err)
+				Phase:   api.PodPending,
-			return podStatus, nil
+				Message: fmt.Sprintf("Query docker container info failed with error (%v)", err),
+			}
+			return pendingStatus, nil
 		}
 	}
 
 	// Assume info is ready to process
-	podStatus.Phase = getPhase(spec, info)
+	podStatus.Phase = getPhase(spec, podStatus.Info)
-	podStatus.Info = api.PodInfo{}
 	for _, c := range spec.Containers {
-		containerStatus := info[c.Name]
+		containerStatus := podStatus.Info[c.Name]
 		containerStatus.Ready = kl.readiness.IsReady(containerStatus)
 		podStatus.Info[c.Name] = containerStatus
 	}
 	podStatus.Conditions = append(podStatus.Conditions, getPodReadyCondition(spec, podStatus.Info)...)
-
-	netContainerInfo, found := info[dockertools.PodInfraContainerName]
-	if found {
-		podStatus.PodIP = netContainerInfo.PodIP
-	}
 	podStatus.Host = kl.hostname
 
-	return podStatus, nil
+	return *podStatus, nil
 }
 
 // Returns logs of current machine.

pkg/kubelet/kubelet_test.go

@@ -636,7 +636,7 @@ func TestSyncPodsWithPodInfraCreatesContainer(t *testing.T) {
 	waitGroup.Wait()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "list", "inspect_container", "inspect_image", "list", "create", "start", "list", "inspect_container", "inspect_container"})
+		"list", "list", "list", "inspect_container", "list", "create", "start", "list", "inspect_container", "inspect_container"})
 
 	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 1 ||
@@ -693,7 +693,7 @@ func TestSyncPodsWithPodInfraCreatesContainerCallsHandler(t *testing.T) {
 	waitGroup.Wait()
 
 	verifyCalls(t, fakeDocker, []string{
-		"list", "list", "list", "inspect_container", "inspect_image", "list", "create", "start", "list", "inspect_container", "inspect_container"})
+		"list", "list", "list", "inspect_container", "list", "create", "start", "list", "inspect_container", "inspect_container"})
 
 	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 1 ||