Skip to content

Commit

Permalink
switch active version does not wait to create room (#445)
Browse files Browse the repository at this point in the history 
* switch active version does not wait to create room

* fix tests

* new test case for fail on delete room

* fix log

* fix rooms being replaced

* fix lint

Co-authored-by: “Caio <“[email protected]”>
  • Loading branch information
@caiordjesus
caiordjesus and “Caio authored May 25, 2022
1 parent 1b9a61c commit 1a6f172
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 24 deletions.
10 changes: 6 additions & 4 deletions internal/core/operations/switch_active_version/switch_active_version_executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,9 @@ func (ex *SwitchActiveVersionExecutor) replaceRoom(logger *zap.Logger, roomsChan
return nil
}

gameRoom, _, err := roomManager.CreateRoomAndWaitForReadiness(ctx, scheduler, false)
gameRoom, _, err := roomManager.CreateRoom(ctx, scheduler, false)
if err != nil {
logger.Error("error creating room", zap.Error(err))
ex.roomsBeingReplaced.Delete(room.ID)
return err
}

err = roomManager.DeleteRoomAndWaitForRoomTerminating(ctx, room)
Expand All @@ -224,8 +222,12 @@ func (ex *SwitchActiveVersionExecutor) replaceRoom(logger *zap.Logger, roomsChan
return err
}

logger.Sugar().Debugf("replaced room \"%s\" with \"%s\"", room.ID, gameRoom.ID)
ex.roomsBeingReplaced.Delete(room.ID)
if gameRoom == nil {
return nil
}

logger.Sugar().Debugf("replaced room \"%s\" with \"%s\"", room.ID, gameRoom.ID)
ex.appendToNewCreatedRooms(scheduler.Name, gameRoom)
}
}
Expand Down
113 changes: 93 additions & 20 deletions internal/core/operations/switch_active_version/switch_active_version_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestSwitchActiveVersionOperation_Execute(t *testing.T) {
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
}
mocks.roomManager.EXPECT().CreateRoomAndWaitForReadiness(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
}
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil).MaxTimes(len(append(gameRoomListCycle1, gameRoomListCycle2...)))

Expand Down Expand Up @@ -168,21 +168,7 @@ func TestSwitchActiveVersionOperation_Execute(t *testing.T) {
require.Nil(t, execErr)
})

t.Run("should fail - Can't update scheduler (switch active version on database)", func(t *testing.T) {
noReplaceDefinition := &switch_active_version.SwitchActiveVersionDefinition{NewActiveVersion: newMinorScheduler.Spec.Version}
mocks := newMockRoomAndSchedulerManager(mockCtrl)

mocks.schedulerManager.EXPECT().GetActiveScheduler(gomock.Any(), activeScheduler.Name).Return(activeScheduler, nil)
mocks.schedulerManager.EXPECT().GetSchedulerByVersion(gomock.Any(), newMinorScheduler.Name, newMinorScheduler.Spec.Version).Return(newMinorScheduler, nil)
mocks.schedulerManager.EXPECT().UpdateScheduler(gomock.Any(), gomock.Any()).Return(errors.New("error"))

executor := switch_active_version.NewExecutor(mocks.roomManager, mocks.schedulerManager)
execErr := executor.Execute(context.Background(), &operation.Operation{SchedulerName: newMinorScheduler.Name}, noReplaceDefinition)
require.NotNil(t, execErr)
require.Equal(t, operations.ErrKindUnexpected, execErr.Kind())
})

t.Run("should fail - Can't create room", func(t *testing.T) {
t.Run("should succeed - Can't create room", func(t *testing.T) {
definition := &switch_active_version.SwitchActiveVersionDefinition{NewActiveVersion: newMajorScheduler.Spec.Version}
mocks := newMockRoomAndSchedulerManager(mockCtrl)

Expand All @@ -204,10 +190,26 @@ func TestSwitchActiveVersionOperation_Execute(t *testing.T) {
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoomListCycle1, nil)
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]*game_room.GameRoom{}, nil).MaxTimes(1)

mocks.roomManager.EXPECT().CreateRoomAndWaitForReadiness(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, errors.New("error")).MaxTimes(maxSurge)
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, errors.New("error")).MaxTimes(maxSurge)
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil).MaxTimes(maxSurge)

mocks.schedulerManager.EXPECT().UpdateScheduler(gomock.Any(), gomock.Any()).Return(nil)

executor := switch_active_version.NewExecutor(mocks.roomManager, mocks.schedulerManager)
execErr := executor.Execute(context.Background(), &operation.Operation{SchedulerName: newMajorScheduler.Name}, definition)
require.Nil(t, execErr)
})

