Upgrade client-go and apimachinery to 0.16.15 (#1794)

* Update k8s.io/client-go to v0.16.15

* Upgrade k8s.io/apiextensions-apiserver to v0.16.15

Note: overwrote grpc to keep at current version.

* Upgrade client-go and apimachinery to 0.16.5

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 #1649

Co-authored-by: Alexander Apalikov <alexander.apalikov@globant.com>

go.mod

@@ -3,50 +3,42 @@ module agones.dev/agones
 go 1.13
 
 require (
-	cloud.google.com/go v0.34.0
+	cloud.google.com/go v0.38.0
 	contrib.go.opencensus.io/exporter/prometheus v0.1.0
 	contrib.go.opencensus.io/exporter/stackdriver v0.8.0
 	fortio.org/fortio v1.3.1
 	github.com/ahmetb/gen-crd-api-reference-docs v0.1.1
 	github.com/aws/aws-sdk-go v1.16.20 // indirect
-	github.com/evanphx/json-patch v4.5.0+incompatible // indirect
 	github.com/fsnotify/fsnotify v1.4.7
-	github.com/go-openapi/spec v0.19.0
-	github.com/gogo/protobuf v1.2.1 // indirect
+	github.com/go-openapi/spec v0.19.2
 	github.com/golang/protobuf v1.3.2
-	github.com/google/gofuzz v1.0.0 // indirect
-	github.com/google/uuid v1.1.0 // indirect
 	github.com/googleapis/gnostic v0.1.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway v1.11.3
 	github.com/hashicorp/golang-lru v0.5.1
 	github.com/heptiolabs/healthcheck v0.0.0-20171201210846-da5fdee475fb
 	github.com/joonix/log v0.0.0-20180502111528-d2d3f2f4a806
-	github.com/json-iterator/go v1.1.5 // indirect
 	github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a
 	github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d
-	github.com/onsi/ginkgo v1.8.0 // indirect
-	github.com/onsi/gomega v1.5.0 // indirect
 	github.com/pkg/errors v0.8.1
 	github.com/prometheus/client_golang v0.9.2
-	github.com/sirupsen/logrus v1.2.0
+	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/pflag v1.0.5
-	github.com/spf13/viper v1.3.1
+	github.com/spf13/viper v1.3.2
 	github.com/stretchr/testify v1.5.0
 	go.opencensus.io v0.22.3
-	golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc
-	golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2
-	golang.org/x/tools v0.0.0-20190328211700-ab21143f2384
-	google.golang.org/api v0.0.0-20190117000611-43037ff31f69 // indirect
+	golang.org/x/net v0.0.0-20191004110552-13f9640d40b9
+	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
+	golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72
 	google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03
-	google.golang.org/grpc v1.20.1
+	google.golang.org/grpc v1.23.0
 	gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0 // indirect
 	gopkg.in/fsnotify.v1 v1.4.7
-	gopkg.in/inf.v0 v0.9.1 // indirect
-	gopkg.in/natefinch/lumberjack.v2 v2.0.0-20170531160350-a96e63847dc3
-	k8s.io/api v0.15.11
-	k8s.io/apiextensions-apiserver v0.0.0-20200318010201-8546efc3bc75 // kubernetes-1.15.11
-	k8s.io/apimachinery v0.15.11
-	k8s.io/client-go v0.15.11
-	k8s.io/kube-openapi v0.0.0-20190709113604-33be087ad058 // indirect
+	gopkg.in/natefinch/lumberjack.v2 v2.0.0
+	k8s.io/api v0.16.15
+	k8s.io/apiextensions-apiserver v0.16.15
+	k8s.io/apimachinery v0.16.15
+	k8s.io/client-go v0.16.15
 	k8s.io/utils v0.0.0-20200124190032-861946025e34 // indirect
 )
+
+replace google.golang.org/grpc v1.23.0 => google.golang.org/grpc v1.20.1 // apiserver updated grpc, but we aren't using that, so it's fine.

pkg/gameservers/controller_test.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"strconv"
 	"testing"
+	"time"
 
 	"agones.dev/agones/pkg/apis/agones"
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
@@ -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"
@@ -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) {
@@ -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))

pkg/gameservers/pernodecounter_test.go

@@ -16,6 +16,7 @@ package gameservers
 
 import (
 	"testing"
+	"time"
 
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	agtesting "agones.dev/agones/pkg/testing"
@@ -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 (
@@ -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())
@@ -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)
@@ -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) {
@@ -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},
@@ -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) {

pkg/gameservers/portallocator_test.go

@@ -19,6 +19,7 @@ import (
 	"strconv"
 	"sync"
 	"testing"
+	"time"
 
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	agtesting "agones.dev/agones/pkg/testing"
@@ -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"
@@ -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)
@@ -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))
@@ -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))