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.

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

Start using kubelet internal PodStatus #17420

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 contrib/mesos/pkg/executor/service/service.go
Expand Up @@ -97,7 +97,7 @@ func (s *KubeletExecutorServer) runExecutor(
return nil, fmt.Errorf("PodStatucFunc called before kubelet is initialized")
}

status, err := s.klet.GetRuntime().GetPodStatus(pod)
status, err := s.klet.GetRuntime().GetAPIPodStatus(pod)
if err != nil {
return nil, err
}
Expand Down
35 changes: 23 additions & 12 deletions pkg/kubelet/container/fake_runtime.go
Expand Up @@ -36,8 +36,8 @@ type FakeRuntime struct {
PodList []*Pod
AllPodList []*Pod
ImageList []Image
PodStatus api.PodStatus
RawPodStatus RawPodStatus
APIPodStatus api.PodStatus
PodStatus PodStatus
StartedPods []string
KilledPods []string
StartedContainers []string
Expand Down Expand Up @@ -93,7 +93,7 @@ func (f *FakeRuntime) ClearCalls() {
f.CalledFunctions = []string{}
f.PodList = []*Pod{}
f.AllPodList = []*Pod{}
f.PodStatus = api.PodStatus{}
f.APIPodStatus = api.PodStatus{}
f.StartedPods = []string{}
f.KilledPods = []string{}
f.StartedContainers = []string{}
Expand Down Expand Up @@ -165,7 +165,7 @@ func (f *FakeRuntime) GetPods(all bool) ([]*Pod, error) {
return f.PodList, f.Err
}

func (f *FakeRuntime) SyncPod(pod *api.Pod, _ Pod, _ api.PodStatus, _ []api.Secret, backOff *util.Backoff) error {
func (f *FakeRuntime) SyncPod(pod *api.Pod, _ Pod, _ api.PodStatus, _ *PodStatus, _ []api.Secret, backOff *util.Backoff) error {
f.Lock()
defer f.Unlock()

Expand Down Expand Up @@ -223,7 +223,16 @@ func (f *FakeRuntime) KillContainerInPod(container api.Container, pod *api.Pod)
return f.Err
}

func (f *FakeRuntime) GetPodStatus(*api.Pod) (*api.PodStatus, error) {
func (f *FakeRuntime) GetAPIPodStatus(*api.Pod) (*api.PodStatus, error) {
f.Lock()
defer f.Unlock()

f.CalledFunctions = append(f.CalledFunctions, "GetAPIPodStatus")
status := f.APIPodStatus
return &status, f.Err
}

func (f *FakeRuntime) GetPodStatus(uid types.UID, name, namespace string) (*PodStatus, error) {
f.Lock()
defer f.Unlock()

Expand All @@ -232,22 +241,24 @@ func (f *FakeRuntime) GetPodStatus(*api.Pod) (*api.PodStatus, error) {
return &status, f.Err
}

func (f *FakeRuntime) GetRawPodStatus(uid types.UID, name, namespace string) (*RawPodStatus, error) {
func (f *FakeRuntime) ConvertPodStatusToAPIPodStatus(_ *api.Pod, _ *PodStatus) (*api.PodStatus, error) {
f.Lock()
defer f.Unlock()

f.CalledFunctions = append(f.CalledFunctions, "GetRawPodStatus")
status := f.RawPodStatus
f.CalledFunctions = append(f.CalledFunctions, "ConvertPodStatusToAPIPodStatus")
status := f.APIPodStatus
return &status, f.Err
}

func (f *FakeRuntime) ConvertRawToPodStatus(_ *api.Pod, _ *RawPodStatus) (*api.PodStatus, error) {
func (f *FakeRuntime) GetPodStatusAndAPIPodStatus(_ *api.Pod) (*PodStatus, *api.PodStatus, error) {
f.Lock()
defer f.Unlock()

f.CalledFunctions = append(f.CalledFunctions, "ConvertRawToPodStatus")
status := f.PodStatus
return &status, f.Err
// This is only a temporary function, it should be logged as GetAPIPodStatus
f.CalledFunctions = append(f.CalledFunctions, "GetAPIPodStatus")
apiPodStatus := f.APIPodStatus
podStatus := f.PodStatus
return &podStatus, &apiPodStatus, f.Err
}

func (f *FakeRuntime) ExecInContainer(containerID ContainerID, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error {
Expand Down
56 changes: 55 additions & 1 deletion pkg/kubelet/container/helpers.go
Expand Up @@ -43,7 +43,37 @@ type RunContainerOptionsGenerator interface {

// ShouldContainerBeRestarted checks whether a container needs to be restarted.
// TODO(yifan): Think about how to refactor this.
func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatus *api.PodStatus) bool {
func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatus *PodStatus) bool {
podFullName := GetPodFullName(pod)

// Get all dead container status.
var resultStatus []*ContainerStatus
for _, containerStatus := range podStatus.ContainerStatuses {
if containerStatus.Name == container.Name && containerStatus.State == ContainerStateExited {
resultStatus = append(resultStatus, containerStatus)
}
}

// Check RestartPolicy for dead container.
if len(resultStatus) > 0 {
if pod.Spec.RestartPolicy == api.RestartPolicyNever {
glog.V(4).Infof("Already ran container %q of pod %q, do nothing", container.Name, podFullName)
return false
}
if pod.Spec.RestartPolicy == api.RestartPolicyOnFailure {
// Check the exit code of last run. Note: This assumes the result is sorted
// by the created time in reverse order.
if resultStatus[0].ExitCode == 0 {
glog.V(4).Infof("Already successfully ran container %q of pod %q, do nothing", container.Name, podFullName)
return false
}
}
}
return true
}

// TODO (random-liu) This should be removed soon after rkt implements GetPodStatus.
func ShouldContainerBeRestartedOldVersion(container *api.Container, pod *api.Pod, podStatus *api.PodStatus) bool {
podFullName := GetPodFullName(pod)

// Get all dead container status.
Expand Down Expand Up @@ -72,6 +102,30 @@ func ShouldContainerBeRestarted(container *api.Container, pod *api.Pod, podStatu
return true
}

// TODO (random-liu) Convert PodStatus to running Pod, should be deprecated soon
func ConvertPodStatusToRunningPod(podStatus *PodStatus) Pod {
runningPod := Pod{
ID: podStatus.ID,
Name: podStatus.Name,
Namespace: podStatus.Namespace,
}
for _, containerStatus := range podStatus.ContainerStatuses {
if containerStatus.State != ContainerStateRunning {
continue
}
container := &Container{
ID: containerStatus.ID,
Name: containerStatus.Name,
Image: containerStatus.Image,
Hash: containerStatus.Hash,
Created: containerStatus.CreatedAt.Unix(),
State: containerStatus.State,
}
runningPod.Containers = append(runningPod.Containers, container)
}
return runningPod
}

// HashContainer returns the hash of the container. It is used to compare
// the running container with its desired spec.
func HashContainer(container *api.Container) uint64 {
Expand Down
80 changes: 51 additions & 29 deletions pkg/kubelet/container/runtime.go
Expand Up @@ -85,27 +85,27 @@ type Runtime interface {
// GarbageCollect removes dead containers using the specified container gc policy
GarbageCollect(gcPolicy ContainerGCPolicy) error
// Syncs the running pod into the desired pod.
SyncPod(pod *api.Pod, runningPod Pod, podStatus api.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error
// TODO (random-liu) The runningPod will be removed after #17420 is done.
SyncPod(pod *api.Pod, runningPod Pod, apiPodStatus api.PodStatus, podStatus *PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error
// KillPod kills all the containers of a pod. Pod may be nil, running pod must not be.
KillPod(pod *api.Pod, runningPod Pod) error
// GetPodStatus retrieves the status of the pod, including the information of
// GetAPIPodStatus retrieves the api.PodStatus of the pod, including the information of
// all containers in the pod. Clients of this interface assume the
// containers' statuses in a pod always have a deterministic ordering
// (e.g., sorted by name).
// TODO: Rename this to GetAPIPodStatus, and eventually deprecate the
// function in favor of GetRawPodStatus.
GetPodStatus(*api.Pod) (*api.PodStatus, error)
// GetRawPodStatus retrieves the status of the pod, including the
GetAPIPodStatus(*api.Pod) (*api.PodStatus, error)
// GetPodStatus retrieves the status of the pod, including the
// information of all containers in the pod that are visble in Runtime.
// TODO: Rename this to GetPodStatus to replace the original function.
GetRawPodStatus(uid types.UID, name, namespace string) (*RawPodStatus, error)
// ConvertRawToPodStatus converts the RawPodStatus object to api.PodStatus.
GetPodStatus(uid types.UID, name, namespace string) (*PodStatus, error)
// ConvertPodStatusToAPIPodStatus converts the PodStatus object to api.PodStatus.
// This function is needed because Docker generates some high-level and/or
// pod-level information for api.PodStatus (e.g., check whether the image
// exists to determine the reason).
// TODO: Deprecate this function once we generalize the logic for all
// container runtimes in kubelet.
ConvertRawToPodStatus(*api.Pod, *RawPodStatus) (*api.PodStatus, error)
// exists to determine the reason). We should try generalizing the logic
// for all container runtimes in kubelet and remove this funciton.
ConvertPodStatusToAPIPodStatus(*api.Pod, *PodStatus) (*api.PodStatus, error)
// Return both PodStatus and api.PodStatus, this is just a temporary function.
// TODO (random-liu) Remove this method later
GetPodStatusAndAPIPodStatus(*api.Pod) (*PodStatus, *api.PodStatus, error)
// PullImage pulls an image from the network to local storage using the supplied
// secrets if necessary.
PullImage(image ImageSpec, pullSecrets []api.Secret) error
Expand Down Expand Up @@ -213,17 +213,17 @@ func (c *ContainerID) UnmarshalJSON(data []byte) error {
return c.ParseString(string(data))
}

type ContainerStatus string
type ContainerState string

const (
ContainerStatusRunning ContainerStatus = "running"
ContainerStatusExited ContainerStatus = "exited"
// This unknown encompasses all the statuses that we currently don't care.
ContainerStatusUnknown ContainerStatus = "unknown"
ContainerStateRunning ContainerState = "running"
ContainerStateExited ContainerState = "exited"
// This unknown encompasses all the states that we currently don't care.
ContainerStateUnknown ContainerState = "unknown"
)

// Container provides the runtime information for a container, such as ID, hash,
// status of the container.
// state of the container.
type Container struct {
// The ID of the container, used by the container runtime to identify
// a container.
Expand All @@ -239,13 +239,13 @@ type Container struct {
// The timestamp of the creation time of the container.
// TODO(yifan): Consider to move it to api.ContainerStatus.
Created int64
// Status is the status of the container.
Status ContainerStatus
// State is the state of the container.
State ContainerState
}

// RawPodStatus represents the status of the pod and its containers.
// api.PodStatus can be derived from examining RawPodStatus and api.Pod.
type RawPodStatus struct {
// PodStatus represents the status of the pod and its containers.
// api.PodStatus can be derived from examining PodStatus and api.Pod.
type PodStatus struct {
// ID of the pod.
ID types.UID
// Name of the pod.
Expand All @@ -255,17 +255,17 @@ type RawPodStatus struct {
// IP of the pod.
IP string
// Status of containers in the pod.
ContainerStatuses []*RawContainerStatus
ContainerStatuses []*ContainerStatus
}

// RawPodContainer represents the status of a container.
type RawContainerStatus struct {
// ContainerStatus represents the status of a container.
type ContainerStatus struct {
// ID of the container.
ID ContainerID
// Name of the container.
Name string
// Status of the container.
Status ContainerStatus
State ContainerState
// Creation time of the container.
CreatedAt time.Time
// Start time of the container.
Expand All @@ -279,7 +279,7 @@ type RawContainerStatus struct {
// ID of the image.
ImageID string
// Hash of the container, used for comparison.
Hash string
Hash uint64
// Number of times that the container has been restarted.
RestartCount int
// A string explains why container is in such a status.
Expand All @@ -289,6 +289,28 @@ type RawContainerStatus struct {
Message string
}

// FindContainerStatusByName returns container status in the pod status with the given name.
// When there are multiple containers' statuses with the same name, the first match will be returned.
func (podStatus *PodStatus) FindContainerStatusByName(containerName string) *ContainerStatus {
for _, containerStatus := range podStatus.ContainerStatuses {
if containerStatus.Name == containerName {
return containerStatus
}
}
return nil
}

// Get container status of all the running containers in a pod
func (podStatus *PodStatus) GetRunningContainerStatuses() []*ContainerStatus {
runnningContainerStatues := []*ContainerStatus{}
for _, containerStatus := range podStatus.ContainerStatuses {
if containerStatus.State == ContainerStateRunning {
runnningContainerStatues = append(runnningContainerStatues, containerStatus)
}
}
return runnningContainerStatues
}

// Basic information about a container image.
type Image struct {
// ID of the image.
Expand Down
50 changes: 42 additions & 8 deletions pkg/kubelet/dockertools/convert.go
Expand Up @@ -21,23 +21,25 @@ import (
"strings"

docker "github.com/fsouza/go-dockerclient"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
)

// This file contains helper functions to convert docker API types to runtime
// (kubecontainer) types.

func mapStatus(status string) kubecontainer.ContainerStatus {
// Parse the status string in docker.APIContainers. This could break when
func mapState(state string) kubecontainer.ContainerState {
// Parse the state string in docker.APIContainers. This could break when
// we upgrade docker.
switch {
case strings.HasPrefix(status, "Up"):
return kubecontainer.ContainerStatusRunning
case strings.HasPrefix(status, "Exited"):
return kubecontainer.ContainerStatusExited
case strings.HasPrefix(state, "Up"):
return kubecontainer.ContainerStateRunning
case strings.HasPrefix(state, "Exited"):
return kubecontainer.ContainerStateExited
default:
return kubecontainer.ContainerStatusUnknown
return kubecontainer.ContainerStateUnknown
}
}

Expand All @@ -58,7 +60,11 @@ func toRuntimeContainer(c *docker.APIContainers) (*kubecontainer.Container, erro
Image: c.Image,
Hash: hash,
Created: c.Created,
Status: mapStatus(c.Status),
// (random-liu) docker uses status to indicate whether a container is running or exited.
// However, in kubernetes we usually use state to indicate whether a container is running or exited,
// while use status to indicate the comprehensive status of the container. So we have different naming
// norm here.
State: mapState(c.Status),
}, nil
}

Expand All @@ -74,3 +80,31 @@ func toRuntimeImage(image *docker.APIImages) (*kubecontainer.Image, error) {
Size: image.VirtualSize,
}, nil
}

// convert ContainerStatus to api.ContainerStatus.
func containerStatusToAPIContainerStatus(containerStatus *kubecontainer.ContainerStatus) *api.ContainerStatus {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Random-Liu Can we move this to runtime package so it can be reused by rkt as well?

For the DockerPrefix, is there any reason we can't add DockerPrefix in the kubecontainer.ContainerStatus.Id ?

cc @yujuhong

Copy link
Member Author

Choose a reason for hiding this comment

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

For the DockerPrefix, we can just use String() of containerStatus.ID, if we want to move this function to runtime package in the future.

However, for now it seems that we couldn't move it to runtime package.
Because we directly set exited container to Terminated state here, but in fact some of them should be in Waiting state. They are just failed to be restarted in last SyncPod() and should be restarted in the future. For those containers, we'll set them to Waiting with the help of reason cache in the following part of ConvertPodStatusToAPIPodStatus() (Although there seems to be a little bug in that part now #17971).

I'm not sure whether there is similar logic like this in rkt. @yifan-gu

And we also plan to move the reason cache logic up to Kubelet layer #18172 , after that I think we'll never need these kinds of convert functions in runtime interface any more. :)

/cc @yujuhong

Copy link
Contributor

Choose a reason for hiding this comment

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

@Random-Liu OK I see. I think rkt needs the similar logic, and use a reason cache to change the status from Terminted to Waiting.
BTW why there is two else if here? Can we make it a function to check the reason ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yifan-gu, the code was patched over time. I expect rewriting this would improves the readability a lot.

Copy link
Member Author

Choose a reason for hiding this comment

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

@yujuhong @yifan-gu Yeah, that part is really confusing, =..=

containerID := DockerPrefix + containerStatus.ID.ID
status := api.ContainerStatus{
Name: containerStatus.Name,
RestartCount: containerStatus.RestartCount,
Image: containerStatus.Image,
ImageID: containerStatus.ImageID,
ContainerID: containerID,
}
switch containerStatus.State {
case kubecontainer.ContainerStateRunning:
status.State.Running = &api.ContainerStateRunning{StartedAt: unversioned.NewTime(containerStatus.StartedAt)}
case kubecontainer.ContainerStateExited:
status.State.Terminated = &api.ContainerStateTerminated{
ExitCode: containerStatus.ExitCode,
Reason: containerStatus.Reason,
Message: containerStatus.Message,
StartedAt: unversioned.NewTime(containerStatus.StartedAt),
FinishedAt: unversioned.NewTime(containerStatus.FinishedAt),
ContainerID: containerID,
}
default:
status.State.Waiting = &api.ContainerStateWaiting{}
}
return &status
}