scheduler: tolerate having only one dynamic port available (#17619)

If the dynamic port range for a node is set so that the min is equal to the max,
there's only one port available and this passes config validation. But the
scheduler panics when it tries to pick a random port. Only add the randomness
when there's more than one to pick from.

Adds a test for the behavior but also adjusts the commentary on a couple of the
existing tests that made it seem like this case was already covered if you
didn't look too closely.

Fixes: #17585
This commit is contained in:
Tim Gross 2023-06-20 13:29:25 -04:00 committed by GitHub
parent e58ba84a9e
commit ff9ba8ff73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 4 deletions

3
.changelog/17619.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
scheduler: Fixed a panic when a node has only one configured dynamic port
```

View File

@ -749,7 +749,11 @@ func getDynamicPortsStochastic(nodeUsed Bitmap, portsInOffer []int, minDynamicPo
return nil, fmt.Errorf("stochastic dynamic port selection failed") return nil, fmt.Errorf("stochastic dynamic port selection failed")
} }
randPort := minDynamicPort + rand.Intn(maxDynamicPort-minDynamicPort) randPort := minDynamicPort
if maxDynamicPort-minDynamicPort > 0 {
randPort = randPort + rand.Intn(maxDynamicPort-minDynamicPort)
}
if nodeUsed != nil && nodeUsed.Check(uint(randPort)) { if nodeUsed != nil && nodeUsed.Check(uint(randPort)) {
goto PICK goto PICK
} }

View File

@ -362,7 +362,7 @@ func TestNetworkIndex_yieldIP(t *testing.T) {
func TestNetworkIndex_AssignPorts(t *testing.T) { func TestNetworkIndex_AssignPorts(t *testing.T) {
ci.Parallel(t) ci.Parallel(t)
// Create a node that only has one free port // Create a node that only two free dynamic ports
idx := NewNetworkIndex() idx := NewNetworkIndex()
n := &Node{ n := &Node{
NodeResources: &NodeResources{ NodeResources: &NodeResources{
@ -424,6 +424,84 @@ func TestNetworkIndex_AssignPorts(t *testing.T) {
must.Between(t, idx.MaxDynamicPort-1, adminPortMapping.Value, idx.MaxDynamicPort) must.Between(t, idx.MaxDynamicPort-1, adminPortMapping.Value, idx.MaxDynamicPort)
} }
// TestNetworkIndex_AssignPorts_SmallRange exercises assigning ports on group
// networks with small dynamic port ranges configured
func TestNetworkIndex_AssignPortss_SmallRange(t *testing.T) {
ci.Parallel(t)
n := &Node{
NodeResources: &NodeResources{
NodeNetworks: []*NodeNetworkResource{
{
Mode: "host",
Device: "eth0",
Speed: 1000,
Addresses: []NodeNetworkAddress{
{
Alias: "default",
Address: "192.168.0.100",
Family: NodeNetworkAF_IPv4,
},
},
},
},
},
}
testCases := []struct {
name string
min int
max int
ask []Port
expectErr string
}{
{
name: "1 dynamic port avail and 1 port requested",
min: 20000,
max: 20000,
ask: []Port{{"http", 0, 80, "default"}},
expectErr: "",
},
{
name: "1 dynamic port avail and 2 ports requested",
min: 20000,
max: 20000,
ask: []Port{{"http", 0, 80, "default"}, {"admin", 0, 80, "default"}},
expectErr: "dynamic port selection failed",
},
{
name: "2 dynamic ports avail and 2 ports requested",
min: 20000,
max: 20001,
ask: []Port{{"http", 0, 80, "default"}, {"admin", 0, 80, "default"}},
expectErr: "",
},
}
for _, tc := range testCases {
idx := NewNetworkIndex()
idx.MinDynamicPort = tc.min
idx.MaxDynamicPort = tc.max
idx.SetNode(n)
ask := &NetworkResource{DynamicPorts: tc.ask}
offer, err := idx.AssignPorts(ask)
if tc.expectErr != "" {
must.EqError(t, err, tc.expectErr)
} else {
must.NoError(t, err)
must.NotNil(t, offer, must.Sprint("did not get an offer"))
for _, port := range tc.ask {
_, ok := offer.Get(port.Label)
must.True(t, ok)
}
}
}
}
func TestNetworkIndex_AssignTaskNetwork(t *testing.T) { func TestNetworkIndex_AssignTaskNetwork(t *testing.T) {
ci.Parallel(t) ci.Parallel(t)
idx := NewNetworkIndex() idx := NewNetworkIndex()
@ -531,7 +609,7 @@ func TestNetworkIndex_AssignTaskNetwork(t *testing.T) {
func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention(t *testing.T) { func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention(t *testing.T) {
ci.Parallel(t) ci.Parallel(t)
// Create a node that only has one free port // Create a node that only has two free dynamic ports
idx := NewNetworkIndex() idx := NewNetworkIndex()
n := &Node{ n := &Node{
NodeResources: &NodeResources{ NodeResources: &NodeResources{
@ -546,6 +624,7 @@ func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention(t *testing.T) {
}, },
ReservedResources: &NodeReservedResources{ ReservedResources: &NodeReservedResources{
Networks: NodeReservedNetworkResources{ Networks: NodeReservedNetworkResources{
// leave only 2 available ports
ReservedHostPorts: fmt.Sprintf("%d-%d", idx.MinDynamicPort, idx.MaxDynamicPort-2), ReservedHostPorts: fmt.Sprintf("%d-%d", idx.MinDynamicPort, idx.MaxDynamicPort-2),
}, },
}, },
@ -561,7 +640,7 @@ func TestNetworkIndex_AssignTaskNetwork_Dynamic_Contention(t *testing.T) {
must.NoError(t, err) must.NoError(t, err)
must.NotNil(t, offer, must.Sprint("did not get an offer")) must.NotNil(t, offer, must.Sprint("did not get an offer"))
must.Eq(t, "192.168.0.100", offer.IP) must.Eq(t, "192.168.0.100", offer.IP)
must.Len(t, 2, offer.DynamicPorts, must.Sprint("There should be one dynamic ports")) must.Len(t, 2, offer.DynamicPorts, must.Sprint("There should be two dynamic ports"))
must.NotEq(t, offer.DynamicPorts[0].Value, offer.DynamicPorts[1].Value, must.NotEq(t, offer.DynamicPorts[0].Value, offer.DynamicPorts[1].Value,
must.Sprint("assigned dynamic ports must not conflict")) must.Sprint("assigned dynamic ports must not conflict"))