Refactor HostIP predicate algorithm

- Remove string decode logic. It's not really helping to find the
  conflict ports, and it's expensive to do encoding/decoding
- Not to parse the container ports information in predicate meta, use
  straight []*v1.ContainerPort
- Use better data structure to search port conflict based on ip
  addresses
- Collect scattered source code into common place
This commit is contained in:
Yongkun Anfernee Gui
2017-11-16 15:43:06 -08:00
parent 8a9954d471
commit 68c2c79362
11 changed files with 398 additions and 370 deletions

View File

@@ -57,7 +57,6 @@ go_test(
"//pkg/scheduler/algorithm:go_default_library",
"//pkg/scheduler/schedulercache:go_default_library",
"//pkg/scheduler/testing:go_default_library",
"//pkg/scheduler/util:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/api/storage/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",

View File

@@ -46,7 +46,7 @@ type predicateMetadata struct {
pod *v1.Pod
podBestEffort bool
podRequest *schedulercache.Resource
podPorts map[string]bool
podPorts []*v1.ContainerPort
//key is a pod full name with the anti-affinity rules.
matchingAntiAffinityTerms map[string][]matchingPodAntiAffinityTerm
serviceAffinityInUse bool
@@ -90,7 +90,7 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf
pod: pod,
podBestEffort: isPodBestEffort(pod),
podRequest: GetResourceRequest(pod),
podPorts: schedutil.GetUsedPorts(pod),
podPorts: schedutil.GetContainerPorts(pod),
matchingAntiAffinityTerms: matchingTerms,
}
for predicateName, precomputeFunc := range predicateMetadataProducers {
@@ -172,10 +172,7 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata {
podRequest: meta.podRequest,
serviceAffinityInUse: meta.serviceAffinityInUse,
}
newPredMeta.podPorts = map[string]bool{}
for k, v := range meta.podPorts {
newPredMeta.podPorts[k] = v
}
newPredMeta.podPorts = append([]*v1.ContainerPort(nil), meta.podPorts...)
newPredMeta.matchingAntiAffinityTerms = map[string][]matchingPodAntiAffinityTerm{}
for k, v := range meta.matchingAntiAffinityTerms {
newPredMeta.matchingAntiAffinityTerms[k] = append([]matchingPodAntiAffinityTerm(nil), v...)

View File

@@ -373,7 +373,15 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
Memory: 300,
AllowedPodNumber: 4,
},
podPorts: map[string]bool{"1234": true, "456": false},
podPorts: []*v1.ContainerPort{
{
Name: "name",
HostPort: 10,
ContainerPort: 20,
Protocol: "TCP",
HostIP: "1.2.3.4",
},
},
matchingAntiAffinityTerms: map[string][]matchingPodAntiAffinityTerm{
"term1": {
{

View File

@@ -966,12 +966,12 @@ func (s *ServiceAffinity) checkServiceAffinity(pod *v1.Pod, meta algorithm.Predi
// PodFitsHostPorts checks if a node has free ports for the requested pod ports.
func PodFitsHostPorts(pod *v1.Pod, meta algorithm.PredicateMetadata, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
var wantPorts map[string]bool
var wantPorts []*v1.ContainerPort
if predicateMeta, ok := meta.(*predicateMetadata); ok {
wantPorts = predicateMeta.podPorts
} else {
// We couldn't parse metadata - fallback to computing it.
wantPorts = schedutil.GetUsedPorts(pod)
wantPorts = schedutil.GetContainerPorts(pod)
}
if len(wantPorts) == 0 {
return true, nil, nil

View File

@@ -20,6 +20,7 @@ import (
"os"
"reflect"
"strconv"
"strings"
"testing"
"k8s.io/api/core/v1"
@@ -32,7 +33,6 @@ import (
"k8s.io/kubernetes/pkg/scheduler/algorithm"
"k8s.io/kubernetes/pkg/scheduler/schedulercache"
schedulertesting "k8s.io/kubernetes/pkg/scheduler/testing"
schedutil "k8s.io/kubernetes/pkg/scheduler/util"
)
var (
@@ -518,13 +518,13 @@ func TestPodFitsHost(t *testing.T) {
func newPod(host string, hostPortInfos ...string) *v1.Pod {
networkPorts := []v1.ContainerPort{}
for _, portInfo := range hostPortInfos {
hostPortInfo := decode(portInfo)
hostPort, _ := strconv.Atoi(hostPortInfo.hostPort)
splited := strings.Split(portInfo, "/")
hostPort, _ := strconv.Atoi(splited[2])
networkPorts = append(networkPorts, v1.ContainerPort{
HostIP: hostPortInfo.hostIP,
HostIP: splited[1],
HostPort: int32(hostPort),
Protocol: v1.Protocol(hostPortInfo.protocol),
Protocol: v1.Protocol(splited[0]),
})
}
return &v1.Pod{
@@ -653,41 +653,6 @@ func TestPodFitsHostPorts(t *testing.T) {
}
}
func TestGetUsedPorts(t *testing.T) {
tests := []struct {
pods []*v1.Pod
ports map[string]bool
}{
{
[]*v1.Pod{
newPod("m1", "UDP/127.0.0.1/9090"),
},
map[string]bool{"UDP/127.0.0.1/9090": true},
},
{
[]*v1.Pod{
newPod("m1", "UDP/127.0.0.1/9090"),
newPod("m1", "UDP/127.0.0.1/9091"),
},
map[string]bool{"UDP/127.0.0.1/9090": true, "UDP/127.0.0.1/9091": true},
},
{
[]*v1.Pod{
newPod("m1", "TCP/0.0.0.0/9090"),
newPod("m2", "UDP/127.0.0.1/9091"),
},
map[string]bool{"TCP/0.0.0.0/9090": true, "UDP/127.0.0.1/9091": true},
},
}
for _, test := range tests {
ports := schedutil.GetUsedPorts(test.pods...)
if !reflect.DeepEqual(test.ports, ports) {
t.Errorf("%s: expected %v, got %v", "test get used ports", test.ports, ports)
}
}
}
func TestGCEDiskConflicts(t *testing.T) {
volState := v1.PodSpec{
Volumes: []v1.Volume{

View File

@@ -17,10 +17,7 @@ limitations under the License.
package predicates
import (
"strings"
"github.com/golang/glog"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
@@ -135,65 +132,11 @@ type EquivalencePod struct {
PVCSet sets.String
}
type hostPortInfo struct {
protocol string
hostIP string
hostPort string
}
// decode decodes string ("protocol/hostIP/hostPort") to *hostPortInfo object.
func decode(info string) *hostPortInfo {
hostPortInfoSlice := strings.Split(info, "/")
protocol := hostPortInfoSlice[0]
hostIP := hostPortInfoSlice[1]
hostPort := hostPortInfoSlice[2]
return &hostPortInfo{
protocol: protocol,
hostIP: hostIP,
hostPort: hostPort,
}
}
// specialPortConflictCheck detects whether specailHostPort(whose hostIP is 0.0.0.0) is conflict with otherHostPorts.
// return true if we have a conflict.
func specialPortConflictCheck(specialHostPort string, otherHostPorts map[string]bool) bool {
specialHostPortInfo := decode(specialHostPort)
if specialHostPortInfo.hostIP == schedutil.DefaultBindAllHostIP {
// loop through all the otherHostPorts to see if there exists a conflict
for hostPortItem := range otherHostPorts {
hostPortInfo := decode(hostPortItem)
// if there exists one hostPortItem which has the same hostPort and protocol with the specialHostPort, that will cause a conflict
if specialHostPortInfo.hostPort == hostPortInfo.hostPort && specialHostPortInfo.protocol == hostPortInfo.protocol {
return true
}
}
}
return false
}
// portsConflict check whether existingPorts and wantPorts conflict with each other
// return true if we have a conflict
func portsConflict(existingPorts, wantPorts map[string]bool) bool {
for existingPort := range existingPorts {
if specialPortConflictCheck(existingPort, wantPorts) {
return true
}
}
for wantPort := range wantPorts {
if specialPortConflictCheck(wantPort, existingPorts) {
return true
}
// general check hostPort conflict procedure for hostIP is not 0.0.0.0
if existingPorts[wantPort] {
func portsConflict(existingPorts schedutil.HostPortInfo, wantPorts []*v1.ContainerPort) bool {
for _, cp := range wantPorts {
if existingPorts.CheckConflict(cp.HostIP, string(cp.Protocol), cp.HostPort) {
return true
}
}

View File

@@ -18,8 +18,6 @@ package predicates
import (
"fmt"
"reflect"
"testing"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -70,194 +68,3 @@ func ExampleFindLabelsInSet() {
// label1=value1,label2=value2,label3=will_see_this
// pod1,pod2,
}
func Test_decode(t *testing.T) {
tests := []struct {
name string
args string
want *hostPortInfo
}{
{
name: "test1",
args: "UDP/127.0.0.1/80",
want: &hostPortInfo{
protocol: "UDP",
hostIP: "127.0.0.1",
hostPort: "80",
},
},
{
name: "test2",
args: "TCP/127.0.0.1/80",
want: &hostPortInfo{
protocol: "TCP",
hostIP: "127.0.0.1",
hostPort: "80",
},
},
{
name: "test3",
args: "TCP/0.0.0.0/80",
want: &hostPortInfo{
protocol: "TCP",
hostIP: "0.0.0.0",
hostPort: "80",
},
},
}
for _, tt := range tests {
if got := decode(tt.args); !reflect.DeepEqual(got, tt.want) {
t.Errorf("test name = %v, decode() = %v, want %v", tt.name, got, tt.want)
}
}
}
func Test_specialPortConflictCheck(t *testing.T) {
type args struct {
specialHostPort string
otherHostPorts map[string]bool
}
tests := []struct {
name string
args args
want bool
}{
{
name: "test-1",
args: args{
specialHostPort: "TCP/0.0.0.0/80",
otherHostPorts: map[string]bool{
"TCP/127.0.0.2/8080": true,
"TCP/127.0.0.1/80": true,
"UDP/127.0.0.2/8080": true,
},
},
want: true,
},
{
name: "test-2",
args: args{
specialHostPort: "TCP/0.0.0.0/80",
otherHostPorts: map[string]bool{
"TCP/127.0.0.2/8080": true,
"UDP/127.0.0.1/80": true,
"UDP/127.0.0.2/8080": true,
},
},
want: false,
},
{
name: "test-3",
args: args{
specialHostPort: "TCP/0.0.0.0/80",
otherHostPorts: map[string]bool{
"TCP/127.0.0.2/8080": true,
"TCP/127.0.0.1/8090": true,
"UDP/127.0.0.2/8080": true,
},
},
want: false,
},
{
name: "test-4",
args: args{
specialHostPort: "TCP/0.0.0.0/80",
otherHostPorts: map[string]bool{
"UDP/127.0.0.2/8080": true,
"UDP/127.0.0.1/8090": true,
"TCP/127.0.0.2/8080": true,
},
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := specialPortConflictCheck(tt.args.specialHostPort, tt.args.otherHostPorts); got != tt.want {
t.Errorf("specialPortConflictCheck() = %v, want %v", got, tt.want)
}
})
}
}
func Test_portsConflict(t *testing.T) {
type args struct {
existingPorts map[string]bool
wantPorts map[string]bool
}
tests := []struct {
name string
args args
want bool
}{
{
name: "test1",
args: args{
existingPorts: map[string]bool{
"UDP/127.0.0.1/8080": true,
},
wantPorts: map[string]bool{
"UDP/127.0.0.1/8080": true,
},
},
want: true,
},
{
name: "test2",
args: args{
existingPorts: map[string]bool{
"UDP/127.0.0.2/8080": true,
},
wantPorts: map[string]bool{
"UDP/127.0.0.1/8080": true,
},
},
want: false,
},
{
name: "test3",
args: args{
existingPorts: map[string]bool{
"TCP/127.0.0.1/8080": true,
},
wantPorts: map[string]bool{
"UDP/127.0.0.1/8080": true,
},
},
want: false,
},
{
name: "test4",
args: args{
existingPorts: map[string]bool{
"TCP/0.0.0.0/8080": true,
},
wantPorts: map[string]bool{
"TCP/127.0.0.1/8080": true,
},
},
want: true,
},
{
name: "test5",
args: args{
existingPorts: map[string]bool{
"TCP/127.0.0.1/8080": true,
},
wantPorts: map[string]bool{
"TCP/0.0.0.0/8080": true,
},
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := portsConflict(tt.args.existingPorts, tt.args.wantPorts); got != tt.want {
t.Errorf("portsConflict() = %v, want %v", got, tt.want)
}
})
}
}