diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index 5d46713b98..05ac46af2b 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -576,9 +576,8 @@ func (c *Controller) syncGameServerCreatingState(ctx context.Context, gs *agones loggerForGameServer(gs, c.baseLogger).Debug("Syncing Create State") // Maybe something went wrong, and the pod was created, but the state was never moved to Starting, so let's check - _, err := c.gameServerPod(gs) + pod, err := c.gameServerPod(gs) if k8serrors.IsNotFound(err) { - for i := range gs.Spec.Ports { if gs.Spec.Ports[i].PortPolicy == agonesv1.Static && gs.Spec.Ports[i].Protocol == agonesv1.ProtocolTCPUDP { name := gs.Spec.Ports[i].Name @@ -607,6 +606,9 @@ func (c *Controller) syncGameServerCreatingState(ctx context.Context, gs *agones } gsCopy := gs.DeepCopy() + + gsCopy, _ = applyGameServerPodIP(gsCopy, pod) + gsCopy.Status.State = agonesv1.GameServerStateStarting gs, err = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) if err != nil { @@ -906,11 +908,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) @@ -921,6 +918,8 @@ func (c *Controller) syncGameServerStartingState(ctx context.Context, gs *agones return gs, err } + gsCopy, _ = applyGameServerPodIP(gsCopy, pod) + gsCopy.Status.State = agonesv1.GameServerStateScheduled gs, err = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) if err != nil { @@ -954,12 +953,28 @@ func (c *Controller) syncGameServerRequestReadyState(ctx context.Context, gs *ag return nil, err } + var gsUpdated bool + + gsCopy, podIPUpdated := applyGameServerPodIP(gsCopy, pod) + if podIPUpdated { + // defer the update of the GameServer until the end of this function + // in case there is no gs state update and the podIP is available and populated + defer func(gs *agonesv1.GameServer, alreadyUpdated *bool) { + // Only update if the GameServer is not already updated + if alreadyUpdated == nil || *alreadyUpdated { + return + } + _, _ = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gs, metav1.UpdateOptions{}) + }(gsCopy, &gsUpdated) + } + // if the address hasn't been populated, and the Ready request comes // before the controller has had a chance to do it, then // do it here instead addressPopulated := false - if gs.Status.NodeName == "" { + if gsCopy.Status.NodeName == "" { addressPopulated = true + if pod.Spec.NodeName == "" { return gs, workerqueue.NewTraceError(errors.Errorf("node not yet populated for Pod %s", pod.ObjectMeta.Name)) } @@ -978,6 +993,9 @@ func (c *Controller) syncGameServerRequestReadyState(ctx context.Context, gs *ag return gs, err } + // if the address is already populated, then we don't need to re-update for podIPs + gsUpdated = true + gsCopy.Status.State = agonesv1.GameServerStateReady gs, err = c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{}) if err != nil { diff --git a/pkg/gameservers/controller_test.go b/pkg/gameservers/controller_test.go index fec5fa5771..187cc1774d 100644 --- a/pkg/gameservers/controller_test.go +++ b/pkg/gameservers/controller_test.go @@ -1189,33 +1189,6 @@ func TestControllerSyncGameServerStartingState(t *testing.T) { assert.NotEmpty(t, gs.Status.Ports) }) - t.Run("Error on podIPs not populated", 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 - - m.KubeClient.AddReactor("list", "nodes", func(_ k8stesting.Action) (bool, runtime.Object, error) { - return true, &corev1.NodeList{Items: []corev1.Node{node}}, 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) { - ua := action.(k8stesting.UpdateAction) - gs := ua.GetObject().(*agonesv1.GameServer) - assert.Equal(t, agonesv1.GameServerStateScheduled, gs.Status.State) - return true, gs, errors.New("update-err") - }) - ctx, cancel := agtesting.StartInformers(m, c.gameServerSynced, c.podSynced, c.nodeSynced) - defer cancel() - - _, err = c.syncGameServerStartingState(ctx, gsFixture) - assert.Error(t, err) - }) - t.Run("Error on update", func(t *testing.T) { c, m := newFakeController() gsFixture := newFixture() @@ -1455,6 +1428,9 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} gsFixture.ApplyDefaults() gsFixture.Status.NodeName = nodeName + gsFixture.Status.Addresses = []corev1.NodeAddress{ + {Type: agonesv1.NodePodIP, Address: "0.0.0.0"}, + } pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) require.NoError(t, err) pod.Status.ContainerStatuses = []corev1.ContainerStatus{ @@ -1515,6 +1491,9 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} gsFixture.ApplyDefaults() gsFixture.Status.NodeName = nodeName + gsFixture.Status.Addresses = []corev1.NodeAddress{ + {Type: agonesv1.NodePodIP, Address: "0.0.0.0"}, + } pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) require.NoError(t, err) pod.Status.ContainerStatuses = []corev1.ContainerStatus{ @@ -1559,6 +1538,9 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} gsFixture.ApplyDefaults() gsFixture.Status.NodeName = nodeName + gsFixture.Status.Addresses = []corev1.NodeAddress{ + {Type: agonesv1.NodePodIP, Address: "0.0.0.0"}, + } pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) require.NoError(t, err) pod.Status.ContainerStatuses = []corev1.ContainerStatus{ @@ -1602,6 +1584,9 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{State: agonesv1.GameServerStateRequestReady}} gsFixture.ApplyDefaults() gsFixture.Status.NodeName = nodeName + gsFixture.Status.Addresses = []corev1.NodeAddress{ + {Type: agonesv1.NodePodIP, Address: "0.0.0.0"}, + } pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{}) require.NoError(t, err) pod.ObjectMeta.Annotations = map[string]string{agonesv1.GameServerReadyContainerIDAnnotation: containerID} @@ -1659,6 +1644,7 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { ContainerID: containerID, }, } + pod.Status.PodIPs = []corev1.PodIP{{IP: ipv6Fixture}} gsUpdated := false podUpdated := false @@ -1699,7 +1685,10 @@ func TestControllerSyncGameServerRequestReadyState(t *testing.T) { 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) + assert.Equal(t, []corev1.NodeAddress{ + {Address: ipFixture, Type: "ExternalIP"}, + {Address: ipv6Fixture, Type: "PodIP"}, + }, gs.Status.Addresses) agtesting.AssertEventContains(t, m.FakeRecorder.Events, "Address and port populated") agtesting.AssertEventContains(t, m.FakeRecorder.Events, "SDK.Ready() complete") diff --git a/pkg/gameservers/gameservers.go b/pkg/gameservers/gameservers.go index d56c7e0096..39dd7d312a 100644 --- a/pkg/gameservers/gameservers.go +++ b/pkg/gameservers/gameservers.go @@ -93,12 +93,7 @@ func applyGameServerAddressAndPort(gs *agonesv1.GameServer, node *corev1.Node, p gs.Status.Addresses = addrs gs.Status.NodeName = pod.Spec.NodeName - for _, ip := range pod.Status.PodIPs { - gs.Status.Addresses = append(gs.Status.Addresses, corev1.NodeAddress{ - Type: agonesv1.NodePodIP, - Address: ip.IP, - }) - } + gs, _ = applyGameServerPodIP(gs, pod) if err := syncPodPortsToGameServer(gs, pod); err != nil { return gs, errors.Wrapf(err, "cloud product error syncing ports on GameServer %s", gs.ObjectMeta.Name) @@ -114,6 +109,34 @@ func applyGameServerAddressAndPort(gs *agonesv1.GameServer, node *corev1.Node, p return gs, nil } +// applyGameServerPodIP checks the pod IPs and adds them to the GameServer status if they are not already there. +func applyGameServerPodIP(gs *agonesv1.GameServer, pod *corev1.Pod) (*agonesv1.GameServer, bool) { + var updated bool + + if pod == nil { + return gs, false + } + + gsPodIPs := make(map[string]interface{}) + for _, addr := range gs.Status.Addresses { + if addr.Type == agonesv1.NodePodIP { + gsPodIPs[addr.Address] = nil + } + } + + for _, ip := range pod.Status.PodIPs { + if _, ok := gsPodIPs[ip.IP]; !ok { + 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