Remove HostPort conflict checking.

We decided to get rid of boundPods.   Removing this check is
a prerequisite for that.  This check had some value before we had
IP-per-Pod.  However, AIUI, use of HostPort is strongly discouraged
in Kubernetes.  It still exists as part of a Pod spec because
of ContainerVM, where it is used.  But, this change does not affect
ContainerVM, where there is no master.

If someone did create pods with HostPort using kubernetes, the following
would happen:
  - The scheduler would try not to put two conflicting pods on the same
    machine (pkg/scheduler/predicates.go : PodFitsPorts() )
  - I'm not sure if it is currently possible for a race to occur where
    the PodFitsPorts check were bypassed.   Maybe it could happen.
  - If the kubelet was sent conflicting pods, it would detect them in
    ( pkg/kubelet/kubelet.go :  filterHostPortConflicts() ).  It would
    arbitrarily pick one pod to run and another to ignore.
  - If all of the above happened and the user filed and issue on github,
    we might figure out that the user used HostPort and tell the user to stop.

TODO:
 - e2e test
 - Several of the demos in examples/ use hostPort.  Change them to
   not specify hostPort and have a service instead.
pull/6/head
Eric Tune 2015-02-19 13:27:41 -08:00
parent 6d465c4d86
commit 5e8d70f76b
5 changed files with 0 additions and 203 deletions

View File

@ -1,33 +0,0 @@
/*
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
}

View File

@ -1,99 +0,0 @@
/*
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)
}
}
}

View File

@ -1,23 +0,0 @@
/*
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

View File

@ -1,43 +0,0 @@
/*
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
}

View File

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