Add a pod status equality method to the status manager.

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.

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
 }

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 {

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.")
+		}
+	}
+}