From 6c07ea9e03a6226d292b76fc6e9db398f408e4ff Mon Sep 17 00:00:00 2001 From: Dimitris Gkanatsios Date: Sat, 16 Apr 2022 12:00:27 -0700 Subject: [PATCH] adding unit tests --- cmd/nodeagent/nodeagentmanager.go | 68 +----- cmd/nodeagent/nodeagentmanager_test.go | 201 ++++++++++++++++++ cmd/nodeagent/types.go | 16 ++ cmd/nodeagent/utilities.go | 49 +++++ docs/architecture.md | 2 +- .../config/manager/kustomization.yaml | 4 +- pkg/operator/http/allocate.go | 14 -- 7 files changed, 271 insertions(+), 83 deletions(-) diff --git a/cmd/nodeagent/nodeagentmanager.go b/cmd/nodeagent/nodeagentmanager.go index 2817cdd7..be16d95c 100644 --- a/cmd/nodeagent/nodeagentmanager.go +++ b/cmd/nodeagent/nodeagentmanager.go @@ -3,7 +3,6 @@ package main import ( "context" "encoding/json" - "errors" "fmt" "net/http" "regexp" @@ -11,8 +10,6 @@ import ( "time" mpsv1alpha1 "github.com/playfab/thundernetes/pkg/operator/api/v1alpha1" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -35,20 +32,6 @@ const ( ErrHealthNotExists = "health does not exist" ) -var ( - GameServerStates = promauto.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: "thundernetes", - Name: "gameserver_states", - Help: "Game server states", - }, []string{"name", "state"}) - - ConnectedPlayersGauge = promauto.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: "thundernetes", - Name: "connected_players", - Help: "Number of connected players per GameServer", - }, []string{"namespace", "name"}) -) - // NodeAgentManager manages the GameServer CRs that reside on this Node // these game server process heartbeat to the NodeAgent process // There is a two way communication between the game server and the NodeAgent @@ -138,7 +121,7 @@ func (n *NodeAgentManager) gameServerCreatedOrUpdated(obj *unstructured.Unstruct n.gameServerMap.Store(gameServerName, gsdi) } - gameServerState, _, err := n.parseStateHealth(obj) + gameServerState, _, err := parseStateHealth(obj) if err != nil { logger.Warnf("parsing state/health: %s. This is OK if the GameServer was just created", err.Error()) } @@ -149,7 +132,7 @@ func (n *NodeAgentManager) gameServerCreatedOrUpdated(obj *unstructured.Unstruct } // server is Active, so get session details as well initial players details - sessionID, sessionCookie, initialPlayers := n.parseSessionDetails(obj, gameServerName, gameServerNamespace) + sessionID, sessionCookie, initialPlayers := parseSessionDetails(obj, gameServerName, gameServerNamespace) logger.Infof("getting values from allocation - GameServer CR, sessionID:%s, sessionCookie:%s, initialPlayers: %v", sessionID, sessionCookie, initialPlayers) // create the GameServerDetails CR @@ -416,53 +399,6 @@ func (n *NodeAgentManager) updateConnectedPlayersIfNeeded(ctx context.Context, h return nil } -// parseSessionDetails returns the sessionID and sessionCookie from the unstructured GameServer CR -func (n *NodeAgentManager) parseSessionDetails(u *unstructured.Unstructured, gameServerName, gameServerNamespace string) (string, string, []string) { - logger := getLogger(gameServerName, gameServerNamespace) - sessionID, sessionIDExists, sessionIDErr := unstructured.NestedString(u.Object, "status", "sessionID") - sessionCookie, sessionCookieExists, sessionCookieErr := unstructured.NestedString(u.Object, "status", "sessionCookie") - initialPlayers, initialPlayersExists, initialPlayersErr := unstructured.NestedStringSlice(u.Object, "status", "initialPlayers") - - if !sessionIDExists || !sessionCookieExists || !initialPlayersExists { - logger.Debugf("sessionID or sessionCookie or initialPlayers do not exist, sessionIDExists: %t, sessionCookieExists: %t, initialPlayersExists: %t", sessionIDExists, sessionCookieExists, initialPlayersExists) - } - - if sessionIDErr != nil { - logger.Debugf("error getting sessionID: %s", sessionIDErr.Error()) - } - - if sessionCookieErr != nil { - logger.Debugf("error getting sessionCookie: %s", sessionCookieErr.Error()) - } - - if initialPlayersErr != nil { - logger.Debugf("error getting initialPlayers: %s", initialPlayersErr.Error()) - } - - return sessionID, sessionCookie, initialPlayers -} - -// parseState parses the GameServer state from the unstructured GameServer CR -func (n *NodeAgentManager) parseStateHealth(u *unstructured.Unstructured) (string, string, error) { - state, stateExists, stateErr := unstructured.NestedString(u.Object, "status", "state") - health, healthExists, healthErr := unstructured.NestedString(u.Object, "status", "health") - - if stateErr != nil { - return "", "", stateErr - } - if !stateExists { - return "", "", errors.New(ErrStateNotExists) - } - - if healthErr != nil { - return "", "", stateErr - } - if !healthExists { - return "", "", errors.New(ErrHealthNotExists) - } - return state, health, nil -} - // createGameServerDetails creates a GameServerDetails CR with the specified name and namespace func (n *NodeAgentManager) createGameServerDetails(ctx context.Context, gsuid types.UID, gsname, gsnamespace string, connectedPlayers []string) error { gs := &mpsv1alpha1.GameServer{ diff --git a/cmd/nodeagent/nodeagentmanager_test.go b/cmd/nodeagent/nodeagentmanager_test.go index 932c5311..3eefd160 100644 --- a/cmd/nodeagent/nodeagentmanager_test.go +++ b/cmd/nodeagent/nodeagentmanager_test.go @@ -15,6 +15,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -167,6 +168,188 @@ var _ = Describe("nodeagent tests", func() { Expect(err).ToNot(HaveOccurred()) Expect(hbr.Operation).To(Equal(GameOperationContinue)) }) + It("should not create a GameServerDetail if the server is not Active", func() { + dynamicClient := newDynamicInterface() + + n := NewNodeAgentManager(dynamicClient, testNodeName, false) + gs := createUnstructuredTestGameServer(testGameServerName, testGameServerNamespace) + + _, err := dynamicClient.Resource(gameserverGVR).Namespace(testGameServerNamespace).Create(context.Background(), gs, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + // wait for the create trigger on the watch + Eventually(func() bool { + var ok bool + _, ok = n.gameServerMap.Load(testGameServerName) + return ok + }).Should(BeTrue()) + + // simulate 5 standingBy heartbeats + for i := 0; i < 5; i++ { + hb := &HeartbeatRequest{ + CurrentGameState: GameStateStandingBy, + CurrentGameHealth: "Healthy", + } + b, _ := json.Marshal(hb) + req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sessionHosts/%s", testGameServerName), bytes.NewReader(b)) + w := httptest.NewRecorder() + + n.heartbeatHandler(w, req) + res := w.Result() + defer res.Body.Close() + Expect(res.StatusCode).To(Equal(http.StatusOK)) + resBody, err := ioutil.ReadAll(res.Body) + Expect(err).ToNot(HaveOccurred()) + hbr := HeartbeatResponse{} + _ = json.Unmarshal(resBody, &hbr) + Expect(hbr.Operation).To(Equal(GameOperationContinue)) + } + + _, err = dynamicClient.Resource(gameserverDetailGVR).Namespace(testGameServerNamespace).Get(context.Background(), gs.GetName(), metav1.GetOptions{}) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + + }) + It("should delete the GameServer from the cache when it's delete on K8s", func() { + dynamicClient := newDynamicInterface() + + n := NewNodeAgentManager(dynamicClient, testNodeName, false) + gs := createUnstructuredTestGameServer(testGameServerName, testGameServerNamespace) + + _, err := dynamicClient.Resource(gameserverGVR).Namespace(testGameServerNamespace).Create(context.Background(), gs, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + // wait for the create trigger on the watch + Eventually(func() bool { + var ok bool + _, ok = n.gameServerMap.Load(testGameServerName) + return ok + }).Should(BeTrue()) + + err = dynamicClient.Resource(gameserverGVR).Namespace(testGameServerNamespace).Delete(context.Background(), gs.GetName(), metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() bool { + var ok bool + _, ok = n.gameServerMap.Load(testGameServerName) + return ok + }).Should(BeTrue()) + }) + It("should create a GameServerDetail on subsequent heartbeats, if it fails on the first time", func() { + dynamicClient := newDynamicInterface() + + n := NewNodeAgentManager(dynamicClient, testNodeName, false) + gs := createUnstructuredTestGameServer(testGameServerName, testGameServerNamespace) + + _, err := dynamicClient.Resource(gameserverGVR).Namespace(testGameServerNamespace).Create(context.Background(), gs, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + // wait for the create trigger on the watch + var gsinfo interface{} + Eventually(func() bool { + var ok bool + gsinfo, ok = n.gameServerMap.Load(testGameServerName) + return ok + }).Should(BeTrue()) + + // simulate subsequent updates by GSDK + gsinfo.(*GameServerInfo).PreviousGameState = GameStateStandingBy + gsinfo.(*GameServerInfo).PreviousGameHealth = "Healthy" + + // update GameServer CR to active + gs.Object["status"].(map[string]interface{})["state"] = "Active" + gs.Object["status"].(map[string]interface{})["health"] = "Healthy" + _, err = dynamicClient.Resource(gameserverGVR).Namespace(testGameServerNamespace).Update(context.Background(), gs, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + // wait for the update trigger on the watch + Eventually(func() bool { + tempgs, ok := n.gameServerMap.Load(testGameServerName) + if !ok { + return false + } + tempgs.(*GameServerInfo).Mutex.RLock() + gsd := *tempgs.(*GameServerInfo) + tempgs.(*GameServerInfo).Mutex.RUnlock() + return gsd.IsActive && gsd.PreviousGameState == GameStateStandingBy + }).Should(BeTrue()) + + // wait till the GameServerDetail CR has been created + Eventually(func(g Gomega) { + u, err := dynamicClient.Resource(gameserverDetailGVR).Namespace(testGameServerNamespace).Get(context.Background(), gs.GetName(), metav1.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(u.GetName()).To(Equal(gs.GetName())) + }).Should(Succeed()) + + // delete the GameServerDetail CR to simulate failure in creating + Eventually(func(g Gomega) { + err := dynamicClient.Resource(gameserverDetailGVR).Namespace(testGameServerNamespace).Delete(context.Background(), gs.GetName(), metav1.DeleteOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + }).Should(Succeed()) + + // heartbeat from the game is still StandingBy + hb := &HeartbeatRequest{ + CurrentGameState: GameStateStandingBy, + CurrentGameHealth: "Healthy", + } + b, _ := json.Marshal(hb) + req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sessionHosts/%s", testGameServerName), bytes.NewReader(b)) + w := httptest.NewRecorder() + + // but the response from NodeAgent should be active + n.heartbeatHandler(w, req) + res := w.Result() + defer res.Body.Close() + Expect(res.StatusCode).To(Equal(http.StatusOK)) + resBody, err := ioutil.ReadAll(res.Body) + Expect(err).ToNot(HaveOccurred()) + hbr := HeartbeatResponse{} + _ = json.Unmarshal(resBody, &hbr) + Expect(hbr.Operation).To(Equal(GameOperationActive)) + + // next heartbeat response should be continue + // at the same time, game is adding some connected players + hb = &HeartbeatRequest{ + CurrentGameState: GameStateActive, // heartbeat is now active + CurrentGameHealth: "Healthy", + CurrentPlayers: getTestConnectedPlayers(), // adding connected players + } + b, _ = json.Marshal(hb) + req = httptest.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sessionHosts/%s", testGameServerName), bytes.NewReader(b)) + w = httptest.NewRecorder() + n.heartbeatHandler(w, req) + res = w.Result() + defer res.Body.Close() + // first heartbeat after Active should fail since the GameServerDetail is missing + Expect(res.StatusCode).To(Equal(http.StatusInternalServerError)) + + // next heartbeat should succeed + hb = &HeartbeatRequest{ + CurrentGameState: GameStateActive, // heartbeat is now active + CurrentGameHealth: "Healthy", + CurrentPlayers: getTestConnectedPlayers(), + } + b, _ = json.Marshal(hb) + req = httptest.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sessionHosts/%s", testGameServerName), bytes.NewReader(b)) + w = httptest.NewRecorder() + n.heartbeatHandler(w, req) + res = w.Result() + defer res.Body.Close() + Expect(res.StatusCode).To(Equal(http.StatusOK)) + resBody, err = ioutil.ReadAll(res.Body) + Expect(err).ToNot(HaveOccurred()) + hbr = HeartbeatResponse{} + err = json.Unmarshal(resBody, &hbr) + Expect(err).ToNot(HaveOccurred()) + Expect(hbr.Operation).To(Equal(GameOperationContinue)) + + // make sure the GameServerDetail CR has been created + Eventually(func(g Gomega) { + u, err := dynamicClient.Resource(gameserverDetailGVR).Namespace(testGameServerNamespace).Get(context.Background(), gs.GetName(), metav1.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(u.GetName()).To(Equal(gs.GetName())) + }).Should(Succeed()) + }) It("should handle a lot of simultaneous heartbeats from different game servers", func() { rand.Seed(time.Now().UnixNano()) @@ -216,6 +399,13 @@ var _ = Describe("nodeagent tests", func() { return gsd.IsActive && tempgs.(*GameServerInfo).PreviousGameState == GameStateStandingBy }).Should(BeTrue()) + // wait till the GameServerDetail CR has been created + Eventually(func(g Gomega) { + u, err := dynamicClient.Resource(gameserverDetailGVR).Namespace(testGameServerNamespace).Get(context.Background(), gs.GetName(), metav1.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(u.GetName()).To(Equal(gs.GetName())) + }).Should(Succeed()) + // heartbeat from the game is still StandingBy hb := &HeartbeatRequest{ CurrentGameState: GameStateStandingBy, @@ -299,6 +489,17 @@ func createUnstructuredTestGameServer(name, namespace string) *unstructured.Unst return &unstructured.Unstructured{Object: g} } +func getTestConnectedPlayers() []ConnectedPlayer { + return []ConnectedPlayer{ + { + PlayerId: "player1", + }, + { + PlayerId: "player2", + }, + } +} + func TestNodeAgent(t *testing.T) { RegisterFailHandler(Fail) diff --git a/cmd/nodeagent/types.go b/cmd/nodeagent/types.go index 47d651a4..683a72ee 100644 --- a/cmd/nodeagent/types.go +++ b/cmd/nodeagent/types.go @@ -3,6 +3,8 @@ package main import ( "sync" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" ) @@ -44,6 +46,20 @@ const ( GameOperationTerminate GameOperation = "Terminate" ) +var ( + GameServerStates = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: "thundernetes", + Name: "gameserver_states", + Help: "Game server states", + }, []string{"name", "state"}) + + ConnectedPlayersGauge = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: "thundernetes", + Name: "connected_players", + Help: "Number of connected players per GameServer", + }, []string{"namespace", "name"}) +) + // HeartbeatRequest contains data for the heartbeat request coming from the GSDK running alongside GameServer type HeartbeatRequest struct { // CurrentGameState is the current state of the game server diff --git a/cmd/nodeagent/utilities.go b/cmd/nodeagent/utilities.go index 6d12c218..2eea8035 100644 --- a/cmd/nodeagent/utilities.go +++ b/cmd/nodeagent/utilities.go @@ -7,6 +7,7 @@ import ( "strings" log "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" ) @@ -90,3 +91,51 @@ func sanitize(s string) string { s2 := strings.Replace(s, "\n", "", -1) return strings.Replace(s2, "\r", "", -1) } + +// parseSessionDetails returns the sessionID and sessionCookie from the unstructured GameServer CR +func parseSessionDetails(u *unstructured.Unstructured, gameServerName, gameServerNamespace string) (string, string, []string) { + logger := getLogger(gameServerName, gameServerNamespace) + sessionID, sessionIDExists, sessionIDErr := unstructured.NestedString(u.Object, "status", "sessionID") + sessionCookie, sessionCookieExists, sessionCookieErr := unstructured.NestedString(u.Object, "status", "sessionCookie") + initialPlayers, initialPlayersExists, initialPlayersErr := unstructured.NestedStringSlice(u.Object, "status", "initialPlayers") + + if !sessionIDExists || !sessionCookieExists || !initialPlayersExists { + logger.Debugf("sessionID or sessionCookie or initialPlayers do not exist, sessionIDExists: %t, sessionCookieExists: %t, initialPlayersExists: %t", sessionIDExists, sessionCookieExists, initialPlayersExists) + } + + if sessionIDErr != nil { + logger.Debugf("error getting sessionID: %s", sessionIDErr.Error()) + } + + if sessionCookieErr != nil { + logger.Debugf("error getting sessionCookie: %s", sessionCookieErr.Error()) + } + + if initialPlayersErr != nil { + logger.Debugf("error getting initialPlayers: %s", initialPlayersErr.Error()) + } + + return sessionID, sessionCookie, initialPlayers +} + +// parseState parses the GameServer state from the unstructured GameServer CR. +// Returns state, health and error +func parseStateHealth(u *unstructured.Unstructured) (string, string, error) { + state, stateExists, stateErr := unstructured.NestedString(u.Object, "status", "state") + health, healthExists, healthErr := unstructured.NestedString(u.Object, "status", "health") + + if stateErr != nil { + return "", "", stateErr + } + if !stateExists { + return "", "", errors.New(ErrStateNotExists) + } + + if healthErr != nil { + return "", "", stateErr + } + if !healthExists { + return "", "", errors.New(ErrHealthNotExists) + } + return state, health, nil +} diff --git a/docs/architecture.md b/docs/architecture.md index 1b825f9c..a5430958 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -75,6 +75,6 @@ There are two ways we could accomplish the second step: For communicating with the NodeAgent, we eventually picked the first approach. The second approach was used initially but was abandoned due to security concerns. -Moreover, when the user allocates a GameServer, we create an instance of the GameServerDetail custom resource which stores the InitialPlayers (if any). The GameServerDetail has a 1:1 relationship with the GameServer CR and share the same name. Moreover, GameServer is the owner of the GameServerDetail instance so both will be deleted, upon GameServer's deletion. The GameServerDetail also tracks the ConnectedPlayersCount and ConnectedPlayers names/IDs, which can be updated by the UpdateConnectedPlayers GSDK method. +Moreover, when the user allocates a game server we create an instance of the GameServerDetail custom resource which stores details about the connected players of the game. User can call the `UpdateConnectedPlayers` GSDK method from within the game server process to update the connected players. The GameServerDetail has a 1:1 relationship with the GameServer CR and share the same name. Moreover, GameServer is the owner of the GameServerDetail instance so both will be deleted, upon GameServer's deletion. Worth mentioning here is the fact that up to 0.1, the NodeAgent process was a sidecar container that lived inside the GameServer Pod. However, on version 0.2 we transitioned to a NodeAgent Pod that runs on all Nodes in the cluster. This was done to avoid the need for a sidecar container and also made `hostNetwork` functionality available. diff --git a/pkg/operator/config/manager/kustomization.yaml b/pkg/operator/config/manager/kustomization.yaml index acc1dff0..cb79ba1d 100755 --- a/pkg/operator/config/manager/kustomization.yaml +++ b/pkg/operator/config/manager/kustomization.yaml @@ -10,5 +10,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization images: - name: controller - newName: thundernetes-operator - newTag: 12b9051 + newName: ghcr.io/playfab/thundernetes-operator + newTag: 0.3.2 diff --git a/pkg/operator/http/allocate.go b/pkg/operator/http/allocate.go index 2c713d8f..4d1695a8 100644 --- a/pkg/operator/http/allocate.go +++ b/pkg/operator/http/allocate.go @@ -1,7 +1,6 @@ package http import ( - "context" "encoding/json" "errors" "math/rand" @@ -17,7 +16,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" - "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -145,15 +143,3 @@ func (h *allocateHandler) handle(w http.ResponseWriter, r *http.Request) { } controllers.AllocationsCounter.WithLabelValues(gs.Labels[controllers.LabelBuildName]).Inc() } - -// deleteFameServerDetail deletes the GameServerDetail object for the given GameServer with backoff retries -func (h *allocateHandler) deleteGameServerDetail(ctx context.Context, gsd *mpsv1alpha1.GameServerDetail) error { - err := retry.OnError(retry.DefaultBackoff, - func(_ error) bool { - return true // TODO: check if we can do something better here, like check for timeouts? - }, func() error { - err := h.client.Delete(ctx, gsd) - return err - }) - return err -}