From 7daaaba49a25ed9143b2c4823413921f8ad48926 Mon Sep 17 00:00:00 2001 From: Zach Loafman Date: Wed, 11 Jan 2023 11:57:07 -0800 Subject: [PATCH] e2e framework: Timing based on cloud product, end on terminal states (#2893) Most of the "wait for state" functions use a hardcoded 5m. To allow for autoscaling variance on Autopilot, allow for 10m on Autopilot. In addition, the WaitForGameServerState function can understand which states are terminal to save some time in the case that a GameServer has already moved into a terminal state. Along the way: Remove New/NewWithRates as they are unused or unnecessary wrappers - in particular, they definitely don't need to be exported. Towards #2777 --- pkg/apis/agones/v1/gameserver.go | 8 ++++++ test/e2e/framework/framework.go | 47 +++++++++++++++++--------------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index c647471f57..dd7e3f8801 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -126,6 +126,14 @@ const ( var ( // GameServerRolePodSelector is the selector to get all GameServer Pods GameServerRolePodSelector = labels.SelectorFromSet(labels.Set{RoleLabel: GameServerLabelRole}) + + // TerminalGameServerStates is a set (map[GameServerState]bool) of states from which a GameServer will not recover. + // From state diagram at https://agones.dev/site/docs/reference/gameserver/ + TerminalGameServerStates = map[GameServerState]bool{ + GameServerStateShutdown: true, + GameServerStateError: true, + GameServerStateUnhealthy: true, + } ) // +genclient diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index a5bcdac86a..2fa3a40ef3 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -77,17 +77,7 @@ type Framework struct { Version string Namespace string CloudProduct string -} - -// New setups a testing framework using a kubeconfig path and the game server image to use for testing. -func New(kubeconfig string) (*Framework, error) { - return newFramework(kubeconfig, 0, 0) -} - -// NewWithRates setups a testing framework using a kubeconfig path and the game server image -// to use for load testing with QPS and Burst overwrites. -func NewWithRates(kubeconfig string, qps float32, burst int) (*Framework, error) { - return newFramework(kubeconfig, qps, burst) + WaitForState time.Duration // default time to wait for state changes, may change based on cloud product. } func newFramework(kubeconfig string, qps float32, burst int) (*Framework, error) { @@ -192,7 +182,7 @@ func NewFromFlags() (*Framework, error) { runtime.Must(runtime.FeaturesBindEnv()) runtime.Must(runtime.ParseFeaturesFromEnv()) - framework, err := New(viper.GetString(kubeconfigFlag)) + framework, err := newFramework(viper.GetString(kubeconfigFlag), 0, 0) if err != nil { return framework, err } @@ -203,6 +193,10 @@ func NewFromFlags() (*Framework, error) { framework.Version = viper.GetString(versionFlag) framework.Namespace = viper.GetString(namespaceFlag) framework.CloudProduct = viper.GetString(cloudProductFlag) + framework.WaitForState = 5 * time.Minute + if framework.CloudProduct == "gke-autopilot" { + framework.WaitForState = 10 * time.Minute // Autopilot can take a little while due to autoscaling, be a little liberal. + } logrus.WithField("gameServerImage", framework.GameServerImage). WithField("pullSecret", framework.PullSecret). @@ -228,7 +222,7 @@ func (f *Framework) CreateGameServerAndWaitUntilReady(t *testing.T, ns string, g log.WithField("gs", newGs.ObjectMeta.Name).Info("GameServer created, waiting for Ready") - readyGs, err := f.WaitForGameServerState(t, newGs, agonesv1.GameServerStateReady, 5*time.Minute) + readyGs, err := f.WaitForGameServerState(t, newGs, agonesv1.GameServerStateReady, f.WaitForState) if err != nil { return nil, fmt.Errorf("waiting for %v GameServer instance readiness timed out (%v): %v", @@ -266,22 +260,31 @@ func (f *Framework) WaitForGameServerState(t *testing.T, gs *agonesv1.GameServer checkGs, err = f.AgonesClient.AgonesV1().GameServers(gs.Namespace).Get(context.Background(), gs.Name, metav1.GetOptions{}) if err != nil { - logrus.WithError(err).Warn("error retrieving gameserver") + log.WithError(err).Warn("error retrieving GameServer") return false, nil } - log.WithField("gs", checkGs.ObjectMeta.Name). - WithField("currentState", checkGs.Status.State). - WithField("awaitingState", state).Info("Waiting for states to match") - - if checkGs.Status.State == state { + checkState := checkGs.Status.State + if checkState == state { + log.WithField("gs", checkGs.ObjectMeta.Name). + WithField("currentState", checkState). + WithField("awaitingState", state).Info("GameServer states match") return true, nil } + if agonesv1.TerminalGameServerStates[checkState] { + log.WithField("gs", checkGs.ObjectMeta.Name). + WithField("currentState", checkState). + WithField("awaitingState", state).Error("GameServer reached terminal state") + return false, errors.Errorf("GameServer reached terminal state %s", checkState) + } + log.WithField("gs", checkGs.ObjectMeta.Name). + WithField("currentState", checkState). + WithField("awaitingState", state).Info("Waiting for states to match") return false, nil }) - return checkGs, errors.Wrapf(err, "waiting for GameServer to be %v %v/%v", + return checkGs, errors.Wrapf(err, "waiting for GameServer %v/%v to be %v", state, gs.Namespace, gs.Name) } @@ -322,7 +325,7 @@ func (f *Framework) AssertFleetCondition(t *testing.T, flt *agonesv1.Fleet, cond func (f *Framework) WaitForFleetCondition(t *testing.T, flt *agonesv1.Fleet, condition func(*logrus.Entry, *agonesv1.Fleet) bool) error { log := TestLogger(t).WithField("fleet", flt.Name) log.Info("waiting for fleet condition") - err := wait.PollImmediate(2*time.Second, 5*time.Minute, func() (bool, error) { + err := wait.PollImmediate(2*time.Second, f.WaitForState, func() (bool, error) { fleet, err := f.AgonesClient.AgonesV1().Fleets(flt.ObjectMeta.Namespace).Get(context.Background(), flt.ObjectMeta.Name, metav1.GetOptions{}) if err != nil { return true, err @@ -421,7 +424,7 @@ func (f *Framework) WaitForFleetGameServersCondition(flt *agonesv1.Fleet, // specified by a callback and the size of GameServers to match fleet's Spec.Replicas. func (f *Framework) WaitForFleetGameServerListCondition(flt *agonesv1.Fleet, cond func(servers []agonesv1.GameServer) bool) error { - return wait.Poll(2*time.Second, 5*time.Minute, func() (done bool, err error) { + return wait.Poll(2*time.Second, f.WaitForState, func() (done bool, err error) { gsList, err := f.ListGameServersFromFleet(flt) if err != nil { return false, err