Skip to content

Commit

Permalink
Fix findOpenPorts portAllocator function (#1725)
Browse files Browse the repository at this point in the history
* Fix findOpenPorts portAllocator function

There could be the possible case when we set PortPolicy as an empty
string on update of GameServer.
  • Loading branch information
aLekSer authored Aug 6, 2020
1 parent 3f44b58 commit c28848a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/gameservers/portallocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ func (pa *PortAllocator) Allocate(gs *agonesv1.GameServer) *agonesv1.GameServer
// Also the return gives an escape from the double loop
findOpenPorts := func(amount int) []pn {
var ports []pn
if amount <= 0 {
return ports
}
for _, n := range pa.portAllocations {
for p, taken := range n {
if !taken {
Expand Down
26 changes: 26 additions & 0 deletions pkg/gameservers/portallocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,32 @@ func TestPortAllocatorAllocate(t *testing.T) {
ports = append(ports, gs.Spec.Ports[0].HostPort)
}
})

t.Run("portPolicy as an empty string", func(t *testing.T) {
m := agtesting.NewMocks()
pa := NewPortAllocator(10, 50, m.KubeInformerFactory, m.AgonesInformerFactory)
nodeWatch := watch.NewFake()
m.KubeClient.AddWatchReactor("nodes", k8stesting.DefaultWatchReactor(nodeWatch, nil))

stop, cancel := agtesting.StartInformers(m, pa.nodeSynced)
defer cancel()

// Make sure the add's don't corrupt the sync
// (no longer an issue, but leave this here for posterity)
nodeWatch.Add(&n1)
nodeWatch.Add(&n2)
assert.True(t, cache.WaitForCacheSync(stop, pa.nodeSynced))

err := pa.syncAll()
require.NoError(t, err)

// single port empty
fd := fixture.DeepCopy()
fd.Spec.Ports[0].PortPolicy = ""
gs := pa.Allocate(fd)
assert.NotNil(t, gs)
assert.Equal(t, 0, countTotalAllocatedPorts(pa))
})
}

func TestPortAllocatorMultithreadAllocate(t *testing.T) {
Expand Down

0 comments on commit c28848a

Please sign in to comment.