From 1bf2f6c9c057bf0fb56bd6f70e551ff6dc72c3f4 Mon Sep 17 00:00:00 2001 From: tangwz Date: Fri, 26 May 2023 09:42:54 +0800 Subject: [PATCH] feat(NodePorts): return Skip status in PreFilter --- .../framework/plugins/nodeports/node_ports.go | 8 ++ .../plugins/nodeports/node_ports_test.go | 90 +++++++++++++------ 2 files changed, 69 insertions(+), 29 deletions(-) diff --git a/pkg/scheduler/framework/plugins/nodeports/node_ports.go b/pkg/scheduler/framework/plugins/nodeports/node_ports.go index bfd648efe4a..da5eb3aed1e 100644 --- a/pkg/scheduler/framework/plugins/nodeports/node_ports.go +++ b/pkg/scheduler/framework/plugins/nodeports/node_ports.go @@ -66,6 +66,10 @@ func getContainerPorts(pods ...*v1.Pod) []*v1.ContainerPort { for j := range pod.Spec.Containers { container := &pod.Spec.Containers[j] for k := range container.Ports { + // Only return ports with a host port specified. + if container.Ports[k].HostPort <= 0 { + continue + } ports = append(ports, &container.Ports[k]) } } @@ -76,6 +80,10 @@ func getContainerPorts(pods ...*v1.Pod) []*v1.ContainerPort { // PreFilter invoked at the prefilter extension point. func (pl *NodePorts) PreFilter(ctx context.Context, cycleState *framework.CycleState, pod *v1.Pod) (*framework.PreFilterResult, *framework.Status) { s := getContainerPorts(pod) + // Skip if a pod has no ports. + if len(s) == 0 { + return nil, framework.NewStatus(framework.Skip) + } cycleState.Write(preFilterStateKey, preFilterState(s)) return nil, nil } diff --git a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go index f148461701f..fa9c419c00b 100644 --- a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go +++ b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go @@ -24,8 +24,8 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/kubernetes/pkg/scheduler/framework" st "k8s.io/kubernetes/pkg/scheduler/testing" ) @@ -47,15 +47,17 @@ func newPod(host string, hostPortInfos ...string) *v1.Pod { func TestNodePorts(t *testing.T) { tests := []struct { - pod *v1.Pod - nodeInfo *framework.NodeInfo - name string - wantStatus *framework.Status + pod *v1.Pod + nodeInfo *framework.NodeInfo + name string + wantPreFilterStatus *framework.Status + wantFilterStatus *framework.Status }{ { - pod: &v1.Pod{}, - nodeInfo: framework.NewNodeInfo(), - name: "nothing running", + pod: &v1.Pod{}, + nodeInfo: framework.NewNodeInfo(), + name: "skip filter", + wantPreFilterStatus: framework.NewStatus(framework.Skip), }, { pod: newPod("m1", "UDP/127.0.0.1/8080"), @@ -67,15 +69,15 @@ func TestNodePorts(t *testing.T) { pod: newPod("m1", "UDP/127.0.0.1/8080"), nodeInfo: framework.NewNodeInfo( newPod("m1", "UDP/127.0.0.1/8080")), - name: "same udp port", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "same udp port", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/127.0.0.1/8080"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/127.0.0.1/8080")), - name: "same tcp port", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "same tcp port", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/127.0.0.1/8080"), @@ -93,36 +95,36 @@ func TestNodePorts(t *testing.T) { pod: newPod("m1", "UDP/127.0.0.1/8000", "UDP/127.0.0.1/8080"), nodeInfo: framework.NewNodeInfo( newPod("m1", "UDP/127.0.0.1/8080")), - name: "second udp port conflict", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "second udp port conflict", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/127.0.0.1/8001", "UDP/127.0.0.1/8080"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/127.0.0.1/8001", "UDP/127.0.0.1/8081")), - name: "first tcp port conflict", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "first tcp port conflict", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/0.0.0.0/8001"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/127.0.0.1/8001")), - name: "first tcp port conflict due to 0.0.0.0 hostIP", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "first tcp port conflict due to 0.0.0.0 hostIP", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/10.0.10.10/8001", "TCP/0.0.0.0/8001"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/127.0.0.1/8001")), - name: "TCP hostPort conflict due to 0.0.0.0 hostIP", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "TCP hostPort conflict due to 0.0.0.0 hostIP", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "TCP/127.0.0.1/8001"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/0.0.0.0/8001")), - name: "second tcp port conflict to 0.0.0.0 hostIP", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "second tcp port conflict to 0.0.0.0 hostIP", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, { pod: newPod("m1", "UDP/127.0.0.1/8001"), @@ -134,8 +136,8 @@ func TestNodePorts(t *testing.T) { pod: newPod("m1", "UDP/127.0.0.1/8001"), nodeInfo: framework.NewNodeInfo( newPod("m1", "TCP/0.0.0.0/8001", "UDP/0.0.0.0/8001")), - name: "UDP hostPort conflict due to 0.0.0.0 hostIP", - wantStatus: framework.NewStatus(framework.Unschedulable, ErrReason), + name: "UDP hostPort conflict due to 0.0.0.0 hostIP", + wantFilterStatus: framework.NewStatus(framework.Unschedulable, ErrReason), }, } @@ -144,12 +146,18 @@ func TestNodePorts(t *testing.T) { p, _ := New(nil, nil) cycleState := framework.NewCycleState() _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(context.Background(), cycleState, test.pod) + if diff := cmp.Diff(test.wantPreFilterStatus, preFilterStatus); diff != "" { + t.Errorf("preFilter status does not match (-want,+got): %s", diff) + } + if preFilterStatus.IsSkip() { + return + } if !preFilterStatus.IsSuccess() { t.Errorf("prefilter failed with status: %v", preFilterStatus) } gotStatus := p.(framework.FilterPlugin).Filter(context.Background(), cycleState, test.pod, test.nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantFilterStatus, gotStatus); diff != "" { + t.Errorf("filter status does not match (-want, +got): %s", diff) } }) } @@ -163,7 +171,7 @@ func TestPreFilterDisabled(t *testing.T) { p, _ := New(nil, nil) cycleState := framework.NewCycleState() gotStatus := p.(framework.FilterPlugin).Filter(context.Background(), cycleState, pod, nodeInfo) - wantStatus := framework.AsStatus(fmt.Errorf(`reading "PreFilterNodePorts" from cycleState: %w`, fmt.Errorf("not found"))) + wantStatus := framework.AsStatus(fmt.Errorf(`reading "PreFilterNodePorts" from cycleState: %w`, framework.ErrNotFound)) if !reflect.DeepEqual(gotStatus, wantStatus) { t.Errorf("status does not match: %v, want: %v", gotStatus, wantStatus) } @@ -179,70 +187,94 @@ func TestGetContainerPorts(t *testing.T) { pod1: st.MakePod().ContainerPort([]v1.ContainerPort{ { ContainerPort: 8001, + HostPort: 8001, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8002, + HostPort: 8002, Protocol: v1.ProtocolTCP, }}).ContainerPort([]v1.ContainerPort{ { ContainerPort: 8003, + HostPort: 8003, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8004, + HostPort: 8004, + Protocol: v1.ProtocolTCP, + }}).ContainerPort([]v1.ContainerPort{ + { + ContainerPort: 8005, Protocol: v1.ProtocolTCP, }, }).Obj(), pod2: st.MakePod().ContainerPort([]v1.ContainerPort{ { ContainerPort: 8011, + HostPort: 8011, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8012, + HostPort: 8012, Protocol: v1.ProtocolTCP, }}).ContainerPort([]v1.ContainerPort{ { ContainerPort: 8013, + HostPort: 8013, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8014, + HostPort: 8014, + Protocol: v1.ProtocolTCP, + }}).ContainerPort([]v1.ContainerPort{ + { + ContainerPort: 8015, Protocol: v1.ProtocolTCP, }, }).Obj(), expected: []*v1.ContainerPort{ { ContainerPort: 8001, + HostPort: 8001, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8002, + HostPort: 8002, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8003, + HostPort: 8003, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8004, + HostPort: 8004, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8011, + HostPort: 8011, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8012, + HostPort: 8012, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8013, + HostPort: 8013, Protocol: v1.ProtocolTCP, }, { ContainerPort: 8014, + HostPort: 8014, Protocol: v1.ProtocolTCP, }, }, @@ -252,8 +284,8 @@ func TestGetContainerPorts(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { result := getContainerPorts(test.pod1, test.pod2) - if !reflect.DeepEqual(test.expected, result) { - t.Errorf("Got different result than expected.\nDifference detected on:\n%s", diff.ObjectGoPrintSideBySide(test.expected, result)) + if diff := cmp.Diff(test.expected, result); diff != "" { + t.Errorf("container ports: container ports does not match (-want,+got): %s", diff) } }) }