fix(healthcontroller): cap room removal in update finish

When rolling update is finishing, we should cap rooms to be deleted to the amount
of rooms leftover in non-active scheduler versions

internal/core/operations/healthcontroller/executor.go

@@ -134,7 +134,7 @@ func (ex *Executor) Execute(ctx context.Context, op *operation.Operation, defini
 	}
 	reportDesiredNumberOfRooms(scheduler.Game, scheduler.Name, desiredNumberOfRooms)
 
-	err = ex.ensureDesiredAmountOfInstances(ctx, op, def, scheduler, logger, len(availableRooms), desiredNumberOfRooms, isRollingUpdating)
+	err = ex.ensureDesiredAmountOfInstances(ctx, op, def, scheduler, logger, availableRooms, desiredNumberOfRooms, isRollingUpdating)
 	if err != nil {
 		logger.Error("cannot ensure desired amount of instances", zap.Error(err))
 		return err
@@ -191,17 +191,18 @@ func (ex *Executor) tryEnsureCorrectRoomsOnStorage(ctx context.Context, op *oper
 	}
 }
 
-func (ex *Executor) ensureDesiredAmountOfInstances(ctx context.Context, op *operation.Operation, def *Definition, scheduler *entities.Scheduler, logger *zap.Logger, actualAmount, desiredAmount int, isRollingUpdating bool) error {
+func (ex *Executor) ensureDesiredAmountOfInstances(ctx context.Context, op *operation.Operation, def *Definition, scheduler *entities.Scheduler, logger *zap.Logger, availableRooms []string, desiredAmount int, isRollingUpdating bool) error {
 	var msgToAppend string
 	var tookAction bool
-
+	actualAmount := len(availableRooms)
 	logger = logger.With(zap.Int("actual", actualAmount), zap.Int("desired", desiredAmount))
 	switch {
 	case actualAmount > desiredAmount: // Need to scale down
 		removeAmount := actualAmount - desiredAmount
 		reason := remove.ScaleDown
 		if isRollingUpdating {
 			reason = remove.RollingUpdateReplace
+			removeAmount = CapRollingUpdateDownscaleAmount(ctx, availableRooms, removeAmount, ex.roomStorage, scheduler, logger)
 		}
 		removeOperation, err := ex.operationManager.CreatePriorityOperation(ctx, op.SchedulerName, &remove.Definition{
 			Amount: removeAmount,
@@ -451,6 +452,32 @@ func GetDesiredNumberOfRooms(
 	return
 }
 
+func CapRollingUpdateDownscaleAmount(ctx context.Context, availableRooms []string, amountToBeDeleted int, roomStorage ports.RoomStorage, scheduler *entities.Scheduler, logger *zap.Logger) (downscaleAmount int) {
+	roomAmountWithInactiveVersion := 0
+	// Get downscale amount based on the number of rooms with inactive versions
+	for _, roomID := range availableRooms {
+		room, err := roomStorage.GetRoom(ctx, scheduler.Name, roomID)
+		if err != nil {
+			logger.Warn("error getting room while downscaling to check delete cap", zap.Error(err))
+			continue
+		}
+		if room.Version != scheduler.Spec.Version {
+			roomAmountWithInactiveVersion += 1
+		}
+		if roomAmountWithInactiveVersion >= amountToBeDeleted {
+			return roomAmountWithInactiveVersion
+		}
+	}
+	logger.Info(
+		"readjusting desired number of rooms",
+		zap.Int("roomAmountWithInactiveVersion", roomAmountWithInactiveVersion),
+		zap.Int("originalDesiredNumberOfRooms", len(availableRooms)-amountToBeDeleted),
+		zap.Int("originalAmountToBeDeleted", amountToBeDeleted),
+		zap.Int("desiredNumberAfterAdjusting", len(availableRooms)-roomAmountWithInactiveVersion),
+	)
+	return roomAmountWithInactiveVersion
+}
+
 // TODO: refactor to SchedulerController execute
 func IsRollingUpdating(
 	// TODO: use context from SchedulerController execute once moved

internal/core/operations/healthcontroller/executor_test.go

@@ -1581,7 +1581,7 @@ func TestCompleteRollingUpdate(t *testing.T) {
 	}
 
 	completeRollingUpdatePlan := map[string][]RollingUpdateExecutionPlan{
-		"Occupancy stable during the update": []RollingUpdateExecutionPlan{
+		"Occupancy stable during the update": {
 			{
 				// Add 25 rooms on newer version
 				currentTotalNumberOfRooms:     100,
@@ -1664,7 +1664,7 @@ func TestCompleteRollingUpdate(t *testing.T) {
 				tookAction:                    false,
 			},
 		},
-		"Occupancy increased during update": []RollingUpdateExecutionPlan{
+		"Occupancy increased during update": {
 			{
 				// Add 25 rooms on newer version
 				currentTotalNumberOfRooms:     100,
@@ -1719,7 +1719,7 @@ func TestCompleteRollingUpdate(t *testing.T) {
 				tookAction:                    false,
 			},
 		},
-		"Occupancy decreased during update": []RollingUpdateExecutionPlan{
+		"Occupancy decreased during update": {
 			{
 				// Add 25 rooms on newer version
 				currentTotalNumberOfRooms:     100,
@@ -1766,7 +1766,7 @@ func TestCompleteRollingUpdate(t *testing.T) {
 				tookAction:                    true,
 			},
 			{
-				// Remove 10 rooms, only 5 left from old versions, so this is the last update iteration
+				// Remove 5 rooms, only 5 left from old versions, so this is the last update iteration
 				currentTotalNumberOfRooms:     50,
 				currentRoomsInActiveVersion:   45,
 				autoscaleDesiredNumberOfRooms: 40,
@@ -1784,7 +1784,7 @@ func TestCompleteRollingUpdate(t *testing.T) {
 				tookAction:                    false,
 			},
 		},
-		"Occupancy stable but only 75% of new rooms become active": []RollingUpdateExecutionPlan{
+		"Occupancy stable but only 75% of new rooms become active": {
 			{
 				// Add 25 rooms on newer version, only 19 will become active
 				currentTotalNumberOfRooms:     100,
@@ -2028,11 +2028,16 @@ func TestCompleteRollingUpdate(t *testing.T) {
 						if cycle.currentRoomsInActiveVersion < cycle.currentTotalNumberOfRooms {
 							reason = remove.RollingUpdateReplace
 						}
+						removeAmount := int(cycle.currentTotalNumberOfRooms - cycle.autoscaleDesiredNumberOfRooms)
+						rommsLeft := cycle.currentTotalNumberOfRooms - cycle.currentRoomsInActiveVersion
+						if rommsLeft < removeAmount {
+							removeAmount = rommsLeft
+						}
 						operationManager.EXPECT().CreatePriorityOperation(
 							gomock.Any(),
 							schedulerV2.Name,
 							&remove.Definition{
-								Amount: int(cycle.currentTotalNumberOfRooms - cycle.autoscaleDesiredNumberOfRooms),
+								Amount: removeAmount,
 								Reason: reason,
 							},
 						).Return(op, nil)