From 241df2d3be21467799e997fb5600cc408fe85ac7 Mon Sep 17 00:00:00 2001
From: Yu-Ju Hong <yjhong@google.com>
Date: Fri, 27 Feb 2015 13:43:21 -0800
Subject: [PATCH] kubelet: record an event with a clear reason on host port
 conflict

Currently, kubelet silently ignores pods that caused host port conflict. This
commit surfaces the error by recording an event.

It also makes sure that kubelet iterates through the pods in the order of the
creation timestamp, which ensures that pods created later are ignored on
conflict.
---
 pkg/kubelet/kubelet.go      | 20 ++++++++++++++++++++
 pkg/kubelet/kubelet_test.go | 35 +++++++++++++++++++++++++++++++++++
 pkg/util/time.go            |  5 +++++
 3 files changed, 60 insertions(+)

diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
index b98a65aac4f9a..ce371aa74555b 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -1378,15 +1378,35 @@ func updateBoundPods(changed []api.BoundPod, current []api.BoundPod) []api.Bound
 	return updated
 }
 
+type podsByCreationTime []api.BoundPod
+
+func (s podsByCreationTime) Len() int {
+	return len(s)
+}
+
+func (s podsByCreationTime) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+
+func (s podsByCreationTime) Less(i, j int) bool {
+	return s[i].CreationTimestamp.Before(s[j].CreationTimestamp)
+}
+
 // filterHostPortConflicts removes pods that conflict on Port.HostPort values
 func filterHostPortConflicts(pods []api.BoundPod) []api.BoundPod {
 	filtered := []api.BoundPod{}
 	ports := map[int]bool{}
 	extract := func(p *api.Port) int { return p.HostPort }
+
+	// Respect the pod creation order when resolving conflicts.
+	sort.Sort(podsByCreationTime(pods))
+
 	for i := range pods {
 		pod := &pods[i]
 		if errs := validation.AccumulateUniquePorts(pod.Spec.Containers, ports, extract); len(errs) != 0 {
 			glog.Warningf("Pod %q: HostPort is already allocated, ignoring: %v", GetPodFullName(pod), errs)
+			record.Eventf(pod, "hostPortConflict", "Cannot start the pod due to host port conflict.")
+			// TODO: Set the pod status to fail.
 			continue
 		}
 		filtered = append(filtered, *pod)
diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go
index 805e45196ab7d..db5e09f21bf8a 100644
--- a/pkg/kubelet/kubelet_test.go
+++ b/pkg/kubelet/kubelet_test.go
@@ -3086,3 +3086,38 @@ func TestPortForward(t *testing.T) {
 		t.Fatalf("stream: expected %v, got %v", e, a)
 	}
 }
+
+// Tests that upon host port conflict, the newer pod is removed.
+func TestFilterHostPortConflicts(t *testing.T) {
+	// Reuse the pod spec with the same port to create a conflict.
+	spec := api.PodSpec{Containers: []api.Container{{Ports: []api.Port{{HostPort: 80}}}}}
+	var pods = []api.BoundPod{
+		{
+			ObjectMeta: api.ObjectMeta{
+				UID:       "123456789",
+				Name:      "newpod",
+				Namespace: "foo",
+			},
+			Spec: spec,
+		},
+		{
+			ObjectMeta: api.ObjectMeta{
+				UID:       "987654321",
+				Name:      "oldpod",
+				Namespace: "foo",
+			},
+			Spec: spec,
+		},
+	}
+	// Make sure the BoundPods are in the reverse order of creation time.
+	pods[1].CreationTimestamp = util.NewTime(time.Now())
+	pods[0].CreationTimestamp = util.NewTime(time.Now().Add(1 * time.Second))
+	filteredPods := filterHostPortConflicts(pods)
+	if len(filteredPods) != 1 {
+		t.Fatalf("Expected one pod. Got pods %#v", filteredPods)
+	}
+	if filteredPods[0].Name != "oldpod" {
+		t.Fatalf("Expected pod %#v. Got pod %#v", pods[1], filteredPods[0])
+	}
+
+}
diff --git a/pkg/util/time.go b/pkg/util/time.go
index 470e9781c7fcb..574de7cccc316 100644
--- a/pkg/util/time.go
+++ b/pkg/util/time.go
@@ -44,6 +44,11 @@ func Now() Time {
 	return Time{time.Now()}
 }
 
+// Before reports whether the time instant t is before u.
+func (t Time) Before(u Time) bool {
+	return t.Time.Before(u.Time)
+}
+
 // Unix returns the local time corresponding to the given Unix time
 // by wrapping time.Unix.
 func Unix(sec int64, nsec int64) Time {