Merge pull request #4915 from yujuhong/hostport

kubelet: record an event with a clear reason on host port conflict

pkg/kubelet/kubelet.go

@@ -1387,15 +1387,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)

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])
+	}
+
+}

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 {