Make migration controller more forgiving wrt Node/GameServer addresses (

#3116) Make migration controller more forgiving wrt Node/GameServer addresses
googleforgames · Apr 24, 2023 · 15411ae · 15411ae
1 parent 0fc3532
commit 15411ae
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 5 deletions.
diff --git a/pkg/gameservers/migration.go b/pkg/gameservers/migration.go
@@ -16,6 +16,7 @@ package gameservers
 
 import (
 	"context"
+	"strings"
 
 	"agones.dev/agones/pkg/apis/agones"
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
@@ -30,6 +31,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
+	k8sv1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/informers"
@@ -179,12 +181,8 @@ func (mc *MigrationController) syncGameServer(ctx context.Context, key string) e
 	if err != nil {
 		return errors.Wrapf(err, "error retrieving node %s for Pod %s", pod.Spec.NodeName, pod.ObjectMeta.Name)
 	}
-	address, err := address(node)
-	if err != nil {
-		return err
-	}
 
-	if pod.Spec.NodeName != gs.Status.NodeName || address != gs.Status.Address {
+	if pod.Spec.NodeName != gs.Status.NodeName || !mc.anyAddressMatch(node, gs) {
 		gsCopy := gs.DeepCopy()
 
 		var eventMsg string
@@ -211,3 +209,19 @@ func (mc *MigrationController) syncGameServer(ctx context.Context, key string) e
 
 	return nil
 }
+
+func (mc *MigrationController) anyAddressMatch(node *k8sv1.Node, gs *agonesv1.GameServer) bool {
+	nodeAddresses := []string{}
+	for _, a := range node.Status.Addresses {
+		if a.Address == gs.Status.Address {
+			return true
+		}
+		nodeAddresses = append(nodeAddresses, a.Address)
+	}
+	mc.loggerForGameServer(gs).
+		WithField("gs", gs.Name).
+		WithField("gs.Status.Address", gs.Status.Address).
+		WithField("node.Status.Addresses", strings.Join(nodeAddresses, ",")).
+		Warn("GameServer/Node address mismatch")
+	return false
+}
diff --git a/pkg/gameservers/migration_test.go b/pkg/gameservers/migration_test.go
@@ -317,4 +317,86 @@ func TestMigrationControllerRun(t *testing.T) {
 	noChange()
 }
 
+func TestMigrationControllerAnyAddressMatch(t *testing.T) {
+	fixtures := map[string]struct {
+		matches       bool
+		gsAddress     string
+		nodeAddresses []corev1.NodeAddress
+	}{
+		"NodeHostName matches": {
+			matches:       true,
+			gsAddress:     "NodeHostName",
+			nodeAddresses: []corev1.NodeAddress{{Address: "NodeHostName", Type: corev1.NodeHostName}},
+		},
+		"NodeExternalDNS matches": {
+			matches:       true,
+			gsAddress:     "NodeExternalDNS",
+			nodeAddresses: []corev1.NodeAddress{{Address: "NodeExternalDNS", Type: corev1.NodeExternalDNS}},
+		},
+		"NodeExternalIP matches": {
+			matches:       true,
+			gsAddress:     "NodeExternalIP",
+			nodeAddresses: []corev1.NodeAddress{{Address: "NodeExternalIP", Type: corev1.NodeExternalIP}},
+		},
+		"NodeInternalDNS matches": {
+			matches:       true,
+			gsAddress:     "NodeInternalDNS",
+			nodeAddresses: []corev1.NodeAddress{{Address: "NodeInternalDNS", Type: corev1.NodeInternalDNS}},
+		},
+		"NodeInternalIP matches": {
+			matches:       true,
+			gsAddress:     "NodeInternalIP",
+			nodeAddresses: []corev1.NodeAddress{{Address: "NodeInternalIP", Type: corev1.NodeInternalIP}},
+		},
+		"no matches": {
+			matches:   false,
+			gsAddress: "no-match",
+			nodeAddresses: []corev1.NodeAddress{
+				{Address: "NodeHostName", Type: corev1.NodeHostName},
+				{Address: "NodeExternalDNS", Type: corev1.NodeExternalDNS},
+				{Address: "NodeExternalIP", Type: corev1.NodeExternalIP},
+				{Address: "NodeInternalDNS", Type: corev1.NodeInternalDNS},
+				{Address: "NodeInternalIP", Type: corev1.NodeInternalIP},
+			},
+		},
+		"matches one of many 1": {
+			matches:   true,
+			gsAddress: "NodeInternalDNS",
+			nodeAddresses: []corev1.NodeAddress{
+				{Address: "NodeHostName", Type: corev1.NodeHostName},
+				{Address: "NodeExternalDNS", Type: corev1.NodeExternalDNS},
+				{Address: "NodeExternalIP", Type: corev1.NodeExternalIP},
+				{Address: "NodeInternalDNS", Type: corev1.NodeInternalDNS},
+				{Address: "NodeInternalIP", Type: corev1.NodeInternalIP},
+			},
+		},
+		"matches one of many 2": {
+			matches:   true,
+			gsAddress: "NodeInternalIP",
+			nodeAddresses: []corev1.NodeAddress{
+				{Address: "NodeHostName", Type: corev1.NodeHostName},
+				{Address: "NodeExternalDNS", Type: corev1.NodeExternalDNS},
+				{Address: "NodeExternalIP", Type: corev1.NodeExternalIP},
+				{Address: "NodeInternalDNS", Type: corev1.NodeInternalDNS},
+				{Address: "NodeInternalIP", Type: corev1.NodeInternalIP},
+			},
+		},
+	}
+	for k, v := range fixtures {
+		t.Run(k, func(t *testing.T) {
+			m := agtesting.NewMocks()
+			c := NewMigrationController(healthcheck.NewHandler(), m.KubeClient, m.AgonesClient, m.KubeInformerFactory, m.AgonesInformerFactory, nilSyncPodPortsToGameServer)
+			gs := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Spec: newSingleContainerSpec(), Status: agonesv1.GameServerStatus{
+					Address: v.gsAddress,
+				}}
+			node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeFixtureName},
+				Status: corev1.NodeStatus{Addresses: v.nodeAddresses}}
+
+			matches := c.anyAddressMatch(node, gs)
+			assert.Equal(t, v.matches, matches)
+		})
+	}
+}
+
 func nilSyncPodPortsToGameServer(*agonesv1.GameServer, *corev1.Pod) error { return nil }