Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial support to propagating the termination reasons and image failures #1569

Merged
merged 5 commits into from
Oct 6, 2014
Merged

Initial support to propagating the termination reasons and image failures #1569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9861eb7
Initial support of propogating the termination reasons and image failure
dchen1107 Oct 3, 2014
8d0ed93
Clean unittests
dchen1107 Oct 3, 2014
39f648b
Fix rebase issues.
dchen1107 Oct 3, 2014
a86d496
Fix e2e tests.
dchen1107 Oct 3, 2014
4fdfeaa
Addressed comments.
dchen1107 Oct 6, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/e2e-suite/basic.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ while [ $ALL_RUNNING -ne 1 ]; do
ALL_RUNNING=1
for id in $POD_ID_LIST; do
CURRENT_STATUS=$($KUBECFG -template '{{and .CurrentState.Info.mynginx.State.Running .CurrentState.Info.net.State.Running}}' get pods/$id)
if [ "$CURRENT_STATUS" != "{}" ]; then
if [ "$CURRENT_STATUS" != "{0001-01-01 00:00:00 +0000 UTC}" ]; then
ALL_RUNNING=0
fi
done
Expand Down
4 changes: 2 additions & 2 deletions hack/e2e-suite/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ function validate() {
local template_string current_status current_image host_ip
template_string="{{and ((index .CurrentState.Info \"${CONTROLLER_NAME}\").State.Running) .CurrentState.Info.net.State.Running}}"
current_status=$($KUBECFG -template="${template_string}" get "pods/$id")
if [[ "$current_status" != "{}" ]]; then
if [ "$current_status" != "{0001-01-01 00:00:00 +0000 UTC}" ]; then
echo " $id is created but not running"
continue
fi

template_string="{{(index .CurrentState.Info \"${CONTROLLER_NAME}\").DetailInfo.Config.Image}}"
template_string="{{(index .CurrentState.Info \"${CONTROLLER_NAME}\").Image}}"
current_image=$($KUBECFG -template="${template_string}" get "pods/$id")
if [[ "$current_image" != "${DOCKER_HUB_USER}/update-demo:${container_image_version}" ]]; then
echo " ${id} is created but running wrong image"
Expand Down
20 changes: 12 additions & 8 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ package api

import (
"strings"
"time"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)

// Common string formats
Expand Down Expand Up @@ -302,12 +302,15 @@ type ContainerStateWaiting struct {
}

type ContainerStateRunning struct {
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
}

type ContainerStateTerminated struct {
ExitCode int `json:"exitCode,omitempty" yaml:"exitCode,omitempty"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
ExitCode int `json:"exitCode" yaml:"exitCode"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
FinishedAt time.Time `json:"finishedAt,omitempty" yaml:"finishedAt,omitempty"`
}

type ContainerState struct {
Expand All @@ -323,12 +326,13 @@ type ContainerStatus struct {
// defined for container?
State ContainerState `json:"state,omitempty" yaml:"state,omitempty"`
RestartCount int `json:"restartCount" yaml:"restartCount"`
// TODO(dchen1107): Introduce our own NetworkSettings struct here?
// 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" yaml:"podIP,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need PodIP? It's already present in the PodState, also since IP is specific to pod not container, why is it in the ContainerState?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need PodIP today because apiserver only pulls PodInfo from kubelet, not CurrentState which defined as PodState. We used to find PodIP from docker.Container -- net's NetworkSettings.IPAddress. This PR deprecated docker.Container completely from our APIs. My initial PR actually wanted to pulls the entire CurrentState from kubelet, but that is too big change, and hard to review; also we changed the API to use PodStatus instead of PodState in v1beta3; thus, at the end, I decided not include such change in this PR.

// TODO(dchen1107): Need to decide how to represent this in v1beta3
Image string `yaml:"image" json:"image"`
// TODO(dchen1107): Once we have done with integration with cadvisor, resource
// usage should be included.
// TODO(dchen1107): In long run, I think we should replace this with our own struct to remove
// the dependency on docker.
DetailInfo docker.Container `json:"detailInfo,omitempty" yaml:"detailInfo,omitempty"`
}

// PodInfo contains one entry for every container with available info.
Expand Down
21 changes: 13 additions & 8 deletions pkg/api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package v1beta1

import (
"time"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)

// Common string formats
Expand Down Expand Up @@ -286,12 +287,15 @@ type ContainerStateWaiting struct {
}

type ContainerStateRunning struct {
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
}

type ContainerStateTerminated struct {
ExitCode int `json:"exitCode,omitempty" yaml:"exitCode,omitempty"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
ExitCode int `json:"exitCode" yaml:"exitCode"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
FinishedAt time.Time `json:"finishedAt,omitempty" yaml:"finishedAt,omitempty"`
}

type ContainerState struct {
Expand All @@ -307,12 +311,13 @@ type ContainerStatus struct {
// defined for container?
State ContainerState `json:"state,omitempty" yaml:"state,omitempty"`
RestartCount int `json:"restartCount" yaml:"restartCount"`
// TODO(dchen1107): Introduce our own NetworkSettings struct here?
// 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" yaml:"podIP,omitempty"`
// TODO(dchen1107): Need to decide how to reprensent this in v1beta3
Image string `yaml:"image" json:"image"`
// TODO(dchen1107): Once we have done with integration with cadvisor, resource
// usage should be included.
// TODO(dchen1107): In long run, I think we should replace this with our own struct to remove
// the dependency on docker.
DetailInfo docker.Container `json:"detailInfo,omitempty" yaml:"detailInfo,omitempty"`
}

// PodInfo contains one entry for every container with available info.
Expand Down
22 changes: 13 additions & 9 deletions pkg/api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package v1beta2

import (
"time"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)

// Common string formats
Expand Down Expand Up @@ -282,12 +283,15 @@ type ContainerStateWaiting struct {
}

type ContainerStateRunning struct {
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
}

type ContainerStateTerminated struct {
ExitCode int `json:"exitCode,omitempty" yaml:"exitCode,omitempty"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
ExitCode int `json:"exitCode" yaml:"exitCode"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
FinishedAt time.Time `json:"finishedAt,omitempty" yaml:"finishedAt,omitempty"`
}

type ContainerState struct {
Expand All @@ -303,16 +307,16 @@ type ContainerStatus struct {
// defined for container?
State ContainerState `json:"state,omitempty" yaml:"state,omitempty"`
RestartCount int `json:"restartCount" yaml:"restartCount"`
// TODO(dchen1107): Introduce our own NetworkSettings struct here?
// 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" yaml:"podIP,omitempty"`
// TODO(dchen1107): Need to decide how to reprensent this in v1beta3
Image string `yaml:"image" json:"image"`
// TODO(dchen1107): Once we have done with integration with cadvisor, resource
// usage should be included.
// TODO(dchen1107): In long run, I think we should replace this with our own struct to remove
// the dependency on docker.
DetailInfo docker.Container `json:"detailInfo,omitempty" yaml:"detailInfo,omitempty"`
}

// PodInfo contains one entry for every container with available info.
// TODO(dchen1107): Replace docker.Container below with ContainerStatus defined above.
type PodInfo map[string]ContainerStatus

type RestartPolicyAlways struct{}
Expand Down
18 changes: 10 additions & 8 deletions pkg/api/v1beta3/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ limitations under the License.
package v1beta3

import (
"time"

"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)

// Common string formats
Expand Down Expand Up @@ -312,12 +313,15 @@ type ContainerStateWaiting struct {
}

type ContainerStateRunning struct {
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
}

type ContainerStateTerminated struct {
ExitCode int `json:"exitCode,omitempty" yaml:"exitCode,omitempty"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
ExitCode int `json:"exitCode" yaml:"exitCode"`
Signal int `json:"signal,omitempty" yaml:"signal,omitempty"`
Reason string `json:"reason,omitempty" yaml:"reason,omitempty"`
StartedAt time.Time `json:"startedAt,omitempty" yaml:"startedAt,omitempty"`
FinishedAt time.Time `json:"finishedAt,omitempty" yaml:"finishedAt,omitempty"`
}

type ContainerState struct {
Expand All @@ -334,16 +338,14 @@ type ContainerStatus struct {
State ContainerState `json:"state,omitempty" yaml:"state,omitempty"`
RestartCount int `json:"restartCount" yaml:"restartCount"`
// TODO(dchen1107): Introduce our own NetworkSettings struct here?
// TODO(dchen1107): Which image the container is running with?
// TODO(dchen1107): Once we have done with integration with cadvisor, resource
// usage should be included.
// TODO(dchen1107): In long run, I think we should replace this with our own struct to remove
// the dependency on docker.
DetailInfo docker.Container `json:"detailInfo,omitempty" yaml:"detailInfo,omitempty"`
}

// PodInfo contains one entry for every container with available info.
// TODO(dchen1107): Replace docker.Container below with ContainerStatus defined above.
type PodInfo map[string]docker.Container
type PodInfo map[string]ContainerStatus

type RestartPolicyAlways struct{}

Expand Down
12 changes: 3 additions & 9 deletions pkg/client/podinfo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,11 @@ import (

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/fsouza/go-dockerclient"
)

func TestHTTPPodInfoGetter(t *testing.T) {
expectObj := api.PodInfo{
"myID": api.ContainerStatus{
DetailInfo: docker.Container{ID: "myID"},
},
"myID": api.ContainerStatus{},
}
body, err := json.Marshal(expectObj)
if err != nil {
Expand Down Expand Up @@ -69,17 +66,14 @@ func TestHTTPPodInfoGetter(t *testing.T) {
}

// reflect.DeepEqual(expectObj, gotObj) doesn't handle blank times well
if len(gotObj) != len(expectObj) ||
expectObj["myID"].DetailInfo.ID != gotObj["myID"].DetailInfo.ID {
if len(gotObj) != len(expectObj) {
t.Errorf("Unexpected response. Expected: %#v, received %#v", expectObj, gotObj)
}
}

func TestHTTPPodInfoGetterNotFound(t *testing.T) {
expectObj := api.PodInfo{
"myID": api.ContainerStatus{
DetailInfo: docker.Container{ID: "myID"},
},
"myID": api.ContainerStatus{},
}
_, err := json.Marshal(expectObj)
if err != nil {
Expand Down
101 changes: 87 additions & 14 deletions pkg/kubelet/dockertools/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,30 +270,66 @@ func GetKubeletDockerContainerLogs(client DockerInterface, containerID, tail str
return
}

func generateContainerStatus(inspectResult *docker.Container) api.ContainerStatus {
var (
// ErrNoContainersInPod is returned when there are no containers for a given pod
ErrNoContainersInPod = errors.New("no containers exist for this pod")

// ErrNoNetworkContainerInPod is returned when there is no network container for a given pod
ErrNoNetworkContainerInPod = errors.New("No network container exists for this pod")

// ErrContainerCannotRun is returned when a container is created, but cannot run properly
ErrContainerCannotRun = errors.New("Container cannot run")
)

func inspectContainer(client DockerInterface, dockerID, containerName string) (*api.ContainerStatus, error) {
inspectResult, err := client.InspectContainer(dockerID)
if err != nil {
return nil, err
}
if inspectResult == nil {
// Why did we not get an error?
return api.ContainerStatus{}
return &api.ContainerStatus{}, nil
}

var containerStatus api.ContainerStatus
glog.V(3).Infof("Container: %s [%s] inspect result %+v", *inspectResult)
containerStatus := api.ContainerStatus{
Image: inspectResult.Config.Image,
}

waiting := true
if inspectResult.State.Running {
containerStatus.State.Running = &api.ContainerStateRunning{}
} else {
containerStatus.State.Running = &api.ContainerStateRunning{
StartedAt: inspectResult.State.StartedAt,
}
if containerName == "net" && inspectResult.NetworkSettings != nil {
containerStatus.PodIP = inspectResult.NetworkSettings.IPAddress
}
waiting = false
} else if !inspectResult.State.FinishedAt.IsZero() {
// TODO(dchen1107): Integrate with event to provide a better reason
containerStatus.State.Termination = &api.ContainerStateTerminated{
ExitCode: inspectResult.State.ExitCode,
ExitCode: inspectResult.State.ExitCode,
Reason: "",
StartedAt: inspectResult.State.StartedAt,
FinishedAt: inspectResult.State.FinishedAt,
}
waiting = false
}

if waiting {
// 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{
Reason: ErrContainerCannotRun.Error(),
}
}
containerStatus.DetailInfo = *inspectResult
return containerStatus
}

// ErrNoContainersInPod is returned when there are no containers for a given pod
var ErrNoContainersInPod = errors.New("no containers exist for this pod")
return &containerStatus, nil
}

// GetDockerPodInfo returns docker info for all containers in the pod/manifest.
func GetDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.PodInfo, error) {
func GetDockerPodInfo(client DockerInterface, manifest api.ContainerManifest, podFullName, uuid string) (api.PodInfo, error) {
info := api.PodInfo{}

containers, err := client.ListContainers(docker.ListContainersOptions{All: true})
Expand All @@ -316,16 +352,53 @@ func GetDockerPodInfo(client DockerInterface, podFullName, uuid string) (api.Pod
continue
}

inspectResult, err := client.InspectContainer(value.ID)
containerStatus, err := inspectContainer(client, value.ID, dockerContainerName)
if err != nil {
return nil, err
}
info[dockerContainerName] = generateContainerStatus(inspectResult)
info[dockerContainerName] = *containerStatus
}

if len(info) == 0 {
return nil, ErrNoContainersInPod
}

// First make sure we are not missing network container
if _, found := info["net"]; !found {
return nil, ErrNoNetworkContainerInPod
}

if len(info) < (len(manifest.Containers) + 1) {
var containerStatus api.ContainerStatus
// Not all containers expected are created, verify if there are
// image related issues
for _, container := range manifest.Containers {
if _, found := info[container.Name]; found {
continue
}

image := container.Image
// Check image is ready on the node or not
// TODO(dchen1107): docker/docker/issues/8365 to figure out if the image exists
_, err := client.InspectImage(image)
if err == nil {
containerStatus.State.Waiting = &api.ContainerStateWaiting{
Reason: fmt.Sprintf("Image: %s is ready, container is creating", image),
}
} else if err == docker.ErrNoSuchImage {
containerStatus.State.Waiting = &api.ContainerStateWaiting{
Reason: fmt.Sprintf("Image: %s is not ready on the node", image),
}
} else {
containerStatus.State.Waiting = &api.ContainerStateWaiting{
Reason: err.Error(),
}
}

info[container.Name] = containerStatus
}
}

return info, nil
}

Expand Down