Applied comments.

This commit is contained in:
Jerzy Szczepkowski 2015-02-05 20:32:51 +01:00
parent 0010e02148
commit 18c5fa1ae7
4 changed files with 20 additions and 17 deletions

View File

@ -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
}

View File

@ -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)
}
}

View File

@ -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
}

View File

@ -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
})