More Local Dev Server Support (#3252)

* More Local Dev Server Support

This commit adds better support for integration between Dev Servers (called [Local Game Server](https://agones.dev/site/docs/guides/local-game-server/) in guides) and [Local Development](https://agones.dev/site/docs/guides/client-sdks/local/).
Note that these changes do not affect prod flows, only developer flows when running manually with extra (new) command line arguments.

* Updating SDK Server (`sdk-server/main.go`) for "out of cluster" mode.
  * Adding `--kubeconfig` cli flag to pass in a suitable k8s config file.
    * Makes use of `clientcmd.BuildConfigFromFlags()` to connect to the k8s cluster.
    * If the flag is not provided (as is the case for typical prod use), the `BuildConfigFromFlags()` call will internally fall back to the `InClusterConfig()`.
    * This is similar to the `--kubeconfig` cli arg for `cmd/controller/main.go`.
  * Adding `--no-graceful-termination` cli flag.
    * If set (default unset), the context `ctx` object will not be wrapped by the `NewSDKServerContext()` call. This means that exiting the program will not wait (block) for the `GameServer` state to update to `Shutdown`.
    * This flag to disable graceful shutdown is added as a convenient dev flow option, to quickly terminate and restart the SDK Server when testing locally.
    * Graceful termination was first added as an optional feature in #2205.
      * Fully rolled into stable (no longer optional feature) in #3231.
    * This flag does not affect normal (prod) flows (graceful termination still enabled by default).
* Updating gameservers/controller.go to handle `RequestReady` to `Ready` transition.
  * `RequestReady` is set via SDK Server (from SDK calls). Need to transition to `Ready` to be allocatable.
* Updated "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) with information with running "out of cluster" mode.
  * Also added information about running from source code instead of prebuilt binaries -- which many devs would prefer.
* Added links between "Local Development" guide page (`docs/Guides/Client SDKs/local.md`) and "Local Game Server" guide page (`docs/Guides/local-game-server.md`) about how they can be used together.

* * Update documentation clarification location... -ation.

* * Fixing formatting (had 1 too many spaces).

* * Fixing ref relative paths.

* * Removing quotes missed in previous cleanups.

* * As per review feedback, updating `GAMESERVER_NAME` and `POD_NAMESPACE` to be passable via cli args instead of only as environment variables.
  * This should still function using environment variables due to calls to `viper.BindEnv()`.
  * Manually verified functionality when running locally with either cli args or environment variables.
  * Moving values to pass through `config` struct, just as other flags are passed.
* Updating documentation to use cli args instead of environment variables.

* * As per review feedback, changing from negative to positive logic for graceful termination flag.
  * Renaming `--no-graceful-termination` to `--graceful-termination`.
  * Setting default value to `true`.
  * Manually verified locally that graceful termination is on by default.
* Updating indention.
* Updating documentation to show renamed cli arg.

* * Updating links to use `ref` instead of absolute site link.

* * Switching "out of cluster" command example to use binary instead of running from source.
  * Done for consistency with other examples.

* * Adding controller test for RequestReady -> Ready state progression.

* * Adding `RequestReady` -> `Ready` state progressing in `TestDevelopmentGameServerLifecycle` e2e test.

* * Fixing call to `StartInformers()` in test.

* * Moving discussion of running from source code down to the bottom.

* * Adding more cross-reference links for game server allocation.

* * Moving "out of cluster" documentation into its own file.
  * Going into much more detail about background and steps to complete.
  * `publishDate` set to `2023-08-15T07:00:00Z` (midnight PST) to not publish doc immediately.
    * `2023-08-15` chosen to coincide with next release, per https://github.com/googleforgames/agones/blob/main/docs/governance/release_process.md#release-calendar.
* Adding links to new page from `Local Game Server` and `Local Development` pages.
  * Links are wrapped by `publishVersion="1.34.0"` to not show immediately. `1.34.0` is the next release, per https://github.com/googleforgames/agones/blob/main/docs/governance/release_process.md#release-calendar.

* * Switching OS/file description back to bulleted form similar to how it was before (but now as 1 set of bullets).

* * Removing link to moved section (covered by "Next Steps" section).

* * Cleaning up some wording.

* * Adding comments about restarting locally run binaries.

* * Proofread and update.

cmd/sdk-server/main.go

@@ -42,26 +42,27 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
 )
 
 const (
 	defaultGRPCPort = 9357
 	defaultHTTPPort = 9358
 
-	// specifically env vars
-	gameServerNameEnv = "GAMESERVER_NAME"
-	podNamespaceEnv   = "POD_NAMESPACE"
-
 	// Flags (that can also be env vars)
-	localFlag       = "local"
-	fileFlag        = "file"
-	testFlag        = "test"
-	testSdkNameFlag = "sdk-name"
-	addressFlag     = "address"
-	delayFlag       = "delay"
-	timeoutFlag     = "timeout"
-	grpcPortFlag    = "grpc-port"
-	httpPortFlag    = "http-port"
+	gameServerNameFlag      = "gameserver-name"
+	podNamespaceFlag        = "pod-namespace"
+	localFlag               = "local"
+	fileFlag                = "file"
+	testFlag                = "test"
+	testSdkNameFlag         = "sdk-name"
+	kubeconfigFlag          = "kubeconfig"
+	gracefulTerminationFlag = "graceful-termination"
+	addressFlag             = "address"
+	delayFlag               = "delay"
+	timeoutFlag             = "timeout"
+	grpcPortFlag            = "grpc-port"
+	httpPortFlag            = "http-port"
 )
 
 var (
@@ -118,7 +119,8 @@ func main() {
 		}
 	default:
 		var config *rest.Config
-		config, err := rest.InClusterConfig()
+		// if the kubeconfig fails BuildConfigFromFlags will try in cluster config
+		config, err := clientcmd.BuildConfigFromFlags("", ctlConf.KubeConfig)
 		if err != nil {
 			logger.WithError(err).Fatal("Could not create in cluster config")
 		}
@@ -136,8 +138,8 @@ func main() {
 		}
 
 		var s *sdkserver.SDKServer
-		s, err = sdkserver.NewSDKServer(viper.GetString(gameServerNameEnv),
-			viper.GetString(podNamespaceEnv), kubeClient, agonesClient)
+		s, err = sdkserver.NewSDKServer(ctlConf.GameServerName, ctlConf.PodNamespace,
+			kubeClient, agonesClient)
 		if err != nil {
 			logger.WithError(err).Fatalf("Could not start sidecar")
 		}
@@ -146,7 +148,9 @@ func main() {
 		if err := s.WaitForConnection(ctx); err != nil {
 			logger.WithError(err).Fatalf("Sidecar networking failure")
 		}
-		ctx = s.NewSDKServerContext(ctx)
+		if ctlConf.GracefulTermination {
+			ctx = s.NewSDKServerContext(ctx)
+		}
 		go func() {
 			err := s.Run(ctx)
 			if err != nil {
@@ -265,8 +269,13 @@ func parseEnvFlags() config {
 	viper.SetDefault(addressFlag, "localhost")
 	viper.SetDefault(delayFlag, 0)
 	viper.SetDefault(timeoutFlag, 0)
+	viper.SetDefault(gracefulTerminationFlag, true)
 	viper.SetDefault(grpcPortFlag, defaultGRPCPort)
 	viper.SetDefault(httpPortFlag, defaultHTTPPort)
+	pflag.String(gameServerNameFlag, viper.GetString(gameServerNameFlag),
+		"Optional flag to set GameServer name. Overrides value given from `GAMESERVER_NAME` environment variable.")
+	pflag.String(podNamespaceFlag, viper.GetString(gameServerNameFlag),
+		"Optional flag to set Kubernetes namespace which the GameServer/pod is in. Overrides value given from `POD_NAMESPACE` environment variable.")
 	pflag.Bool(localFlag, viper.GetBool(localFlag),
 		"Set this, or LOCAL env, to 'true' to run this binary in local development mode. Defaults to 'false'")
 	pflag.StringP(fileFlag, "f", viper.GetString(fileFlag), "Set this, or FILE env var to the path of a local yaml or json file that contains your GameServer resoure configuration")
@@ -277,17 +286,22 @@ func parseEnvFlags() config {
 	pflag.Int(timeoutFlag, viper.GetInt(timeoutFlag), "Time of execution (in seconds) before close. Useful for tests")
 	pflag.String(testFlag, viper.GetString(testFlag), "List functions which should be called during the SDK Conformance test run.")
 	pflag.String(testSdkNameFlag, viper.GetString(testSdkNameFlag), "SDK name which is tested by this SDK Conformance test.")
+	pflag.String(kubeconfigFlag, viper.GetString(kubeconfigFlag),
+		"Optional. kubeconfig to run the SDK server out of the cluster.")
+	pflag.Bool(gracefulTerminationFlag, viper.GetBool(gracefulTerminationFlag),
+		"Immediately quits when receiving interrupt instead of waiting for GameServer state to progress to \"Shutdown\".")
 	runtime.FeaturesBindFlags()
 	pflag.Parse()
 
 	viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
+	runtime.Must(viper.BindEnv(gameServerNameFlag))
+	runtime.Must(viper.BindEnv(podNamespaceFlag))
 	runtime.Must(viper.BindEnv(localFlag))
 	runtime.Must(viper.BindEnv(fileFlag))
 	runtime.Must(viper.BindEnv(addressFlag))
 	runtime.Must(viper.BindEnv(testFlag))
 	runtime.Must(viper.BindEnv(testSdkNameFlag))
-	runtime.Must(viper.BindEnv(gameServerNameEnv))
-	runtime.Must(viper.BindEnv(podNamespaceEnv))
+	runtime.Must(viper.BindEnv(kubeconfigFlag))
 	runtime.Must(viper.BindEnv(delayFlag))
 	runtime.Must(viper.BindEnv(timeoutFlag))
 	runtime.Must(viper.BindEnv(grpcPortFlag))
@@ -298,29 +312,37 @@ func parseEnvFlags() config {
 	runtime.Must(runtime.ParseFeaturesFromEnv())
 
 	return config{
-		IsLocal:     viper.GetBool(localFlag),
-		Address:     viper.GetString(addressFlag),
-		LocalFile:   viper.GetString(fileFlag),
-		Delay:       viper.GetInt(delayFlag),
-		Timeout:     viper.GetInt(timeoutFlag),
-		Test:        viper.GetString(testFlag),
-		TestSdkName: viper.GetString(testSdkNameFlag),
-		GRPCPort:    viper.GetInt(grpcPortFlag),
-		HTTPPort:    viper.GetInt(httpPortFlag),
+		GameServerName:      viper.GetString(gameServerNameFlag),
+		PodNamespace:        viper.GetString(podNamespaceFlag),
+		IsLocal:             viper.GetBool(localFlag),
+		Address:             viper.GetString(addressFlag),
+		LocalFile:           viper.GetString(fileFlag),
+		Delay:               viper.GetInt(delayFlag),
+		Timeout:             viper.GetInt(timeoutFlag),
+		Test:                viper.GetString(testFlag),
+		TestSdkName:         viper.GetString(testSdkNameFlag),
+		KubeConfig:          viper.GetString(kubeconfigFlag),
+		GracefulTermination: viper.GetBool(gracefulTerminationFlag),
+		GRPCPort:            viper.GetInt(grpcPortFlag),
+		HTTPPort:            viper.GetInt(httpPortFlag),
 	}
 }
 
 // config is all the configuration for this program
 type config struct {
-	Address     string
-	IsLocal     bool
-	LocalFile   string
-	Delay       int
-	Timeout     int
-	Test        string
-	TestSdkName string
-	GRPCPort    int
-	HTTPPort    int
+	GameServerName      string
+	PodNamespace        string
+	Address             string
+	IsLocal             bool
+	LocalFile           string
+	Delay               int
+	Timeout             int
+	Test                string
+	TestSdkName         string
+	KubeConfig          string
+	GracefulTermination bool
+	GRPCPort            int
+	HTTPPort            int
 }
 
 // healthCheckWrapper ensures that an http 400 response is returned

pkg/gameservers/controller.go

@@ -534,23 +534,29 @@ func (c *Controller) syncDevelopmentGameServer(ctx context.Context, gs *agonesv1
 		return gs, nil
 	}
 
-	// Only move from Creating -> Ready. Other manual state changes are up to the end user.
-	// We also don't want to move from Allocated -> Ready every time someone allocates a GameServer.
-	if gs.Status.State != agonesv1.GameServerStateCreating {
+	// Only move from Creating -> Ready or RequestReady -> Ready.
+	// Shutdown -> Delete will still be handled normally by syncGameServerShutdownState.
+	// Other manual state changes are up to the end user.
+	if gs.Status.State != agonesv1.GameServerStateCreating && gs.Status.State != agonesv1.GameServerStateRequestReady {
 		return gs, nil
 	}
 
 	loggerForGameServer(gs, c.baseLogger).Debug("GS is a development game server and will not be managed by Agones.")
 	gsCopy := gs.DeepCopy()
-	var ports []agonesv1.GameServerStatusPort
-	for _, p := range gs.Spec.Ports {
-		ports = append(ports, p.Status())
-	}
 
 	gsCopy.Status.State = agonesv1.GameServerStateReady
-	gsCopy.Status.Ports = ports
-	gsCopy.Status.Address = devIPAddress
-	gsCopy.Status.NodeName = devIPAddress
+
+	if gs.Status.State == agonesv1.GameServerStateCreating {
+		var ports []agonesv1.GameServerStatusPort
+		for _, p := range gs.Spec.Ports {
+			ports = append(ports, p.Status())
+		}
+
+		gsCopy.Status.Ports = ports
+		gsCopy.Status.Address = devIPAddress
+		gsCopy.Status.NodeName = devIPAddress
+	}
+
 	gs, err := c.gameServerGetter.GameServers(gs.ObjectMeta.Namespace).Update(ctx, gsCopy, metav1.UpdateOptions{})
 	if err != nil {
 		return gs, errors.Wrapf(err, "error updating GameServer %s to %v status", gs.Name, gs.Status)

pkg/gameservers/controller_test.go

@@ -224,6 +224,41 @@ func TestControllerSyncGameServerWithDevIP(t *testing.T) {
 		assert.Equal(t, 1, updateCount, "update reactor should fire once")
 	})
 
+	t.Run("GameServer with ReadyRequest State", func(t *testing.T) {
+		c, mocks := newFakeController()
+
+		updateCount := 0
+
+		gsFixture := templateDevGs.DeepCopy()
+		gsFixture.ApplyDefaults()
+		gsFixture.Status.State = agonesv1.GameServerStateRequestReady
+
+
+		mocks.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			gameServers := &agonesv1.GameServerList{Items: []agonesv1.GameServer{*gsFixture}}
+			return true, gameServers, nil
+		})
+		mocks.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			ua := action.(k8stesting.UpdateAction)
+			gs := ua.GetObject().(*agonesv1.GameServer)
+			updateCount++
+
+			assert.Equal(t, agonesv1.GameServerStateReady, gs.Status.State)
+
+			return true, gs, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(mocks, c.gameServerSynced)
+		defer cancel()
+
+		err := c.portAllocator.Run(ctx)
+		assert.NoError(t, err, "should not error")
+
+		err = c.syncGameServer(ctx, "default/test")
+		assert.NoError(t, err, "should not error")
+		assert.Equal(t, 1, updateCount, "update reactor should fire once")
+	})
+
 	t.Run("Allocated GameServer", func(t *testing.T) {
 		c, mocks := newFakeController()