Skip to content

Commit

Permalink
Feat(gameservers): Add PodIP to GameServer Addresses (#3764)
Browse files Browse the repository at this point in the history
Add a new `PodIP` type to the `GameServer.addresses` array; populate with any IPs on the Pod.
  • Loading branch information
lacroixthomas authored Apr 19, 2024
1 parent 772fffb commit 86f535d
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 4 deletions.
3 changes: 3 additions & 0 deletions pkg/apis/agones/v1/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ const (
// error state. The timestamp is encoded in RFC3339 format.
GameServerErroredAtAnnotation = agones.GroupName + "/errored-at"

// NodePodIP identifies an IP address from a pod.
NodePodIP corev1.NodeAddressType = "PodIP"

// True is the string "true" to appease the goconst lint.
True = "true"
// False is the string "false" to appease the goconst lint.
Expand Down
6 changes: 6 additions & 0 deletions pkg/gameservers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,12 @@ func (c *Controller) syncGameServerStartingState(ctx context.Context, gs *agones
if pod.Spec.NodeName == "" {
return gs, workerqueue.NewDebugError(errors.Errorf("node not yet populated for Pod %s", pod.ObjectMeta.Name))
}

// Ensure the pod IPs are populated
if pod.Status.PodIPs == nil || len(pod.Status.PodIPs) == 0 {
return gs, workerqueue.NewDebugError(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)
Expand Down
41 changes: 39 additions & 2 deletions pkg/gameservers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import (

const (
ipFixture = "12.12.12.12"
ipv6Fixture = "2001:0db8:85a3:0000:0000:8a2e:0370:7334"
nodeFixtureName = "node1"
)

Expand Down Expand Up @@ -88,6 +89,7 @@ func TestControllerSyncGameServer(t *testing.T) {
ca := action.(k8stesting.CreateAction)
pod := ca.GetObject().(*corev1.Pod)
pod.Spec.NodeName = node.ObjectMeta.Name
pod.Status.PodIPs = []corev1.PodIP{{IP: ipv6Fixture}}
podCreated = true
assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Name)
watchPods.Add(pod)
Expand Down Expand Up @@ -120,7 +122,10 @@ func TestControllerSyncGameServer(t *testing.T) {
assert.Equal(t, expectedState, gs.Status.State)
if expectedState == agonesv1.GameServerStateScheduled {
assert.Equal(t, ipFixture, gs.Status.Address)
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)
assert.NotEmpty(t, gs.Status.Ports[0].Port)
}

Expand Down Expand Up @@ -1093,6 +1098,7 @@ func TestControllerSyncGameServerStartingState(t *testing.T) {
pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{})
assert.Nil(t, err)
pod.Spec.NodeName = nodeFixtureName
pod.Status.PodIPs = []corev1.PodIP{{IP: ipv6Fixture}}
gsUpdated := false

m.KubeClient.AddReactor("list", "nodes", func(action k8stesting.Action) (bool, runtime.Object, error) {
Expand All @@ -1118,19 +1124,50 @@ func TestControllerSyncGameServerStartingState(t *testing.T) {
assert.True(t, gsUpdated)
assert.Equal(t, gs.Status.NodeName, node.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")
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(action k8stesting.Action) (bool, runtime.Object, error) {
return true, &corev1.NodeList{Items: []corev1.Node{node}}, nil
})
m.KubeClient.AddReactor("list", "pods", func(action 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()
gsFixture.ApplyDefaults()
pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{})
require.NoError(t, err)
pod.Spec.NodeName = nodeFixtureName
pod.Status.PodIPs = []corev1.PodIP{{IP: ipv6Fixture}}

m.KubeClient.AddReactor("list", "nodes", func(action k8stesting.Action) (bool, runtime.Object, error) {
return true, &corev1.NodeList{Items: []corev1.Node{node}}, nil
Expand Down
7 changes: 7 additions & 0 deletions pkg/gameservers/gameservers.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ 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,
})
}

if err := syncPodPortsToGameServer(gs, pod); err != nil {
return gs, errors.Wrapf(err, "cloud product error syncing ports on GameServer %s", gs.ObjectMeta.Name)
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/gameservers/gameservers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func TestApplyGameServerAddressAndPort(t *testing.T) {
pod, err := gsFixture.Pod(agtesting.FakeAPIHooks{})
require.NoError(t, err)
pod.Spec.NodeName = node.ObjectMeta.Name
pod.Status.PodIPs = []corev1.PodIP{{IP: ipFixture}}
tc.podMod(pod)

gs, err := applyGameServerAddressAndPort(gsFixture, node, pod, tc.podSyncer)
Expand All @@ -146,6 +147,10 @@ func TestApplyGameServerAddressAndPort(t *testing.T) {
}
assert.Equal(t, ipFixture, gs.Status.Address)
assert.Equal(t, node.ObjectMeta.Name, gs.Status.NodeName)
assert.Equal(t, []corev1.NodeAddress{
{Address: ipFixture, Type: "ExternalIP"},
{Address: ipFixture, Type: "PodIP"},
}, gs.Status.Addresses)
})
}

Expand Down
6 changes: 4 additions & 2 deletions site/content/en/docs/Reference/gameserver.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ Game Servers are created through Kubernetes API (either directly or through a [F

[`GameServer.Status`][gss] has two fields which reflect the network address of the `GameServer`: `address` and `addresses`.
The `address` field is a policy-based choice of "primary address" that will work for many use cases,
and will always be one of the `addresses`. The `addresses` field contains every address in the [`Node.Status.addresses`][addresses],
and will always be one of the `addresses`. The `addresses` field contains every address in the [`Node.Status.addresses`][addresses] and [`Pod.Status.podIPs`][podIPs] (to allow a direct pod access),
representing all known ways to reach the `GameServer` over the network.

To choose `address` from `addresses`, [Agones looks for the following address types][addressFunc], in highest to lowest priorty:
Expand All @@ -199,11 +199,13 @@ To choose `address` from `addresses`, [Agones looks for the following address ty
* `InternalDNS`
* `InternalIP`

e.g. if any `ExternalDNS` address is found in the respective `Node`, it is used as the `address`.
e.g. if any `ExternalDNS` address is found in the respective `Node`, it is used as the `address`. (`PodIP` is not considered
for `address`.)

The policy for `address` will work for many use-cases, but for some advanced cases, such as IPv6 enablement, you may need
to evaluate all `addresses` and pick the addresses that best suits your needs.

[addresses]: https://v1-26.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#nodeaddress-v1-core
[podIPs]: https://v1-26.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#podip-v1-core
[addressFunc]: https://github.com/googleforgames/agones/blob/a59c5394c7f5bac66e530d21446302581c10c225/pkg/gameservers/gameservers.go#L37-L71
[gss]: {{% ref "/docs/Reference/agones_crd_api_reference.html#agones.dev/v1.GameServerStatus" %}}
10 changes: 10 additions & 0 deletions test/e2e/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ func TestCreateConnect(t *testing.T) {
assert.NotEmpty(t, readyGs.Status.Ports[0].Port)
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)

Expand Down

0 comments on commit 86f535d

Please sign in to comment.