Skip to content

Commit

Permalink
Upgrade client-go and apimachinery to 0.16.5
Browse files Browse the repository at this point in the history 
Worth noting that cache.WaitForCacheSync has changed its internal
implementation to now use `err := wait.PollImmediateUntil(...)`, so
there is no more implicit 100ms sleep before the sync. Therefore it
didn't _actually_ wait to populate from Watch or allow events to fire -
it just made room for it to occur.

So had to now using assert/require.Eventually() to make sure that the
system is in a state that matches what we expect before testing.

Work on googleforgames#1649
  • Loading branch information
@markmandel
markmandel committed Sep 5, 2020
1 parent 6d417c9 commit f8f9ae4
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 100 deletions.
31 changes: 25 additions & 6 deletions pkg/gameservers/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"net/http"
"strconv"
"testing"
"time"

"agones.dev/agones/pkg/apis/agones"
agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
Expand All @@ -37,6 +38,7 @@ import (
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -89,7 +91,11 @@ func TestControllerSyncGameServer(t *testing.T) {
assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Name)
watchPods.Add(pod)
// wait for the change to propagate
assert.True(t, cache.WaitForCacheSync(context.Background().Done(), mocks.KubeInformerFactory.Core().V1().Pods().Informer().HasSynced))
require.Eventually(t, func() bool {
list, err := c.podLister.List(labels.Everything())
assert.NoError(t, err)
return len(list) == 1
}, 5*time.Second, time.Second)
return true, pod, nil
})
mocks.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
Expand Down Expand Up @@ -1452,30 +1458,43 @@ func TestControllerGameServerPod(t *testing.T) {
}

t.Run("no pod exists", func(t *testing.T) {
c, gs, _, stop, cancel := setup()
c, gs, _, _, cancel := setup()
defer cancel()

cache.WaitForCacheSync(stop, c.gameServerSynced)
require.Never(t, func() bool {
list, err := c.podLister.List(labels.Everything())
assert.NoError(t, err)
return len(list) > 0
}, time.Second, 100*time.Millisecond)
_, err := c.gameServerPod(gs)
assert.Error(t, err)
assert.True(t, k8serrors.IsNotFound(err))
})

