diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index a8a49c36756..c0eeaea422c 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -57,7 +57,8 @@ type Runtime interface { // KillPod kills all the containers of a pod. KillPod(pod Pod) error // GetPodStatus retrieves the status of the pod, including the information of - // all containers in the pod. + // all containers in the pod. Clients of this interface assume the containers + // statuses in a pod always have a deterministic ordering (eg: sorted by name). GetPodStatus(*api.Pod) (*api.PodStatus, error) // PullImage pulls an image from the network to local storage using the supplied // secrets if necessary. diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 372db12efad..5a5265fc7e0 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -471,8 +471,9 @@ func (dm *DockerManager) GetPodStatus(pod *api.Pod) (*api.PodStatus, error) { } podStatus.ContainerStatuses = append(podStatus.ContainerStatuses, *status) } - // TODO: Move this to a custom equals method on the pod status. A container manager - // shouldn't have to guarantee order. + // Sort the container statuses since clients of this interface expect the list + // of containers in a pod to behave like the output of `docker list`, which has a + // deterministic order. sort.Sort(kubeletTypes.SortedContainerStatuses(podStatus.ContainerStatuses)) return &podStatus, nil } diff --git a/pkg/kubelet/status_manager.go b/pkg/kubelet/status_manager.go index aaeedd0c04f..89e9081a10c 100644 --- a/pkg/kubelet/status_manager.go +++ b/pkg/kubelet/status_manager.go @@ -19,11 +19,13 @@ package kubelet import ( "fmt" "reflect" + "sort" "sync" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container" + kubeletTypes "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/types" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/golang/glog" ) @@ -51,6 +53,16 @@ func newStatusManager(kubeClient client.Interface) *statusManager { } } +// isStatusEqual returns true if the given pod statuses are equal, false otherwise. +// This method sorts container statuses so order does not affect equality. +func isStatusEqual(oldStatus, status *api.PodStatus) bool { + sort.Sort(kubeletTypes.SortedContainerStatuses(status.ContainerStatuses)) + sort.Sort(kubeletTypes.SortedContainerStatuses(oldStatus.ContainerStatuses)) + + // TODO: More sophisticated equality checking. + return reflect.DeepEqual(status, oldStatus) +} + func (s *statusManager) Start() { // syncBatch blocks when no updates are available, we can run it in a tight loop. glog.Info("Starting to sync pod status with apiserver") @@ -102,7 +114,7 @@ func (s *statusManager) SetPodStatus(pod *api.Pod, status api.PodStatus) { // Currently this routine is not called for the same pod from multiple // workers and/or the kubelet but dropping the lock before sending the // status down the channel feels like an easy way to get a bullet in foot. - if !found || !reflect.DeepEqual(oldStatus, status) { + if !found || !isStatusEqual(&oldStatus, &status) { s.podStatuses[podFullName] = status s.podStatusChannel <- podStatusSyncRequest{pod, status} } else { diff --git a/pkg/kubelet/status_manager_test.go b/pkg/kubelet/status_manager_test.go index f04a87781b2..fc473aa8032 100644 --- a/pkg/kubelet/status_manager_test.go +++ b/pkg/kubelet/status_manager_test.go @@ -17,6 +17,7 @@ limitations under the License. package kubelet import ( + "fmt" "math/rand" "strconv" "testing" @@ -159,3 +160,35 @@ func TestSyncBatch(t *testing.T) { } verifyActions(t, syncer.kubeClient, []string{"get-pod", "update-status-pod"}) } + +// shuffle returns a new shuffled list of container statuses. +func shuffle(statuses []api.ContainerStatus) []api.ContainerStatus { + numStatuses := len(statuses) + randIndexes := rand.Perm(numStatuses) + shuffled := make([]api.ContainerStatus, numStatuses) + for i := 0; i < numStatuses; i++ { + shuffled[i] = statuses[randIndexes[i]] + } + return shuffled +} + +func TestStatusEquality(t *testing.T) { + containerStatus := []api.ContainerStatus{} + for i := 0; i < 10; i++ { + s := api.ContainerStatus{ + Name: fmt.Sprintf("container%d", i), + } + containerStatus = append(containerStatus, s) + } + podStatus := api.PodStatus{ + ContainerStatuses: containerStatus, + } + for i := 0; i < 10; i++ { + oldPodStatus := api.PodStatus{ + ContainerStatuses: shuffle(podStatus.ContainerStatuses), + } + if !isStatusEqual(&oldPodStatus, &podStatus) { + t.Fatalf("Order of container statuses should not affect equality.") + } + } +}