Upgrade client-go and apimachinery to 0.16.15 #1794
Conversation
Note: overwrote grpc to keep at current version.
|
Build Failed 😱 Build Id: 68a4c348-6b8f-40f8-ad6a-eaadb9d85c1d To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
markmandel
force-pushed
the
upgrade/client-go-1.13
branch
from
f8f9ae4
to
66ce69b
Compare
|
sdk-conformance failure 😢 |
|
Build Succeeded 👏 Build Id: 058f6e17-de56-42a8-93b4-aa9b924dc536 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
| 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. |
There was a problem hiding this comment.
We should update at some point; want to file a bug to follow up on this?
There was a problem hiding this comment.
Agreed - makes sense. We should take this as a motivator to keep grpc up to date 👍
| @@ -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() | |||
There was a problem hiding this comment.
It looks like stop is never used from setup() now so it can just be removed as a return var.
There was a problem hiding this comment.
Just found where we do use this (and shows an example of what you mentioned below)
https://github.com/markmandel/agones/blob/66ce69bf64c1e58b0e38fd642556e282ee70d060/pkg/gameservers/controller_test.go#L1504-L1512
SYAC
c, gs, fakeWatch, stop, cancel := setup()
defer cancel()
pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: gs.ObjectMeta.Name, Labels: map[string]string{agonesv1.GameServerPodLabel: gs.ObjectMeta.Name, "owned": "false"}}}
fakeWatch.Add(pod.DeepCopy())
// gate
cache.WaitForCacheSync(stop, c.podSynced)
pkg/metrics/controller_test.go
Outdated
| require.NoError(t, err) | ||
| return len(list) == expected | ||
| }, 5*time.Second, time.Second) | ||
| // write a good message if something goes wrong |
There was a problem hiding this comment.
Not sure I can follow this comment. Could you please remove or explain more what it is for?
There was a problem hiding this comment.
Updated comment, PTAL!
There was a problem hiding this comment.
Now it is much more verbose, I like it.
pkg/metrics/controller_test.go
Outdated
| c.sync() | ||
| reader := metricexport.NewReader() | ||
|
|
||
| // wait until we have a some nodes and gameservers |
There was a problem hiding this comment.
| // wait until we have a some nodes and gameservers | |
| // wait until we have some nodes and gameservers |
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
markmandel
force-pushed
the
upgrade/client-go-1.13
branch
from
66ce69b
to
f7daa1b
Compare
|
Build Failed 😱 Build Id: e2f2d946-e3fc-4a3b-8fb6-fd1b994a8ab7 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
SDK conformance failure |
|
Build Succeeded 👏 Build Id: 887117ce-35a1-4b89-9dad-e3659d414263 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aLekSer, markmandel, roberthbailey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Build Succeeded 👏 Build Id: 22780755-ff66-4489-9751-b573231aa432 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
aLekSer
changed the title
* 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 googleforgames#1649 Co-authored-by: Alexander Apalikov <alexander.apalikov@globant.com>
What type of PR is this?
/kind breaking
What this PR does / Why we need it:
Upgrading the supporting libraries to be inline with the updated Kubernetes version to 1.16.x.
Which issue(s) this PR fixes:
Work on #1649
Special notes for your reviewer:
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.