t.Run("should fail - Can't update scheduler (switch active version on database)", func(t *testing.T) {
noReplaceDefinition := &switch_active_version.SwitchActiveVersionDefinition{NewActiveVersion: newMinorScheduler.Spec.Version}
mocks := newMockRoomAndSchedulerManager(mockCtrl)

mocks.schedulerManager.EXPECT().GetActiveScheduler(gomock.Any(), activeScheduler.Name).Return(activeScheduler, nil)
mocks.schedulerManager.EXPECT().GetSchedulerByVersion(gomock.Any(), newMinorScheduler.Name, newMinorScheduler.Spec.Version).Return(newMinorScheduler, nil)
mocks.schedulerManager.EXPECT().UpdateScheduler(gomock.Any(), gomock.Any()).Return(errors.New("error"))

executor := switch_active_version.NewExecutor(mocks.roomManager, mocks.schedulerManager)
execErr := executor.Execute(context.Background(), &operation.Operation{SchedulerName: newMinorScheduler.Name}, noReplaceDefinition)
require.NotNil(t, execErr)
require.Equal(t, operations.ErrKindUnexpected, execErr.Kind())
})
Expand All @@ -234,7 +236,7 @@ func TestSwitchActiveVersionOperation_Execute(t *testing.T) {
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoomListCycle1, nil)
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]*game_room.GameRoom{}, nil).MaxTimes(1)

mocks.roomManager.EXPECT().CreateRoomAndWaitForReadiness(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, nil).MaxTimes(maxSurge)
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, nil).MaxTimes(maxSurge)
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(errors.New("error")).MaxTimes(maxSurge)

executor := switch_active_version.NewExecutor(mocks.roomManager, mocks.schedulerManager)
Expand Down Expand Up @@ -368,7 +370,7 @@ func TestSwitchActiveVersionOperation_Rollback(t *testing.T) {
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
}
mocks.roomManager.EXPECT().CreateRoomAndWaitForReadiness(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil)
}

Expand All @@ -393,6 +395,77 @@ func TestSwitchActiveVersionOperation_Rollback(t *testing.T) {
require.NoError(t, err)
})

t.Run("should succeed - a create room fail, then a delete room fails and triggers rollback", func(t *testing.T) {

mocks.schedulerManager.EXPECT().GetActiveScheduler(gomock.Any(), activeScheduler.Name).Return(activeScheduler, nil)
mocks.schedulerManager.EXPECT().GetSchedulerByVersion(gomock.Any(), newMajorScheduler.Name, newMajorScheduler.Spec.Version).Return(newMajorScheduler, nil)
mocks.roomManager.EXPECT().SchedulerMaxSurge(gomock.Any(), gomock.Any()).Return(3, nil)

var gameRoomListCycle1 []*game_room.GameRoom
var gameRoomListCycle2 []*game_room.GameRoom
var gameRoomListCycle3 []*game_room.GameRoom
for i := 0; i < maxSurge; i++ {
gameRoomListCycle1 = append(gameRoomListCycle1, &game_room.GameRoom{
ID: fmt.Sprintf("room-%v", i),
SchedulerID: newMajorScheduler.Name,
Version: activeScheduler.Spec.Version,
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
})
}
for i := maxSurge; i < maxSurge*2; i++ {
gameRoomListCycle2 = append(gameRoomListCycle2, &game_room.GameRoom{
ID: fmt.Sprintf("room-%v", i),
SchedulerID: newMajorScheduler.Name,
Version: activeScheduler.Spec.Version,
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
})
}
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoomListCycle1, nil)
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoomListCycle2, nil)
mocks.roomManager.EXPECT().ListRoomsWithDeletionPriority(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoomListCycle3, nil)

allRooms := append(gameRoomListCycle1, gameRoomListCycle2...)
for i := range allRooms {
if i == len(allRooms)-1 {
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil, errors.New("error"))
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(errors.New("error"))
continue
}
gameRoom := &game_room.GameRoom{
ID: fmt.Sprintf("new-room-%v", i),
SchedulerID: newMajorScheduler.Name,
Version: activeScheduler.Spec.Version,
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
}
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil)
}

executor := switch_active_version.NewExecutor(mocks.roomManager, mocks.schedulerManager)
op := &operation.Operation{
ID: "op",
DefinitionName: definition.Name(),
SchedulerName: newMajorScheduler.Name,
CreatedAt: time.Now(),
}
execErr := executor.Execute(context.Background(), op, definition)
require.NotNil(t, execErr)
require.Equal(t, operations.ErrKindUnexpected, execErr.Kind())

for i := range allRooms {
if i == len(allRooms)-1 {
continue
}
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil)
}

err = executor.Rollback(context.Background(), op, definition, nil)
require.NoError(t, err)
})

t.Run("should fail - error deleting rooms", func(t *testing.T) {
mocks.schedulerManager.EXPECT().GetActiveScheduler(gomock.Any(), activeScheduler.Name).Return(activeScheduler, nil)
mocks.schedulerManager.EXPECT().GetSchedulerByVersion(gomock.Any(), newMajorScheduler.Name, newMajorScheduler.Spec.Version).Return(newMajorScheduler, nil)
Expand Down Expand Up @@ -431,7 +504,7 @@ func TestSwitchActiveVersionOperation_Rollback(t *testing.T) {
Status: game_room.GameStatusReady,
LastPingAt: time.Now(),
}
mocks.roomManager.EXPECT().CreateRoomAndWaitForReadiness(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().CreateRoom(gomock.Any(), gomock.Any(), gomock.Any()).Return(gameRoom, nil, nil)
mocks.roomManager.EXPECT().DeleteRoomAndWaitForRoomTerminating(gomock.Any(), gomock.Any()).Return(nil)
}

Expand Down

0 comments on commit 1a6f172

Please sign in to comment.