t.Run("a pod exists", func(t *testing.T) {
c, gs, fakeWatch, stop, cancel := setup()
c, gs, fakeWatch, _, cancel := setup()

defer cancel()
pod, err := gs.Pod()
require.NoError(t, err)

fakeWatch.Add(pod.DeepCopy())
cache.WaitForCacheSync(stop, c.gameServerSynced)
require.Eventually(t, func() bool {
list, err := c.podLister.List(labels.Everything())
assert.NoError(t, err)
return len(list) == 1
}, 5*time.Second, time.Second)

pod2, err := c.gameServerPod(gs)
require.NoError(t, err)
assert.Equal(t, pod, pod2)

fakeWatch.Delete(pod.DeepCopy())
cache.WaitForCacheSync(stop, c.gameServerSynced)
require.Eventually(t, func() bool {
list, err := c.podLister.List(labels.Everything())
assert.NoError(t, err)
return len(list) == 0
}, 5*time.Second, time.Second)
_, err = c.gameServerPod(gs)
assert.Error(t, err)
assert.True(t, k8serrors.IsNotFound(err))
Expand Down
70 changes: 34 additions & 36 deletions pkg/gameservers/pernodecounter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package gameservers

import (
"testing"
"time"

agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
agtesting "agones.dev/agones/pkg/testing"
Expand All @@ -26,7 +27,6 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/watch"
k8stesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache"
)

const (
Expand All @@ -43,8 +43,7 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) {
fakeWatch := watch.NewFake()
m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(fakeWatch, nil))

hasSynced := m.AgonesInformerFactory.Agones().V1().GameServers().Informer().HasSynced
stop, cancel := agtesting.StartInformers(m)
_, cancel := agtesting.StartInformers(m)
defer cancel()

assert.Empty(t, pnc.Counts())
Expand All @@ -57,46 +56,47 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) {
}

fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

assert.Empty(t, pnc.Counts())
require.Eventuallyf(t, func() bool {
return len(pnc.Counts()) == 0
}, 5*time.Second, time.Second, "Should be empty, instead has %v elements", len(pnc.Counts()))

gs.Status.State = agonesv1.GameServerStateReady
fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

counts := pnc.Counts()
require.Len(t, counts, 1)
var counts map[string]NodeCount
require.Eventuallyf(t, func() bool {
counts = pnc.Counts()
return len(counts) == 1
}, 5*time.Second, time.Second, "len should be 1, instead: %v", len(counts))
assert.Equal(t, int64(1), counts[name1].Ready)
assert.Equal(t, int64(0), counts[name1].Allocated)

gs.Status.State = agonesv1.GameServerStateAllocated
fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

counts = pnc.Counts()
require.Len(t, counts, 1)
assert.Equal(t, int64(0), counts[name1].Ready)
require.Eventuallyf(t, func() bool {
counts = pnc.Counts()
return len(counts) == 1 && int64(0) == counts[name1].Ready
}, 5*time.Second, time.Second, "Ready should be 0, but is instead", counts[name1].Ready)
assert.Equal(t, int64(1), counts[name1].Allocated)

gs.Status.State = agonesv1.GameServerStateShutdown
fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

counts = pnc.Counts()
require.Len(t, counts, 1)
require.Eventuallyf(t, func() bool {
counts = pnc.Counts()
return len(counts) == 1 && int64(0) == counts[name1].Allocated
}, 5*time.Second, time.Second, "Allocated should be 0, but is instead", counts[name1].Allocated)
assert.Equal(t, int64(0), counts[name1].Ready)
assert.Equal(t, int64(0), counts[name1].Allocated)

gs.ObjectMeta.Name = "gs2"
gs.Status.State = agonesv1.GameServerStateReady
gs.Status.NodeName = name2

fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

counts = pnc.Counts()
require.Len(t, counts, 2)
require.Eventuallyf(t, func() bool {
counts = pnc.Counts()
return len(counts) == 2
}, 5*time.Second, time.Second, "len should be 2, instead: %v", len(counts))
assert.Equal(t, int64(0), counts[name1].Ready)
assert.Equal(t, int64(0), counts[name1].Allocated)
assert.Equal(t, int64(1), counts[name2].Ready)
Expand All @@ -108,14 +108,13 @@ func TestPerNodeCounterGameServerEvents(t *testing.T) {
gs.Status.NodeName = name2

fakeWatch.Add(gs.DeepCopy())
cache.WaitForCacheSync(stop, hasSynced)

counts = pnc.Counts()
require.Len(t, counts, 2)
require.Eventuallyf(t, func() bool {
counts = pnc.Counts()
return len(counts) == 2 && int64(1) == counts[name2].Allocated
}, 5*time.Second, time.Second, "Allocated should be 1, but is instead", counts[name2].Allocated)
assert.Equal(t, int64(0), counts[name1].Ready)
assert.Equal(t, int64(0), counts[name1].Allocated)
assert.Equal(t, int64(1), counts[name2].Ready)
assert.Equal(t, int64(1), counts[name2].Allocated)
}

func TestPerNodeCounterNodeEvents(t *testing.T) {
Expand All @@ -128,13 +127,10 @@ func TestPerNodeCounterNodeEvents(t *testing.T) {
m.AgonesClient.AddWatchReactor("gameservers", k8stesting.DefaultWatchReactor(gsWatch, nil))
m.KubeClient.AddWatchReactor("nodes", k8stesting.DefaultWatchReactor(nodeWatch, nil))

gsSynced := m.AgonesInformerFactory.Agones().V1().GameServers().Informer().HasSynced
nodeSynced := m.KubeInformerFactory.Core().V1().Nodes().Informer().HasSynced

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

assert.Empty(t, pnc.Counts())
require.Empty(t, pnc.Counts())

gs := &agonesv1.GameServer{
ObjectMeta: metav1.ObjectMeta{Name: "gs1", Namespace: defaultNs},
Expand All @@ -144,12 +140,14 @@ func TestPerNodeCounterNodeEvents(t *testing.T) {

gsWatch.Add(gs.DeepCopy())
nodeWatch.Add(node.DeepCopy())
cache.WaitForCacheSync(stop, gsSynced, nodeSynced)
assert.Len(t, pnc.Counts(), 1)
require.Eventuallyf(t, func() bool {
return len(pnc.Counts()) == 1
}, 5*time.Second, time.Second, "Should be 1 element, not %v", len(pnc.Counts()))

nodeWatch.Delete(node.DeepCopy())
cache.WaitForCacheSync(stop, nodeSynced)
assert.Empty(t, pnc.Counts())
require.Eventually(t, func() bool {
return len(pnc.Counts()) == 0
}, 5*time.Second, time.Second, "pnc.Counts() should be empty, but is instead has %v element", len(pnc.Counts()))
}

func TestPerNodeCounterRun(t *testing.T) {
Expand Down
23 changes: 18 additions & 5 deletions pkg/gameservers/portallocator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strconv"
"sync"
"testing"
"time"

agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
agtesting "agones.dev/agones/pkg/testing"
Expand All @@ -27,6 +28,7 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/watch"
Expand Down Expand Up @@ -445,14 +447,17 @@ func TestPortAllocatorSyncDeleteGameServer(t *testing.T) {
return true, nl, nil
})

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

gsWatch.Add(gs1.DeepCopy())
gsWatch.Add(gs2.DeepCopy())
gsWatch.Add(gs3.DeepCopy())

assert.True(t, cache.WaitForCacheSync(stop, pa.gameServerSynced))
require.Eventually(t, func() bool {
list, err := pa.gameServerLister.GameServers(gs1.ObjectMeta.Namespace).List(labels.Everything())
assert.NoError(t, err)
return len(list) == 3
}, 5*time.Second, time.Second)

err := pa.syncAll()
require.NoError(t, err)
Expand All @@ -465,7 +470,11 @@ func TestPortAllocatorSyncDeleteGameServer(t *testing.T) {

// delete allocated gs
gsWatch.Delete(gs3.DeepCopy())
assert.True(t, cache.WaitForCacheSync(stop, pa.gameServerSynced))
require.Eventually(t, func() bool {
list, err := pa.gameServerLister.GameServers(gs1.ObjectMeta.Namespace).List(labels.Everything())
assert.NoError(t, err)
return len(list) == 2
}, 5*time.Second, time.Second)

pa.mutex.RLock() // reading mutable state, so read lock
assert.Equal(t, 1, countAllocatedPorts(pa, 10))
Expand All @@ -475,7 +484,11 @@ func TestPortAllocatorSyncDeleteGameServer(t *testing.T) {
// delete the currently non allocated server, all should be the same
// simulated getting an old delete message
gsWatch.Delete(gs4.DeepCopy())
assert.True(t, cache.WaitForCacheSync(stop, pa.gameServerSynced))
require.Never(t, func() bool {
list, err := pa.gameServerLister.GameServers(gs1.ObjectMeta.Namespace).List(labels.Everything())
assert.NoError(t, err)
return len(list) != 2
}, time.Second, 100*time.Millisecond)
pa.mutex.RLock() // reading mutable state, so read lock
assert.Equal(t, 1, countAllocatedPorts(pa, 10))
assert.Equal(t, 1, countAllocatedPorts(pa, 11))
Expand Down
Loading

0 comments on commit f8f9ae4

Please sign in to comment.