Skip to content

Commit

Permalink
fix(room_manager): only sort rooms removal if major version diff
Browse files Browse the repository at this point in the history
We only want to apply sorting by version if the room has a major version of
difference compared to the active scheduler version. This prevents minor
updates forcing the sort, which would cause occupied rooms to be deleted
first (from a prior minor version) when we don't want that behavior. Thus,
check if it has a major version diff between room and active scheduler
  • Loading branch information
hspedro committed Oct 8, 2024
1 parent 4fe0422 commit 57f54aa
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 3 deletions.
24 changes: 24 additions & 0 deletions internal/core/entities/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ package entities

import (
"errors"
"strconv"
"strings"
"time"

"github.com/topfreegames/maestro/internal/core/entities/autoscaling"
Expand Down Expand Up @@ -202,3 +204,25 @@ func (s *Scheduler) HasValidPortRangeConfiguration() error {

return nil
}

func (s *Scheduler) HasMajorVersionDifference(otherVersion string) bool {
return extractMajorVersion(otherVersion) != extractMajorVersion(s.Spec.Version)
}

func extractMajorVersion(version string) int {
if version == "" {
return -1
}

parts := strings.Split(version[1:], ".")
if len(parts) < 1 {
return -1
}

majorVersion, err := strconv.Atoi(parts[0])
if err != nil {
return -1
}

return majorVersion
}
32 changes: 32 additions & 0 deletions internal/core/entities/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,3 +399,35 @@ func TestHasValidPortRangeConfiguration(t *testing.T) {
})
}
}

func TestHasMajorVersionDifference(t *testing.T) {
s := &entities.Scheduler{}
s.Spec.Version = "v10.5.0"

testCases := []struct {
name string
otherVersion string
expected bool
}{
{"Major version difference", "v11.0.0", true},
{"No major version difference", "v10.6.0", false},
{"Empty otherVersion", "", true},
{"otherVersion without v prefix", "10.0.0", true},
{"otherVersion not semantic", "version10", true},
{"otherVersion semantic but not convertible to integer", "vX.5.0", true},
{"Both versions are the same", "v10.5.0", false},
{"Difference only in bugfix version", "v10.5.1", false},
{"Scheduler.Spec.Version without v prefix", "v11.0.0", true},
{"Scheduler.Spec.Version not semantic", "v11.0.0", true},
{"Scheduler.Spec.Version semantic but not convertible to integer", "v11.0.0", true},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := s.HasMajorVersionDifference(tc.otherVersion)
if result != tc.expected {
t.Errorf("Test %s failed: expected %v, got %v", tc.name, tc.expected, result)
}
})
}
}
2 changes: 1 addition & 1 deletion internal/core/services/rooms/room_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func (m *RoomManager) ListRoomsWithDeletionPriority(ctx context.Context, activeS
}

isRoomActive := room.Status == game_room.GameStatusOccupied || room.Status == game_room.GameStatusReady || room.Status == game_room.GameStatusPending
if isRoomActive && room.Version == activeScheduler.Spec.Version {
if isRoomActive && activeScheduler.HasMajorVersionDifference(room.Version) {
activeVersionRoomPool = append(activeVersionRoomPool, room)
} else {
toDeleteRooms = append(toDeleteRooms, room)
Expand Down
7 changes: 5 additions & 2 deletions internal/core/services/rooms/room_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,8 +596,11 @@ func TestRoomManager_ListRoomsWithDeletionPriority(t *testing.T) {
rooms, err := roomManager.ListRoomsWithDeletionPriority(ctx, scheduler, 2)
require.NoError(t, err)

availableRooms = append(availableRooms, &game_room.GameRoom{ID: notFoundRoomID, SchedulerID: scheduler.Name, Status: game_room.GameStatusError})
require.Equal(t, rooms, availableRooms)
expectedRooms := []*game_room.GameRoom{
{ID: notFoundRoomID, SchedulerID: scheduler.Name, Status: game_room.GameStatusError},
{ID: "first-room", SchedulerID: scheduler.Name, Status: game_room.GameStatusReady},
}
require.Equal(t, rooms, expectedRooms)
})

t.Run("when error happens while fetch a room it returns error", func(t *testing.T) {
Expand Down

0 comments on commit 57f54aa

Please sign in to comment.