diff --git a/pkg/kubelet/container/fake_runtime.go b/pkg/kubelet/container/fake_runtime.go index a684b3ad293..bedf81f38fc 100644 --- a/pkg/kubelet/container/fake_runtime.go +++ b/pkg/kubelet/container/fake_runtime.go @@ -174,7 +174,7 @@ func (f *FakeRuntime) GetPods(all bool) ([]*Pod, error) { return f.PodList, f.Err } -func (f *FakeRuntime) SyncPod(pod *api.Pod, _ api.PodStatus, _ *PodStatus, _ []api.Secret, backOff *util.Backoff) error { +func (f *FakeRuntime) SyncPod(pod *api.Pod, _ api.PodStatus, _ *PodStatus, _ []api.Secret, backOff *util.Backoff) (result PodSyncResult) { f.Lock() defer f.Unlock() @@ -183,7 +183,11 @@ func (f *FakeRuntime) SyncPod(pod *api.Pod, _ api.PodStatus, _ *PodStatus, _ []a for _, c := range pod.Spec.Containers { f.StartedContainers = append(f.StartedContainers, c.Name) } - return f.Err + // TODO(random-liu): Add SyncResult for starting and killing containers + if f.Err != nil { + result.Fail(f.Err) + } + return } func (f *FakeRuntime) KillPod(pod *api.Pod, runningPod Pod) error { diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 036e53907fc..7240ade46e1 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -65,8 +65,9 @@ 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, apiPodStatus api.PodStatus, podStatus *PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error + SyncPod(pod *api.Pod, apiPodStatus api.PodStatus, podStatus *PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) PodSyncResult // KillPod kills all the containers of a pod. Pod may be nil, running pod must not be. + // TODO(random-liu): Return PodSyncResult in KillPod. KillPod(pod *api.Pod, runningPod Pod) error // GetAPIPodStatus retrieves the api.PodStatus of the pod, including the information of // all containers in the pod. Clients of this interface assume the diff --git a/pkg/kubelet/container/runtime_mock.go b/pkg/kubelet/container/runtime_mock.go index fd5ddad88d2..d5e23f7f230 100644 --- a/pkg/kubelet/container/runtime_mock.go +++ b/pkg/kubelet/container/runtime_mock.go @@ -57,9 +57,9 @@ func (r *Mock) GetPods(all bool) ([]*Pod, error) { return args.Get(0).([]*Pod), args.Error(1) } -func (r *Mock) SyncPod(pod *api.Pod, apiStatus api.PodStatus, status *PodStatus, secrets []api.Secret, backOff *util.Backoff) error { +func (r *Mock) SyncPod(pod *api.Pod, apiStatus api.PodStatus, status *PodStatus, secrets []api.Secret, backOff *util.Backoff) PodSyncResult { args := r.Called(pod, apiStatus, status, secrets, backOff) - return args.Error(0) + return args.Get(0).(PodSyncResult) } func (r *Mock) KillPod(pod *api.Pod, runningPod Pod) error { diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 2d1fa7d07b1..07b22afd59b 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -1814,13 +1814,7 @@ func (dm *DockerManager) clearReasonCache(pod *api.Pod, container *api.Container } // Sync the running pod to match the specified desired pod. -func (dm *DockerManager) SyncPod(pod *api.Pod, apiPodStatus api.PodStatus, podStatus *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error { - result := dm.syncPodWithSyncResult(pod, apiPodStatus, podStatus, pullSecrets, backOff) - return result.Error() -} - -// (random-liu) This is just a temporary function, will be removed when we acturally add PodSyncEvent -func (dm *DockerManager) syncPodWithSyncResult(pod *api.Pod, _ api.PodStatus, podStatus *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) (result kubecontainer.PodSyncResult) { +func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) (result kubecontainer.PodSyncResult) { start := time.Now() defer func() { metrics.ContainerManagerLatency.WithLabelValues("SyncPod").Observe(metrics.SinceInMicroseconds(start)) diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 9243d498c13..ec3f1c9a07a 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -551,7 +551,9 @@ func runSyncPod(t *testing.T, dm *DockerManager, fakeDocker *FakeDockerClient, p if backOff == nil { backOff = util.NewBackOff(time.Second, time.Minute) } - err = dm.SyncPod(pod, *apiPodStatus, podStatus, []api.Secret{}, backOff) + //TODO(random-liu): Add test for PodSyncResult + result := dm.SyncPod(pod, *apiPodStatus, podStatus, []api.Secret{}, backOff) + err = result.Error() if err != nil && !expectErr { t.Errorf("unexpected error: %v", err) } else if err == nil && expectErr { diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index d51d39cb081..3da402d4a4f 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1675,7 +1675,8 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont return err } - err = kl.containerRuntime.SyncPod(pod, apiPodStatus, podStatus, pullSecrets, kl.backOff) + result := kl.containerRuntime.SyncPod(pod, apiPodStatus, podStatus, pullSecrets, kl.backOff) + err = result.Error() if err != nil { return err } diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index b9e149ce09f..a9a85b4a9c8 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -976,7 +976,13 @@ func (r *Runtime) APIVersion() (kubecontainer.Version, error) { } // SyncPod syncs the running pod to match the specified desired pod. -func (r *Runtime) SyncPod(pod *api.Pod, podStatus api.PodStatus, internalPodStatus *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) error { +func (r *Runtime) SyncPod(pod *api.Pod, podStatus api.PodStatus, internalPodStatus *kubecontainer.PodStatus, pullSecrets []api.Secret, backOff *util.Backoff) (result kubecontainer.PodSyncResult) { + var err error + defer func() { + if err != nil { + result.Fail(err) + } + }() // TODO: (random-liu) Stop using running pod in SyncPod() // TODO: (random-liu) Rename podStatus to apiPodStatus, rename internalPodStatus to podStatus, and use new pod status as much as possible, // we may stop using apiPodStatus someday. @@ -1031,15 +1037,15 @@ func (r *Runtime) SyncPod(pod *api.Pod, podStatus api.PodStatus, internalPodStat if restartPod { // Kill the pod only if the pod is actually running. if len(runningPod.Containers) > 0 { - if err := r.KillPod(pod, runningPod); err != nil { - return err + if err = r.KillPod(pod, runningPod); err != nil { + return } } - if err := r.RunPod(pod, pullSecrets); err != nil { - return err + if err = r.RunPod(pod, pullSecrets); err != nil { + return } } - return nil + return } // GarbageCollect collects the pods/containers.