diff --git a/pkg/constraint/constraint.go b/pkg/constraint/constraint.go index 078ca6d59d132..430c1a85cb2de 100644 --- a/pkg/constraint/constraint.go +++ b/pkg/constraint/constraint.go @@ -24,9 +24,10 @@ import ( // Allowed returns true if pods is a collection of bound pods // which can run without conflict on a single minion. -func Allowed(pods []api.BoundPod) error { - if PortsConflict(pods) { - return fmt.Errorf("conflicting ports") +func Allowed(pods []api.BoundPod) []error { + errors := []error{} + for _, port := range hostPortsConflict(pods) { + errors = append(errors, fmt.Errorf("host port %v is already in use", port)) } - return nil + return errors } diff --git a/pkg/constraint/constraint_test.go b/pkg/constraint/constraint_test.go index bb6f84fc89da6..eae452212701c 100644 --- a/pkg/constraint/constraint_test.go +++ b/pkg/constraint/constraint_test.go @@ -41,11 +41,11 @@ func podWithContainers(containers ...api.Container) api.BoundPod { func TestAllowed(t *testing.T) { table := []struct { - err error + err string pods []api.BoundPod }{ { - err: nil, + err: "[]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(1, 2, 3), @@ -58,7 +58,7 @@ func TestAllowed(t *testing.T) { }, }, { - err: nil, + err: "[]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(0, 0), @@ -71,7 +71,7 @@ func TestAllowed(t *testing.T) { }, }, { - err: fmt.Errorf("conflicting ports"), + err: "[host port 3 is already in use]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(3, 3), @@ -79,7 +79,7 @@ func TestAllowed(t *testing.T) { }, }, { - err: fmt.Errorf("conflicting ports"), + err: "[host port 6 is already in use]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(6), @@ -92,7 +92,7 @@ func TestAllowed(t *testing.T) { } for _, item := range table { - if e, a := item.err, Allowed(item.pods); e != a && e.Error() != a.Error() { + if e, a := item.err, Allowed(item.pods); e != fmt.Sprintf("%v", a) { t.Errorf("Expected %v, got %v: \n%v\v", e, a, item.pods) } } diff --git a/pkg/constraint/ports.go b/pkg/constraint/ports.go index f5c95f5fb2d42..b7398a6ea78dc 100644 --- a/pkg/constraint/ports.go +++ b/pkg/constraint/ports.go @@ -20,10 +20,12 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) -// PortsConflict returns true iff two containers attempt to expose -// the same host port. -func PortsConflict(pods []api.BoundPod) bool { +// hostPortsConflict returns an array of host ports that at least two +// containers attempt to expose. The array is empty if no such port +// exists. +func hostPortsConflict(pods []api.BoundPod) []int { hostPorts := map[int]struct{}{} + conflictingPorts := []int{} for _, pod := range pods { for _, container := range pod.Spec.Containers { for _, port := range container.Ports { @@ -31,11 +33,11 @@ func PortsConflict(pods []api.BoundPod) bool { continue } if _, exists := hostPorts[port.HostPort]; exists { - return true + conflictingPorts = append(conflictingPorts, port.HostPort) } hostPorts[port.HostPort] = struct{}{} } } } - return false + return conflictingPorts } diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index 49c32eff69e7f..c5114a8320b0f 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -214,8 +214,8 @@ func (r *Registry) assignPod(ctx api.Context, podID string, machine string) erro err = r.AtomicUpdate(contKey, &api.BoundPods{}, func(in runtime.Object) (runtime.Object, error) { boundPodList := in.(*api.BoundPods) boundPodList.Items = append(boundPodList.Items, *boundPod) - if e := constraint.Allowed(boundPodList.Items); e != nil { - return nil, fmt.Errorf("the assignment would cause the following constraint violation: %v", e) + if errors := constraint.Allowed(boundPodList.Items); len(errors) > 0 { + return nil, fmt.Errorf("the assignment would cause the following constraints violation: %v", errors) } return boundPodList, nil })