Merge pull request #4619 from erictune/no_port_conflict

pkg/constraint/constraint.go

-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-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) []error {
-	errors := []error{}
-	for _, port := range hostPortsConflict(pods) {
-		errors = append(errors, fmt.Errorf("host port %v is already in use", port))
-	}
-	return errors
-}

pkg/constraint/constraint_test.go

-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package constraint
-
-import (
-	"fmt"
-	"testing"
-
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-)
-
-func containerWithHostPorts(ports ...int) api.Container {
-	c := api.Container{}
-	for _, p := range ports {
-		c.Ports = append(c.Ports, api.ContainerPort{HostPort: p})
-	}
-	return c
-}
-
-func podWithContainers(containers ...api.Container) api.BoundPod {
-	m := api.BoundPod{}
-	for _, c := range containers {
-		m.Spec.Containers = append(m.Spec.Containers, c)
-	}
-	return m
-}
-
-func TestAllowed(t *testing.T) {
-	table := []struct {
-		err  string
-		pods []api.BoundPod
-	}{
-		{
-			err: "[]",
-			pods: []api.BoundPod{
-				podWithContainers(
-					containerWithHostPorts(1, 2, 3),
-					containerWithHostPorts(4, 5, 6),
-				),
-				podWithContainers(
-					containerWithHostPorts(7, 8, 9),
-					containerWithHostPorts(10, 11, 12),
-				),
-			},
-		},
-		{
-			err: "[]",
-			pods: []api.BoundPod{
-				podWithContainers(
-					containerWithHostPorts(0, 0),
-					containerWithHostPorts(0, 0),
-				),
-				podWithContainers(
-					containerWithHostPorts(0, 0),
-					containerWithHostPorts(0, 0),
-				),
-			},
-		},
-		{
-			err: "[host port 3 is already in use]",
-			pods: []api.BoundPod{
-				podWithContainers(
-					containerWithHostPorts(3, 3),
-				),
-			},
-		},
-		{
-			err: "[host port 6 is already in use]",
-			pods: []api.BoundPod{
-				podWithContainers(
-					containerWithHostPorts(6),
-				),
-				podWithContainers(
-					containerWithHostPorts(6),
-				),
-			},
-		},
-	}
-
-	for _, item := range table {
-		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)
-		}
-	}
-}

pkg/constraint/doc.go

-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-// Package constraint has functions for ensuring that collections of
-// containers are allowed to run together on a single host.
-//
-// TODO: Add resource math. Phrase this code in a way that makes it easy
-// to call from schedulers as well as from the feasiblity check that
-// apiserver performs.
-package constraint

pkg/constraint/ports.go

-/*
-Copyright 2014 Google Inc. All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package constraint
-
-import (
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-)
-
-// 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 {
-				if port.HostPort == 0 {
-					continue
-				}
-				if _, exists := hostPorts[port.HostPort]; exists {
-					conflictingPorts = append(conflictingPorts, port.HostPort)
-				}
-				hostPorts[port.HostPort] = struct{}{}
-			}
-		}
-	}
-	return conflictingPorts
-}

pkg/registry/pod/etcd/etcd.go

@@ -23,7 +23,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	etcderr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/constraint"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic"
@@ -193,14 +192,10 @@ func (r *BindingREST) assignPod(ctx api.Context, podID string, machine string) e
 	if err != nil {
 		return err
 	}
-	// Doing the constraint check this way provides atomicity guarantees.
 	contKey := makeBoundPodsKey(machine)
 	err = r.store.Helper.AtomicUpdate(contKey, &api.BoundPods{}, true, func(in runtime.Object) (runtime.Object, error) {
 		boundPodList := in.(*api.BoundPods)
 		boundPodList.Items = append(boundPodList.Items, *boundPod)
-		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
 	})
 	if err != nil {