diff --git a/pkg/constraint/constraint.go b/pkg/constraint/constraint.go index 8acc8c073d..430c1a85cb 100644 --- a/pkg/constraint/constraint.go +++ b/pkg/constraint/constraint.go @@ -17,11 +17,17 @@ limitations under the License. package constraint import ( + "fmt" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" ) // 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) bool { - return !PortsConflict(pods) +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 errors } diff --git a/pkg/constraint/constraint_test.go b/pkg/constraint/constraint_test.go index 1677d935df..eae4522127 100644 --- a/pkg/constraint/constraint_test.go +++ b/pkg/constraint/constraint_test.go @@ -17,6 +17,7 @@ limitations under the License. package constraint import ( + "fmt" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -40,11 +41,11 @@ func podWithContainers(containers ...api.Container) api.BoundPod { func TestAllowed(t *testing.T) { table := []struct { - allowed bool - pods []api.BoundPod + err string + pods []api.BoundPod }{ { - allowed: true, + err: "[]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(1, 2, 3), @@ -57,7 +58,7 @@ func TestAllowed(t *testing.T) { }, }, { - allowed: true, + err: "[]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(0, 0), @@ -70,7 +71,7 @@ func TestAllowed(t *testing.T) { }, }, { - allowed: false, + err: "[host port 3 is already in use]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(3, 3), @@ -78,7 +79,7 @@ func TestAllowed(t *testing.T) { }, }, { - allowed: false, + err: "[host port 6 is already in use]", pods: []api.BoundPod{ podWithContainers( containerWithHostPorts(6), @@ -91,7 +92,7 @@ func TestAllowed(t *testing.T) { } for _, item := range table { - if e, a := item.allowed, Allowed(item.pods); e != a { + 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 f5c95f5fb2..b7398a6ea7 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 d2bd576cb5..c5114a8320 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 !constraint.Allowed(boundPodList.Items) { - return nil, fmt.Errorf("the assignment would cause a constraint violation") + 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 })