Skip to content

Commit

Permalink
Fix error on room manager during create room method (#542)
Browse files Browse the repository at this point in the history 
### Why

During the new room's creation in the RoomsManager there are a lot of errors like this one:
```
"msg":"failed to process event","service_name":"runtime_watcher","error":"failed to update room instance 
<<INSTANCE_NAME>>: failed to update game room status: failed to get game room: room <<INSTANCE_NAME>> 
not found in scheduler <<SCHEDULER_NAME>>"}
```
This error happens when the room starts before the creation process finishes and the proper game room was created in the room storage.
### What

This PR proposes to change the sequence during the creation process to the room being created before the instance creation, so if the room starts to ping the game room will already be created in the storage and it could update the room status by the ping.
  • Loading branch information
@arthur29
arthur29 authored Oct 6, 2022
1 parent 566faca commit ada676e
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 82 deletions.
20 changes: 18 additions & 2 deletions internal/adapters/runtime/kubernetes/game_room.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,18 @@ package kubernetes

import (
"context"
"fmt"

"github.com/topfreegames/maestro/internal/core/entities"
"github.com/topfreegames/maestro/internal/core/entities/game_room"
"github.com/topfreegames/maestro/internal/core/ports/errors"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilrand "k8s.io/apimachinery/pkg/util/rand"
)

func (k *kubernetes) CreateGameRoomInstance(ctx context.Context, schedulerID string, gameRoomSpec game_room.Spec) (*game_room.Instance, error) {
pod, err := convertGameRoomSpec(schedulerID, gameRoomSpec)
func (k *kubernetes) CreateGameRoomInstance(ctx context.Context, schedulerID, gameRoomName string, gameRoomSpec game_room.Spec) (*game_room.Instance, error) {
pod, err := convertGameRoomSpec(schedulerID, gameRoomName, gameRoomSpec)
if err != nil {
return nil, errors.NewErrInvalidArgument("invalid game room spec: %s", err)
}
Expand Down Expand Up @@ -68,3 +71,16 @@ func (k *kubernetes) DeleteGameRoomInstance(ctx context.Context, gameRoomInstanc

return nil
}

func (k *kubernetes) CreateGameRoomName(ctx context.Context, scheduler entities.Scheduler) (string, error) {
base := scheduler.Name
const (
maxNameLength = 63
randomLength = 5
MaxGeneratedNameLength = maxNameLength - randomLength
)
if len(base) > MaxGeneratedNameLength {
base = base[:MaxGeneratedNameLength]
}
return fmt.Sprintf("%s-%s", base, utilrand.String(randomLength)), nil
}
18 changes: 2 additions & 16 deletions internal/adapters/runtime/kubernetes/game_room_convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ import (
"net"
"strings"

utilrand "k8s.io/apimachinery/pkg/util/rand"

"github.com/topfreegames/maestro/internal/core/entities/game_room"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -64,10 +62,10 @@ var invalidPodWaitingStates = []string{
"RunContainerError",
}

func convertGameRoomSpec(schedulerID string, gameRoomSpec game_room.Spec) (*v1.Pod, error) {
func convertGameRoomSpec(schedulerID, gameRoomName string, gameRoomSpec game_room.Spec) (*v1.Pod, error) {
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: generateName(schedulerID),
Name: gameRoomName,
Namespace: schedulerID,
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand Down Expand Up @@ -372,15 +370,3 @@ func convertPod(pod *v1.Pod, nodeAddress string) (*game_room.Instance, error) {
ResourceVersion: pod.ResourceVersion,
}, nil
}

func generateName(base string) string {
const (
maxNameLength = 63
randomLength = 5
MaxGeneratedNameLength = maxNameLength - randomLength
)
if len(base) > MaxGeneratedNameLength {
base = base[:MaxGeneratedNameLength]
}
return fmt.Sprintf("%s-%s", base, utilrand.String(randomLength))
}
14 changes: 13 additions & 1 deletion internal/adapters/runtime/kubernetes/game_room_convert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,17 +372,20 @@ func TestConvertContainer(t *testing.T) {
func TestConvertGameSpec(t *testing.T) {
cases := map[string]struct {
schedulerID string
roomName string
gameSpec game_room.Spec
expectedPod v1.Pod
withError bool
}{
"without containers": {
schedulerID: "sample",
roomName: "roomName",
gameSpec: game_room.Spec{
Version: "version",
},
expectedPod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "roomName",
Namespace: "sample",
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand All @@ -394,6 +397,7 @@ func TestConvertGameSpec(t *testing.T) {
},
"with containers": {
schedulerID: "sample",
roomName: "roomName",
gameSpec: game_room.Spec{
Version: "version",
Containers: []game_room.Container{
Expand All @@ -403,6 +407,7 @@ func TestConvertGameSpec(t *testing.T) {
},
expectedPod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "roomName",
Namespace: "sample",
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand All @@ -420,12 +425,14 @@ func TestConvertGameSpec(t *testing.T) {
},
"with toleration": {
schedulerID: "sample",
roomName: "roomName",
gameSpec: game_room.Spec{
Version: "version",
Toleration: "some-toleration",
},
expectedPod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "roomName",
Namespace: "sample",
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand All @@ -442,12 +449,14 @@ func TestConvertGameSpec(t *testing.T) {
},
"with affinity": {
schedulerID: "sample",
roomName: "roomName",
gameSpec: game_room.Spec{
Version: "version",
Affinity: "sample-affinity",
},
expectedPod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "roomName",
Namespace: "sample",
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand All @@ -462,12 +471,14 @@ func TestConvertGameSpec(t *testing.T) {
},
"with termination grace period": {
schedulerID: "sample",
roomName: "roomName",
gameSpec: game_room.Spec{
Version: "version",
TerminationGracePeriod: 10 * time.Second,
},
expectedPod: v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "roomName",
Namespace: "sample",
Labels: map[string]string{
maestroLabelKey: maestroLabelValue,
Expand All @@ -484,14 +495,15 @@ func TestConvertGameSpec(t *testing.T) {

for name, test := range cases {
t.Run(name, func(t *testing.T) {
res, err := convertGameRoomSpec(test.schedulerID, test.gameSpec)
res, err := convertGameRoomSpec(test.schedulerID, test.roomName, test.gameSpec)
if test.withError {
require.Error(t, err)
return
}

require.NoError(t, err)
require.Equal(t, test.expectedPod.ObjectMeta.Labels, res.ObjectMeta.Labels)
require.Equal(t, test.expectedPod.ObjectMeta.Name, res.ObjectMeta.Name)
require.Equal(t, test.expectedPod.ObjectMeta.Namespace, res.ObjectMeta.Namespace)
require.Equal(t, len(test.expectedPod.Spec.Containers), len(res.Spec.Containers))
require.Equal(t, len(test.expectedPod.Spec.Tolerations), len(res.Spec.Tolerations))
Expand Down
42 changes: 39 additions & 3 deletions internal/adapters/runtime/kubernetes/game_room_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/topfreegames/maestro/internal/core/entities"
"github.com/topfreegames/maestro/internal/core/entities/game_room"
Expand All @@ -41,6 +42,7 @@ import (
func TestGameRoomCreation(t *testing.T) {
t.Parallel()
ctx := context.Background()
gameRoomName := "some-game-room-name"
client := test.GetKubernetesClientSet(t, kubernetesContainer)
kubernetesRuntime := New(client)

Expand All @@ -59,7 +61,7 @@ func TestGameRoomCreation(t *testing.T) {
},
},
}
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

pods, err := client.CoreV1().Pods(scheduler.Name).List(ctx, metav1.ListOptions{})
Expand All @@ -81,7 +83,7 @@ func TestGameRoomCreation(t *testing.T) {
// no containers, meaning it will fail (because it can be a pod
// without containers).
gameRoomSpec := game_room.Spec{}
_, err = kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
_, err = kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.Error(t, err)
require.ErrorIs(t, err, errors.ErrInvalidArgument)

Expand All @@ -95,6 +97,7 @@ func TestGameRoomDeletion(t *testing.T) {
t.Parallel()
ctx := context.Background()
client := test.GetKubernetesClientSet(t, kubernetesContainer)
gameRoomName := "some-game-room"
kubernetesRuntime := New(client)

t.Run("successfully delete a room", func(t *testing.T) {
Expand All @@ -112,7 +115,7 @@ func TestGameRoomDeletion(t *testing.T) {
},
},
}
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

pods, err := client.CoreV1().Pods(scheduler.Name).List(ctx, metav1.ListOptions{})
Expand Down Expand Up @@ -147,3 +150,36 @@ func TestGameRoomDeletion(t *testing.T) {
require.ErrorIs(t, err, errors.ErrNotFound)
})
}

func TestCreateGameRoomName(t *testing.T) {
t.Parallel()
ctx := context.Background()
client := test.GetKubernetesClientSet(t, kubernetesContainer)
kubernetesRuntime := New(client)

t.Run("When scheduler name is greater than max name length minus randomLength", func(t *testing.T) {
t.Run("return the name with randomLength", func(t *testing.T) {
name, err := kubernetesRuntime.CreateGameRoomName(
ctx,
entities.Scheduler{
Name: "lllllllllllllllllllllllllllllllllllllllllllllllllllllllllll",
},
)
assert.NoError(t, err)
assert.Len(t, name, 64)
})
})
t.Run("To any other scheduler name size lower than max name length minus randomLength", func(t *testing.T) {
t.Run("return the name plus 5 caracters random", func(t *testing.T) {
schedulerName := "lllllllllllllllllllllllllllllll"
name, err := kubernetesRuntime.CreateGameRoomName(
ctx,
entities.Scheduler{
Name: schedulerName,
},
)
assert.NoError(t, err)
assert.Len(t, name, len(schedulerName)+6)
})
})
}
9 changes: 5 additions & 4 deletions internal/adapters/runtime/kubernetes/game_room_watcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (

func TestGameRoomsWatch(t *testing.T) {
t.Parallel()
gameRoomName := "some-game-room"

t.Run("watch pod addition", func(t *testing.T) {
t.Parallel()
Expand All @@ -63,7 +64,7 @@ func TestGameRoomsWatch(t *testing.T) {
},
}

instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

require.Eventually(t, func() bool {
Expand Down Expand Up @@ -122,7 +123,7 @@ func TestGameRoomsWatch(t *testing.T) {
},
}

instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

require.Eventually(t, func() bool {
Expand Down Expand Up @@ -177,7 +178,7 @@ func TestGameRoomsWatch(t *testing.T) {
},
}

instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

require.Eventually(t, func() bool {
Expand Down Expand Up @@ -231,7 +232,7 @@ func TestGameRoomsWatch(t *testing.T) {
},
}

instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomSpec)
instance, err := kubernetesRuntime.CreateGameRoomInstance(ctx, scheduler.Name, gameRoomName, gameRoomSpec)
require.NoError(t, err)

err = kubernetesRuntime.DeleteGameRoomInstance(ctx, instance)
Expand Down
23 changes: 19 additions & 4 deletions internal/core/ports/mock/runtime_mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion internal/core/ports/runtime.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ type Runtime interface {
DeleteScheduler(ctx context.Context, scheduler *entities.Scheduler) error
// CreateGameRoomInstance Creates a game room instance on the runtime using
// the specification provided.
CreateGameRoomInstance(ctx context.Context, schedulerId string, spec game_room.Spec) (*game_room.Instance, error)
CreateGameRoomInstance(ctx context.Context, schedulerId, gameRoomName string, spec game_room.Spec) (*game_room.Instance, error)
// DeleteGameRoomInstance Deletes a game room instance on the runtime.
DeleteGameRoomInstance(ctx context.Context, gameRoomInstance *game_room.Instance) error
// WatchGameRoomInstances Watches for changes of a scheduler game room instances.
WatchGameRoomInstances(ctx context.Context, scheduler *entities.Scheduler) (RuntimeWatcher, error)
// Create a name to the room.
CreateGameRoomName(ctx context.Context, scheduler entities.Scheduler) (string, error)
}

// RuntimeWatcher defines a process of watcher, it will have a chan with the
Expand Down
Loading

0 comments on commit ada676e

Please sign in to comment.