diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index 82166b14d2..410f8fc19f 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -520,10 +520,13 @@ func (c *Controller) syncDevelopmentGameServer(ctx context.Context, gs *agonesv1 return gs, nil } - if !(gs.Status.State == agonesv1.GameServerStateReady) { - c.loggerForGameServer(gs).Debug("GS is a development game server and will not be managed by Agones.") + // 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 { + return gs, nil } + c.loggerForGameServer(gs).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 { diff --git a/pkg/gameservers/controller_test.go b/pkg/gameservers/controller_test.go index 670a9e576e..eec2460f77 100644 --- a/pkg/gameservers/controller_test.go +++ b/pkg/gameservers/controller_test.go @@ -223,6 +223,33 @@ func TestControllerSyncGameServerWithDevIP(t *testing.T) { assert.Equal(t, 1, updateCount, "update reactor should fire once") }) + t.Run("Allocated GameServer", func(t *testing.T) { + c, mocks := newFakeController() + + fixture := templateDevGs.DeepCopy() + + fixture.ApplyDefaults() + fixture.Status.State = agonesv1.GameServerStateAllocated + + mocks.AgonesClient.AddReactor("list", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + gameServers := &agonesv1.GameServerList{Items: []agonesv1.GameServer{*fixture}} + return true, gameServers, nil + }) + mocks.AgonesClient.AddReactor("update", "gameservers", func(action k8stesting.Action) (bool, runtime.Object, error) { + require.Fail(t, "should not update") + return true, nil, nil + }) + + ctx, cancel := agtesting.StartInformers(mocks, c.gameServerSynced, c.portAllocator.nodeSynced) + defer cancel() + + err := c.portAllocator.syncAll() + require.NoError(t, err) + + err = c.syncGameServer(ctx, "default/test") + require.NoError(t, err) + }) + t.Run("When a GameServer has been deleted, the sync operation should be a noop", func(t *testing.T) { runReconcileDeleteGameServer(t, &agonesv1.GameServer{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e/gameserver_test.go b/test/e2e/gameserver_test.go index f048f78f8b..825dc3b736 100644 --- a/test/e2e/gameserver_test.go +++ b/test/e2e/gameserver_test.go @@ -27,6 +27,7 @@ import ( "time" agonesv1 "agones.dev/agones/pkg/apis/agones/v1" + allocationv1 "agones.dev/agones/pkg/apis/allocation/v1" "agones.dev/agones/pkg/util/runtime" e2eframework "agones.dev/agones/test/e2e/framework" "github.com/sirupsen/logrus" @@ -417,11 +418,14 @@ func TestGameServerUnhealthyAfterReadyCrash(t *testing.T) { func TestDevelopmentGameServerLifecycle(t *testing.T) { t.Parallel() ctx := context.Background() + + labels := map[string]string{"development": "true"} gs := &agonesv1.GameServer{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "udp-server", Namespace: framework.Namespace, Annotations: map[string]string{agonesv1.DevAddressAnnotation: fakeIPAddress}, + Labels: labels, }, Spec: agonesv1.GameServerSpec{ Container: "udp-server", @@ -442,28 +446,58 @@ func TestDevelopmentGameServerLifecycle(t *testing.T) { }, }, } - readyGs, err := framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs) + readyGs, err := framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs.DeepCopy()) if err != nil { t.Fatalf("Could not get a GameServer ready: %v", err) } - - assert.Equal(t, readyGs.Status.State, agonesv1.GameServerStateReady) + require.Equal(t, readyGs.Status.State, agonesv1.GameServerStateReady) // confirm delete works, because if the finalisers don't get removed, this won't work. err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Delete(ctx, readyGs.ObjectMeta.Name, metav1.DeleteOptions{}) - assert.NoError(t, err) + require.NoError(t, err) - err = wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) { + err = wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { _, err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Get(ctx, readyGs.ObjectMeta.Name, metav1.GetOptions{}) - if k8serrors.IsNotFound(err) { return true, nil } return false, err }) + require.NoError(t, err) - assert.NoError(t, err) + // let's make sure we can allocate a dev gameserver + readyGs, err = framework.CreateGameServerAndWaitUntilReady(t, framework.Namespace, gs.DeepCopy()) + require.NoError(t, err) + + gsa := &allocationv1.GameServerAllocation{ + Spec: allocationv1.GameServerAllocationSpec{ + Selectors: []allocationv1.GameServerSelector{{ + LabelSelector: metav1.LabelSelector{MatchLabels: labels}, + }}, + }, + } + gsa, err = framework.AgonesClient.AllocationV1().GameServerAllocations(framework.Namespace).Create(ctx, gsa, metav1.CreateOptions{}) + require.NoError(t, err) + + require.Equal(t, readyGs.ObjectMeta.Name, gsa.Status.GameServerName) + + _, err = framework.WaitForGameServerState(t, readyGs, agonesv1.GameServerStateAllocated, time.Minute) + require.NoError(t, err) + + // Also confirm that delete works for Allocated state, it should be fine but let's be sure. + err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Delete(ctx, readyGs.ObjectMeta.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + + err = wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { + _, err = framework.AgonesClient.AgonesV1().GameServers(framework.Namespace).Get(ctx, readyGs.ObjectMeta.Name, metav1.GetOptions{}) + if k8serrors.IsNotFound(err) { + return true, nil + } + + return false, err + }) + require.NoError(t, err) } func TestGameServerSelfAllocate(t *testing.T) {