diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index acac9d7cb9..3f21f08b4a 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "fmt" + "slices" "strconv" "sync" "time" @@ -189,10 +190,18 @@ func NewController( _, _ = pods.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ UpdateFunc: func(oldObj, newObj interface{}) { oldPod := oldObj.(*corev1.Pod) + enqueue := false if isGameServerPod(oldPod) { newPod := newObj.(*corev1.Pod) // node name has changed -- i.e. it has been scheduled if oldPod.Spec.NodeName != newPod.Spec.NodeName { + enqueue = true + } else if !slices.Equal(oldPod.Status.PodIPs, newPod.Status.PodIPs) { + // PodIPs have changed, so it's time to go populate the GameServer with the new PodIPs. + enqueue = true + } + + if enqueue { owner := metav1.GetControllerOf(newPod) c.workerqueue.Enqueue(cache.ExplicitKey(newPod.ObjectMeta.Namespace + "/" + owner.Name)) } @@ -491,6 +500,9 @@ func (c *Controller) syncGameServer(ctx context.Context, key string) error { if gs, err = c.syncGameServerRequestReadyState(ctx, gs); err != nil { return err } + if gs, err = c.syncGameServerPodIPs(ctx, gs); err != nil { + return err + } if gs, err = c.syncDevelopmentGameServer(ctx, gs); err != nil { return err } @@ -924,11 +936,6 @@ func (c *Controller) syncGameServerStartingState(ctx context.Context, gs *agones return gs, workerqueue.NewTraceError(errors.Errorf("node not yet populated for Pod %s", pod.ObjectMeta.Name)) } - // Ensure the pod IPs are populated - if len(pod.Status.PodIPs) == 0 { - return gs, workerqueue.NewTraceError(errors.Errorf("pod IPs not yet populated for Pod %s", pod.ObjectMeta.Name)) - } - node, err := c.nodeLister.Get(pod.Spec.NodeName) if err != nil { return gs, errors.Wrapf(err, "error retrieving node %s for Pod %s", pod.Spec.NodeName, pod.ObjectMeta.Name) @@ -938,6 +945,8 @@ func (c *Controller) syncGameServerStartingState(ctx context.Context, gs *agones if err != nil { return gs, err } + // append pod ip addresses if there are any + gsCopy, _ = applyGameServerPodIPs(gsCopy, pod) gsCopy.Status.State = agonesv1.GameServerStateScheduled gs, err = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) @@ -991,6 +1000,9 @@ func (c *Controller) syncGameServerRequestReadyState(ctx context.Context, gs *ag } } + // append pod ip addresses if there are any + gsCopy, _ = applyGameServerPodIPs(gsCopy, pod) + gsCopy, err = c.applyGameServerReadyContainerIDAnnotation(ctx, gsCopy, pod) if err != nil { return gs, err @@ -1009,6 +1021,44 @@ func (c *Controller) syncGameServerRequestReadyState(ctx context.Context, gs *ag return gs, nil } +// syncGameServerPodIPs handles stable-state GameServers (Scheduled, Ready, Reserved, Allocated) where +// PodIPs may arrive after the initial state transition. It updates the GameServer with any new PodIPs +// from the pod that are not already in gs.Status.Addresses. +func (c *Controller) syncGameServerPodIPs(ctx context.Context, gs *agonesv1.GameServer) (*agonesv1.GameServer, error) { + switch gs.Status.State { + case agonesv1.GameServerStateScheduled, + agonesv1.GameServerStateReady, + agonesv1.GameServerStateReserved, + agonesv1.GameServerStateAllocated: + default: + return gs, nil + } + if !gs.ObjectMeta.DeletionTimestamp.IsZero() { + return gs, nil + } + if _, isDev := gs.GetDevAddress(); isDev { + return gs, nil + } + + pod, err := c.gameServerPod(gs) + if err != nil { + if k8serrors.IsNotFound(err) { + return gs, nil + } + return gs, err + } + + gsCopy := gs.DeepCopy() + gsCopy, updated := applyGameServerPodIPs(gsCopy, pod) + if !updated { + return gs, nil + } + + loggerForGameServer(gs, c.baseLogger).Debug("Updating GameServer with new PodIPs") + gs, err = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) + return gs, errors.Wrapf(err, "error updating GameServer %s with new PodIPs", gsCopy.ObjectMeta.Name) +} + // applyGameServerReadyContainerIDAnnotation updates the GameServer and its corresponding Pod with an annotation // indicating the ID of the container that is running the game server, once it's in a Running state. func (c *Controller) applyGameServerReadyContainerIDAnnotation(ctx context.Context, gsCopy *agonesv1.GameServer, pod *corev1.Pod) (*agonesv1.GameServer, error) { diff --git a/pkg/gameservers/controller_test.go b/pkg/gameservers/controller_test.go index 164a296f40..661c3479e7 100644 --- a/pkg/gameservers/controller_test.go +++ b/pkg/gameservers/controller_test.go @@ -1289,13 +1289,16 @@ func TestControllerSyncGameServerStartingState(t *testing.T) { assert.NotEmpty(t, gs.Status.Ports) }) - t.Run("Error on podIPs not populated", func(t *testing.T) { + t.Run("Successful transition to Scheduled without PodIPs", func(t *testing.T) { c, m := newFakeController() gsFixture := newFixture() gsFixture.ApplyDefaults() pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) require.NoError(t, err) pod.Spec.NodeName = nodeFixtureName + // no PodIPs set on the pod + + gsUpdated := false m.KubeClient.AddReactor("list", "nodes", func(_ k8stesting.Action) (bool, runtime.Object, error) { return true, &corev1.NodeList{Items: []corev1.Node{node}}, nil @@ -1304,16 +1307,23 @@ func TestControllerSyncGameServerStartingState(t *testing.T) { return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil }) m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsUpdated = true ua := action.(k8stesting.UpdateAction) gs := ua.GetObject().(*agonesv1.GameServer) assert.Equal(t, agonesv1.GameServerStateScheduled, gs.Status.State) - return true, gs, errors.New("update-err") + return true, gs, nil }) ctx, cancel := agtesting.StartInformers(m, c.gameServerSynced, c.podSynced, c.nodeSynced) defer cancel() - _, err = c.syncGameServerStartingState(ctx, gsFixture) - assert.Error(t, err) + gs, err := c.syncGameServerStartingState(ctx, gsFixture) + require.NoError(t, err) + assert.True(t, gsUpdated) + assert.Equal(t, agonesv1.GameServerStateScheduled, gs.Status.State) + // No PodIP addresses should be present + for _, addr := range gs.Status.Addresses { + assert.NotEqual(t, agonesv1.NodePodIP, addr.Type, "unexpected PodIP address in status") + } }) t.Run("Error on update", func(t *testing.T) { @@ -1359,6 +1369,201 @@ func TestControllerSyncGameServerStartingState(t *testing.T) { }) } +func TestControllerSyncGameServerPodIPs(t *testing.T) { + t.Parallel() + + podIPFixture := "10.0.0.1" + + newGS := func(state agonesv1.GameServerState) *agonesv1.GameServer { + gs := &agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: newSingleContainerSpec(), + Status: agonesv1.GameServerStatus{State: state}, + } + gs.ApplyDefaults() + return gs + } + + type expected struct { + updated bool + test func(t *testing.T, original *agonesv1.GameServer, result *agonesv1.GameServer) + } + type gameserver struct { + state agonesv1.GameServerState + addresses []corev1.NodeAddress + deleted bool + } + type pod struct { + podIPs []corev1.PodIP + } + fixtures := map[string]struct { + gameserver gameserver + expected expected + pod pod + }{ + "Scheduled state: pod has PodIPs not in GS, GS is updated": { + gameserver: gameserver{ + state: agonesv1.GameServerStateScheduled, + addresses: []corev1.NodeAddress{}, + }, + pod: pod{ + podIPs: []corev1.PodIP{{IP: podIPFixture}}, + }, + expected: expected{ + updated: true, + test: func(t *testing.T, _ *agonesv1.GameServer, result *agonesv1.GameServer) { + var found bool + for _, addr := range result.Status.Addresses { + if addr.Type == agonesv1.NodePodIP && addr.Address == podIPFixture { + found = true + } + } + require.True(t, found, "PodIP should be in addresses") + }, + }, + }, + "Scheduled state: PodIPs already in GS, no update": { + gameserver: gameserver{ + state: agonesv1.GameServerStateScheduled, + addresses: []corev1.NodeAddress{{Type: agonesv1.NodePodIP, Address: podIPFixture}}, + }, + pod: pod{ + podIPs: []corev1.PodIP{{IP: podIPFixture}}, + }, + expected: expected{ + updated: false, + test: func(t *testing.T, original *agonesv1.GameServer, result *agonesv1.GameServer) { + assert.Equal(t, original, result) + }, + }, + }, + "Ready state: pod has PodIPs not in GS, GS is updated": { + gameserver: gameserver{ + addresses: []corev1.NodeAddress{}, + state: agonesv1.GameServerStateReady, + }, + pod: pod{ + podIPs: []corev1.PodIP{{IP: podIPFixture}}, + }, + expected: expected{ + updated: true, + test: func(t *testing.T, _ *agonesv1.GameServer, result *agonesv1.GameServer) { + var found bool + for _, addr := range result.Status.Addresses { + if addr.Type == agonesv1.NodePodIP && addr.Address == podIPFixture { + found = true + } + } + require.True(t, found, "PodIP should be in addresses") + }, + }, + }, + "Starting is a no-op": { + gameserver: gameserver{ + state: agonesv1.GameServerStateStarting, + addresses: []corev1.NodeAddress{}, + }, + pod: pod{ + podIPs: []corev1.PodIP{{IP: podIPFixture}}, + }, + expected: expected{ + updated: false, + test: func(t *testing.T, original *agonesv1.GameServer, result *agonesv1.GameServer) { + assert.Equal(t, original, result) + }, + }, + }, + "DeletionTimestamp set is a no-op": { + gameserver: gameserver{ + state: agonesv1.GameServerStateScheduled, + addresses: []corev1.NodeAddress{}, + deleted: true, + }, + pod: pod{ + podIPs: []corev1.PodIP{{IP: podIPFixture}}, + }, + expected: expected{ + updated: false, + test: func(t *testing.T, original *agonesv1.GameServer, result *agonesv1.GameServer) { + assert.Equal(t, original, result) + }, + }, + }, + "Pod has no PodIPs is a no-op": { + gameserver: gameserver{ + state: agonesv1.GameServerStateScheduled, + addresses: []corev1.NodeAddress{}, + }, + pod: pod{ + podIPs: []corev1.PodIP{}, + }, + expected: expected{ + updated: false, + test: func(t *testing.T, original *agonesv1.GameServer, result *agonesv1.GameServer) { + assert.Equal(t, original, result) + }, + }, + }, + } + + for k, v := range fixtures { + t.Run(k, func(t *testing.T) { + c, m := newFakeController() + gs := newGS(v.gameserver.state) + gs.Status.Addresses = v.gameserver.addresses + if v.gameserver.deleted { + now := metav1.Now() + gs.ObjectMeta.DeletionTimestamp = &now + } + pod, err := gs.Pod(agtesting.FakeAPIHooks{}) + require.NoError(t, err) + pod.Status.PodIPs = v.pod.podIPs + gsUpdated := false + + m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil + }) + m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsUpdated = true + ua := action.(k8stesting.UpdateAction) + updated := ua.GetObject().(*agonesv1.GameServer) + return true, updated, nil + }) + + ctx, cancel := agtesting.StartInformers(m, c.gameServerSynced, c.podSynced) + defer cancel() + + result, err := c.syncGameServerPodIPs(ctx, gs) + require.NoError(t, err) + require.Equal(t, v.expected.updated, gsUpdated, "GameServer update operation did not occur as expected") + v.expected.test(t, gs, result) + }) + } + + // this one is too hard to convert to a table test, so it can live by itself. + t.Run("Update error is returned", func(t *testing.T) { + c, m := newFakeController() + gs := newGS(agonesv1.GameServerStateScheduled) + pod, err := gs.Pod(agtesting.FakeAPIHooks{}) + require.NoError(t, err) + pod.Status.PodIPs = []corev1.PodIP{{IP: podIPFixture}} + + m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil + }) + m.AgonesClient.AddReactor("update", "gameservers", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, gs, errors.New("update-err") + }) + + ctx, cancel := agtesting.StartInformers(m, c.gameServerSynced, c.podSynced) + defer cancel() + + _, err = c.syncGameServerPodIPs(ctx, gs) + require.Error(t, err) + assert.ErrorContains(t, err, "error updating GameServer test with new PodIPs") + }) +} + func TestControllerCreateGameServerPod(t *testing.T) { t.Parallel() @@ -1548,313 +1753,299 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { agruntime.FeatureTestMutex.Lock() defer agruntime.FeatureTestMutex.Unlock() - t.Run("GameServer with ReadyRequest State", func(t *testing.T) { - c, m := newFakeController() + runningStatus := func(containerName string) []corev1.ContainerStatus { + return []corev1.ContainerStatus{{ + Name: containerName, + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + ContainerID: containerID, + }} + } - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - require.NoError(t, err) - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, + fixtures := map[string]struct { + // GS setup + gsNodeName string + gsAnnotations map[string]string + // Pod setup + podIPs []corev1.PodIP + podAnnotations map[string]string + makeContainerStatuses func(string) []corev1.ContainerStatus + setupNode bool + // Feature flag override ("" = no change) + featureFlags string + // Reactor errors + gsUpdateErr error + podUpdateErr error + // Reactor assertions + checkGSUpdate func(*testing.T, *agonesv1.GameServer) + checkPodUpdate func(*testing.T, *corev1.Pod) + // Post-call assertions + check func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) + wantEvents []string + }{ + "ReadyRequest with no PodIP": { + gsNodeName: nodeName, + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + if !agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + } }, - } - - gsUpdated := false - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - - // only set if not using sidecars. - if !agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.NoError(t, err) + assert.True(t, gsUpdated, "GameServer wasn't updated") + if agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.False(t, podUpdated, "Pod was updated") + } else { + assert.True(t, podUpdated, "Pod was not updated") + } + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.Empty(t, gs.Status.Addresses) + }, + wantEvents: []string{"SDK.Ready() complete"}, + }, + "ReadyRequest with PodIP": { + gsNodeName: nodeName, + podIPs: []corev1.PodIP{{IP: ipFixture}}, + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.Equal(t, []corev1.NodeAddress{{Type: agonesv1.NodePodIP, Address: ipFixture}}, gs.Status.Addresses) + if !agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + } + }, + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.NoError(t, err) + assert.True(t, gsUpdated, "GameServer wasn't updated") + if agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.False(t, podUpdated, "Pod was updated") + } else { + assert.True(t, podUpdated, "Pod was not updated") + } + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.Equal(t, []corev1.NodeAddress{{Type: agonesv1.NodePodIP, Address: ipFixture}}, gs.Status.Addresses) + }, + wantEvents: []string{"SDK.Ready() complete"}, + }, + "Error on GameServer update": { + gsNodeName: nodeName, + featureFlags: string(agruntime.FeatureSidecarContainers) + "=false", + gsUpdateErr: errors.New("update-err"), + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - } - return true, gs, nil - }) - m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - ua := action.(k8stesting.UpdateAction) - pod := ua.GetObject().(*corev1.Pod) - assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, pod, nil - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - gs, err := c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.NoError(t, err, "should not error") - assert.True(t, gsUpdated, "GameServer wasn't updated") - if agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { - assert.False(t, podUpdated, "Pod was updated") - } else { - assert.True(t, podUpdated, "Pod was not updated") - } - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SDK.Ready() complete") - }) - - t.Run("Error on GameServer update", func(t *testing.T) { - require.NoError(t, agruntime.ParseFeatures(string(agruntime.FeatureSidecarContainers)+"=false")) - - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - require.NoError(t, err) - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, }, - } - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, gs, errors.New("update-err") - }) - m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - ua := action.(k8stesting.UpdateAction) - pod := ua.GetObject().(*corev1.Pod) - assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, pod, nil - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - _, err = c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.True(t, podUpdated, "pod was not updated") - require.EqualError(t, err, "error setting Ready, Port and address on GameServer test Status: update-err") - }) - - t.Run("Error on pod update", func(t *testing.T) { - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - require.NoError(t, err) - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) }, - } - gsUpdated := false - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(_ k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - return true, nil, nil - }) - m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - ua := action.(k8stesting.UpdateAction) - pod := ua.GetObject().(*corev1.Pod) - assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, pod, errors.New("pod-error") - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - _, err = c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.True(t, podUpdated, "pod was not updated") - assert.False(t, gsUpdated, "GameServer was updated") - require.EqualError(t, err, "error updating ready annotation on Pod: test: pod-error") - }) - - t.Run("Pod annotation already set", func(t *testing.T) { - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - require.NoError(t, err) - pod.ObjectMeta.Annotations = map[string]string{agonesv1.GameServerReadyContainerIDAnnotation: containerID} - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, + check: func(t *testing.T, _ *agonesv1.GameServer, err error, _, podUpdated bool) { + assert.True(t, podUpdated, "pod was not updated") + require.EqualError(t, err, "error setting Ready, Port and address on GameServer test Status: update-err") }, - } - gsUpdated := false - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, gs, nil - }) - m.KubeClient.AddReactor("update", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - return true, nil, nil - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - gs, err := c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.NoError(t, err, "should not error") - assert.True(t, gsUpdated, "GameServer wasn't updated") - assert.False(t, podUpdated, "Pod was updated") - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SDK.Ready() complete") - - }) - - t.Run("GameServer without an Address, but RequestReady State", func(t *testing.T) { - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - assert.Nil(t, err) - pod.Spec.NodeName = nodeFixtureName - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, + }, + "Error on pod update": { + gsNodeName: nodeName, + featureFlags: string(agruntime.FeatureSidecarContainers) + "=false", + podUpdateErr: errors.New("pod-error"), + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) }, - } - gsUpdated := false - podUpdated := false - - ipFixture := "12.12.12.12" - nodeFixture := corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeFixtureName}, Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{Address: ipFixture, Type: corev1.NodeExternalIP}}}} - - m.KubeClient.AddReactor("list", "nodes", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.NodeList{Items: []corev1.Node{nodeFixture}}, nil - }) - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, gs, nil - }) - m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - ua := action.(k8stesting.UpdateAction) - pod := ua.GetObject().(*corev1.Pod) - assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, pod, nil - }) + check: func(t *testing.T, _ *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + assert.True(t, podUpdated, "pod was not updated") + assert.False(t, gsUpdated, "GameServer was updated") + require.EqualError(t, err, "error updating ready annotation on Pod: test: pod-error") + }, + }, + "Pod annotation already set": { + gsNodeName: nodeName, + featureFlags: string(agruntime.FeatureSidecarContainers) + "=false", + podAnnotations: map[string]string{agonesv1.GameServerReadyContainerIDAnnotation: containerID}, + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.NoError(t, err) + assert.True(t, gsUpdated, "GameServer wasn't updated") + assert.False(t, podUpdated, "Pod was updated") + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + }, + wantEvents: []string{"SDK.Ready() complete"}, + }, + "GameServer without an Address but in RequestReady State": { + setupNode: true, + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + if !agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.Equal(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + } + }, + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.Equal(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.NoError(t, err) + assert.True(t, gsUpdated, "GameServer wasn't updated") + if agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.False(t, podUpdated, "Pod was updated") + } else { + assert.True(t, podUpdated, "Pod wasn't updated") + } + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.Equal(t, nodeFixtureName, gs.Status.NodeName) + assert.Equal(t, ipFixture, gs.Status.Address) + assert.Equal(t, []corev1.NodeAddress{{Address: ipFixture, Type: "ExternalIP"}}, gs.Status.Addresses) + }, + wantEvents: []string{"Address and port populated", "SDK.Ready() complete"}, + }, + "GameServer with ReadyContainerIDAnnotation already set": { + gsNodeName: nodeName, + gsAnnotations: map[string]string{agonesv1.GameServerReadyContainerIDAnnotation: "4321"}, + checkGSUpdate: func(t *testing.T, gs *agonesv1.GameServer) { + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + assert.NotEqual(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + checkPodUpdate: func(t *testing.T, pod *corev1.Pod) { + assert.NotEqual(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + }, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.NoError(t, err) + assert.True(t, gsUpdated, "GameServer wasn't updated") + if agruntime.FeatureEnabled(agruntime.FeatureSidecarContainers) { + assert.False(t, podUpdated, "Pod was updated") + } else { + assert.True(t, podUpdated, "Pod wasn't updated") + } + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + }, + wantEvents: []string{"SDK.Ready() complete"}, + }, + "Pod not in running state": { + gsNodeName: nodeName, + featureFlags: string(agruntime.FeatureSidecarContainers) + "=false", + makeContainerStatuses: func(containerName string) []corev1.ContainerStatus { + return []corev1.ContainerStatus{{Name: containerName}} + }, + check: func(t *testing.T, _ *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.EqualError(t, err, "game server container for GameServer test in namespace default is not currently running, try again") + assert.False(t, gsUpdated, "GameServer was updated") + assert.False(t, podUpdated, "Pod was updated") + }, + }, + "Pod missing ContainerStatuses": { + gsNodeName: nodeName, + featureFlags: string(agruntime.FeatureSidecarContainers) + "=false", + makeContainerStatuses: func(_ string) []corev1.ContainerStatus { + return nil + }, + check: func(t *testing.T, _ *agonesv1.GameServer, err error, gsUpdated, podUpdated bool) { + require.EqualError(t, err, "game server container for GameServer test in namespace default not present in pod status, try again") + assert.False(t, gsUpdated, "GameServer was updated") + assert.False(t, podUpdated, "Pod was updated") + }, + }, + "PodIPs are folded into the Ready state update": { + gsNodeName: nodeName, + podIPs: []corev1.PodIP{{IP: ipFixture}}, + check: func(t *testing.T, gs *agonesv1.GameServer, err error, _, _ bool) { + require.NoError(t, err) + assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + var found bool + for _, addr := range gs.Status.Addresses { + if addr.Type == agonesv1.NodePodIP && addr.Address == ipFixture { + found = true + } + } + assert.True(t, found, "PodIP should be folded into Ready update") + }, + }, + } - ctx, cancel := agtesting.StartInformers(m, c.podSynced, c.nodeSynced) - defer cancel() + for name, fixture := range fixtures { + t.Run(name, func(t *testing.T) { + if fixture.featureFlags != "" { + require.NoError(t, agruntime.ParseFeatures(fixture.featureFlags)) + } - gs, err := c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.Nil(t, err, "should not error") - assert.True(t, gsUpdated, "GameServer wasn't updated") - assert.True(t, podUpdated, "Pod wasn't updated") - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) + c, m := newFakeController() - assert.Equal(t, gs.Status.NodeName, nodeFixture.ObjectMeta.Name) - assert.Equal(t, gs.Status.Address, ipFixture) - assert.Equal(t, []corev1.NodeAddress{{Address: ipFixture, Type: "ExternalIP"}}, gs.Status.Addresses) + gsFixture := &agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: newSingleContainerSpec(), + Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}, + } + gsFixture.ApplyDefaults() + gsFixture.Status.NodeName = fixture.gsNodeName + for k, v := range fixture.gsAnnotations { + gsFixture.Annotations[k] = v + } - agtesting.AssertEventContains(t, m.FakeRecorder.Events, "Address and port populated") - agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SDK.Ready() complete") - }) + pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) + require.NoError(t, err) + makeStatuses := fixture.makeContainerStatuses + if makeStatuses == nil { + makeStatuses = runningStatus + } + pod.Status.ContainerStatuses = makeStatuses(gsFixture.Spec.Container) + pod.Status.PodIPs = fixture.podIPs + if fixture.podAnnotations != nil { + pod.ObjectMeta.Annotations = fixture.podAnnotations + } + if fixture.setupNode { + pod.Spec.NodeName = nodeFixtureName + node := corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: nodeFixtureName}, + Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{Address: ipFixture, Type: corev1.NodeExternalIP}}}, + } + m.KubeClient.AddReactor("list", "nodes", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, &corev1.NodeList{Items: []corev1.Node{node}}, nil + }) + } - t.Run("GameServer with a GameServerReadyContainerIDAnnotation already", func(t *testing.T) { - c, m := newFakeController() + gsUpdated := false + podUpdated := false - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - gsFixture.Annotations[agonesv1.GameServerReadyContainerIDAnnotation] = "4321" - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - pod.Status.ContainerStatuses = []corev1.ContainerStatus{ - { - Name: gsFixture.Spec.Container, - State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, - ContainerID: containerID, - }, - } - assert.Nil(t, err) - gsUpdated := false - podUpdated := false + m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil + }) + m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gsUpdated = true + ua := action.(k8stesting.UpdateAction) + gs := ua.GetObject().(*agonesv1.GameServer) + if fixture.checkGSUpdate != nil { + fixture.checkGSUpdate(t, gs) + } + return true, gs, fixture.gsUpdateErr + }) + m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { + podUpdated = true + ua := action.(k8stesting.UpdateAction) + p := ua.GetObject().(*corev1.Pod) + if fixture.checkPodUpdate != nil { + fixture.checkPodUpdate(t, p) + } + return true, p, fixture.podUpdateErr + }) - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - assert.NotEqual(t, containerID, gs.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) + synced := []cache.InformerSynced{c.podSynced} + if fixture.setupNode { + synced = append(synced, c.nodeSynced) + } + ctx, cancel := agtesting.StartInformers(m, synced...) + defer cancel() - return true, gs, nil - }) - m.KubeClient.AddReactor("update", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - ua := action.(k8stesting.UpdateAction) - pod := ua.GetObject().(*corev1.Pod) - assert.NotEqual(t, containerID, pod.Annotations[agonesv1.GameServerReadyContainerIDAnnotation]) - return true, pod, nil + gs, err := c.syncGameServerRequestReadyState(ctx, gsFixture) + fixture.check(t, gs, err, gsUpdated, podUpdated) + for _, event := range fixture.wantEvents { + agtesting.AssertEventContains(t, m.FakeRecorder.Events, event) + } }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - gs, err := c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.NoError(t, err, "should not error") - assert.True(t, gsUpdated, "GameServer wasn't updated") - assert.True(t, podUpdated, "Pod wasn't updated") - assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State) - agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SDK.Ready() complete") - }) + } for _, s := range []agonesv1.GameServerState{"Unknown", agonesv1.GameServerStateUnhealthy} { name := fmt.Sprintf("GameServer with %s state", s) @@ -1865,73 +2056,6 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { }) } - t.Run("GameServer whose pod is currently not in a running state, so should retry and not update", func(t *testing.T) { - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - pod.Status.ContainerStatuses = []corev1.ContainerStatus{{Name: gsFixture.Spec.Container}} - assert.Nil(t, err) - gsUpdated := false - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(_ k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - return true, nil, nil - }) - m.KubeClient.AddReactor("update", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - return true, nil, nil - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - _, err = c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.EqualError(t, err, "game server container for GameServer test in namespace default is not currently running, try again") - assert.False(t, gsUpdated, "GameServer was updated") - assert.False(t, podUpdated, "Pod was updated") - }) - - t.Run("GameServer whose pod is missing ContainerStatuses, so should retry and not update", func(t *testing.T) { - c, m := newFakeController() - - gsFixture := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} - gsFixture.ApplyDefaults() - gsFixture.Status.NodeName = nodeName - pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) - assert.Nil(t, err) - gsUpdated := false - podUpdated := false - - m.KubeClient.AddReactor("list", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.PodList{Items: []corev1.Pod{*pod}}, nil - }) - m.AgonesClient.AddReactor("update", "gameservers", func(_ k8stesting.Action) (bool, runtime.Object, error) { - gsUpdated = true - return true, nil, nil - }) - m.KubeClient.AddReactor("update", "pods", func(_ k8stesting.Action) (bool, runtime.Object, error) { - podUpdated = true - return true, nil, nil - }) - - ctx, cancel := agtesting.StartInformers(m, c.podSynced) - defer cancel() - - _, err = c.syncGameServerRequestReadyState(ctx, gsFixture) - assert.EqualError(t, err, "game server container for GameServer test in namespace default not present in pod status, try again") - assert.False(t, gsUpdated, "GameServer was updated") - assert.False(t, podUpdated, "Pod was updated") - }) - t.Run("GameServer with non zero deletion datetime", func(t *testing.T) { testWithNonZeroDeletionTimestamp(t, func(c *Controller, fixture *agonesv1.GameServer) (*agonesv1.GameServer, error) { return c.syncGameServerRequestReadyState(context.Background(), fixture) diff --git a/pkg/gameservers/gameservers.go b/pkg/gameservers/gameservers.go index c8aad6664b..227aae290c 100644 --- a/pkg/gameservers/gameservers.go +++ b/pkg/gameservers/gameservers.go @@ -114,6 +114,31 @@ func applyGameServerAddressAndPort(gs *agonesv1.GameServer, node *corev1.Node, p return gs, nil } +// applyGameServerPodIPs appends any PodIPs from the pod not already present in +// gs.Status.Addresses, returning whether any were added. +func applyGameServerPodIPs(gs *agonesv1.GameServer, pod *corev1.Pod) (*agonesv1.GameServer, bool) { + if len(pod.Status.PodIPs) == 0 { + return gs, false + } + existing := make(map[string]bool, len(gs.Status.Addresses)) + for _, addr := range gs.Status.Addresses { + if addr.Type == agonesv1.NodePodIP { + existing[addr.Address] = true + } + } + updated := false + for _, ip := range pod.Status.PodIPs { + if !existing[ip.IP] { + gs.Status.Addresses = append(gs.Status.Addresses, corev1.NodeAddress{ + Type: agonesv1.NodePodIP, + Address: ip.IP, + }) + updated = true + } + } + return gs, updated +} + // isBeforePodCreated checks to see if the GameServer is in a state in which the pod could not have been // created yet. This includes "Starting" in which a pod MAY exist, but may not yet be available, depending on when the // informer cache updates diff --git a/pkg/gameservers/gameservers_test.go b/pkg/gameservers/gameservers_test.go index 0d33700b11..a36a1ae928 100644 --- a/pkg/gameservers/gameservers_test.go +++ b/pkg/gameservers/gameservers_test.go @@ -203,6 +203,60 @@ func TestApplyGameServerAddressAndPort(t *testing.T) { }) } +func TestApplyGameServerPodIPs(t *testing.T) { + t.Parallel() + + newGS := func(existingAddresses []corev1.NodeAddress) *agonesv1.GameServer { + gs := &agonesv1.GameServer{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: newSingleContainerSpec(), + Status: agonesv1.GameServerStatus{Addresses: existingAddresses}, + } + gs.ApplyDefaults() + return gs + } + + t.Run("adds new PodIPs not in status", func(t *testing.T) { + gs := newGS(nil) + pod := &corev1.Pod{Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: "10.0.0.1"}}}} + + result, updated := applyGameServerPodIPs(gs, pod) + assert.True(t, updated) + assert.Len(t, result.Status.Addresses, 1) + assert.Equal(t, agonesv1.NodePodIP, result.Status.Addresses[0].Type) + assert.Equal(t, "10.0.0.1", result.Status.Addresses[0].Address) + }) + + t.Run("no-op when all PodIPs already present", func(t *testing.T) { + gs := newGS([]corev1.NodeAddress{{Type: agonesv1.NodePodIP, Address: "10.0.0.1"}}) + pod := &corev1.Pod{Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: "10.0.0.1"}}}} + + result, updated := applyGameServerPodIPs(gs, pod) + assert.False(t, updated) + assert.Len(t, result.Status.Addresses, 1) + }) + + t.Run("no-op when pod has no PodIPs", func(t *testing.T) { + gs := newGS(nil) + pod := &corev1.Pod{} + + result, updated := applyGameServerPodIPs(gs, pod) + assert.False(t, updated) + assert.Empty(t, result.Status.Addresses) + }) + + t.Run("adds only missing PodIPs from multiple pod IPs", func(t *testing.T) { + gs := newGS([]corev1.NodeAddress{{Type: agonesv1.NodePodIP, Address: "10.0.0.1"}}) + pod := &corev1.Pod{Status: corev1.PodStatus{PodIPs: []corev1.PodIP{{IP: "10.0.0.1"}, {IP: "10.0.0.2"}}}} + + result, updated := applyGameServerPodIPs(gs, pod) + assert.True(t, updated) + assert.Len(t, result.Status.Addresses, 2) + assert.Equal(t, "10.0.0.1", result.Status.Addresses[0].Address) + assert.Equal(t, "10.0.0.2", result.Status.Addresses[1].Address) + }) +} + func TestIsBeforePodCreated(t *testing.T) { fixture := map[string]struct { state agonesv1.GameServerState diff --git a/test/e2e/gameserver_test.go b/test/e2e/gameserver_test.go index e544d9ecfa..d69cd9b78c 100644 --- a/test/e2e/gameserver_test.go +++ b/test/e2e/gameserver_test.go @@ -54,6 +54,7 @@ const ( func TestCreateConnect(t *testing.T) { t.Parallel() + ctx := context.Background() gs := framework.DefaultGameServer(framework.Namespace) readyGs, err := framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs) if err != nil { @@ -64,24 +65,24 @@ func TestCreateConnect(t *testing.T) { assert.NotEmpty(t, readyGs.Status.Address) assert.NotEmpty(t, readyGs.Status.Addresses) - var hasPodIPAddress bool - for i, addr := range readyGs.Status.Addresses { - if addr.Type == agonesv1.NodePodIP { - assert.NotEmpty(t, readyGs.Status.Addresses[i].Address) - hasPodIPAddress = true - } - } - assert.True(t, hasPodIPAddress) - - assert.NotEmpty(t, readyGs.Status.NodeName) - assert.Equal(t, readyGs.Status.State, agonesv1.GameServerStateReady) + require.NotEmpty(t, readyGs.Status.NodeName) + require.Equal(t, readyGs.Status.State, agonesv1.GameServerStateReady) + // check connectivity before anything else. reply, err := framework.SendGameServerUDP(t, readyGs, "Hello World !") - if err != nil { - t.Fatalf("Could ping GameServer: %v", err) - } - - assert.Equal(t, "ACK: Hello World !\n", reply) + require.NoError(t, err) + require.Equal(t, "ACK: Hello World !\n", reply) + + // since the PodIP could come at any point, let's eventually it. + require.EventuallyWithT(t, func(c *assert.CollectT) { + readyGs, err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Get(ctx, readyGs.ObjectMeta.Name, metav1.GetOptions{}) + require.NoError(c, err) + for i, addr := range readyGs.Status.Addresses { + if addr.Type == agonesv1.NodePodIP { + require.NotEmpty(c, readyGs.Status.Addresses[i].Address) + } + } + }, 3*time.Minute, time.Second, "Failed to get PodIP") } func TestHostName(t *testing.T) {