Skip to content

Commit

Permalink
refactor(healthcontroller): only append to history if tookAction
Browse files Browse the repository at this point in the history 
If the operation did not took action, do not append to history. This will avoid bloat
of operation history when listing, simplifying debug.
  • Loading branch information
@hspedro
hspedro committed Aug 23, 2024
1 parent 34688fe commit 785a118
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 18 deletions.
4 changes: 3 additions & 1 deletion internal/core/operations/healthcontroller/executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,10 @@ func (ex *Executor) ensureDesiredAmountOfInstances(ctx context.Context, op *oper
}

logger.Info(msgToAppend)
ex.operationManager.AppendOperationEventToExecutionHistory(ctx, op, msgToAppend)
ex.setTookAction(def, tookAction)
if tookAction {
ex.operationManager.AppendOperationEventToExecutionHistory(ctx, op, msgToAppend)
}
return nil
}

Expand Down
21 changes: 5 additions & 16 deletions internal/core/operations/healthcontroller/executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return([]string{}, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return([]*game_room.Instance{}, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())
},
},
},
Expand Down Expand Up @@ -146,7 +145,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), gomock.Any()).Return(gameRoomIDs, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), gomock.Any()).Return(instances, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), gomock.Any()).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

// Find game room
gameRoom := &game_room.GameRoom{
Expand Down Expand Up @@ -338,7 +336,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), gomock.Any()).Return(gameRoomIDs, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), gomock.Any()).Return(instances, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), gomock.Any()).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

// Find game room
gameRoom := &game_room.GameRoom{
Expand Down Expand Up @@ -394,7 +391,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return(gameRoomIDs, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return(instances, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), genericSchedulerNoAutoscaling.Name).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

// Find game room
gameRoom := &game_room.GameRoom{
Expand Down Expand Up @@ -440,7 +436,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), gomock.Any()).Return(gameRoomIDs, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), gomock.Any()).Return(instances, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), gomock.Any()).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

// Find game room
gameRoom := &game_room.GameRoom{
Expand Down Expand Up @@ -494,7 +489,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetAllRoomIDs(gomock.Any(), gomock.Any()).Return(gameRoomIDs, nil)
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), gomock.Any()).Return(instances, nil)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), gomock.Any()).Return(genericSchedulerNoAutoscaling, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

// Find existent game room
gameRoom := &game_room.GameRoom{
Expand All @@ -504,7 +498,7 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
LastPingAt: time.Now(),
Version: genericSchedulerNoAutoscaling.Spec.Version,
}
roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerNoAutoscaling.Name, gameRoomIDs[0]).Return(gameRoom, nil)
roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerNoAutoscaling.Name, gameRoomIDs[0]).Return(gameRoom, nil).MinTimes(1)

// Find game room
expiredGameRoom := &game_room.GameRoom{
Expand All @@ -514,14 +508,11 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
LastPingAt: time.Now().Add(-time.Minute * 60),
Version: genericSchedulerNoAutoscaling.Spec.Version,
}
roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerNoAutoscaling.Name, gameRoomIDs[1]).Return(expiredGameRoom, nil)

roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerNoAutoscaling.Name, gameRoomIDs[1]).Return(expiredGameRoom, nil).MinTimes(1)
genericSchedulerNoAutoscaling.RoomsReplicas = 1
op := operation.New(genericSchedulerNoAutoscaling.Name, definition.Name(), nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())
operationManager.EXPECT().CreatePriorityOperation(gomock.Any(), genericSchedulerNoAutoscaling.Name, &remove.Definition{RoomsIDs: []string{gameRoomIDs[1]}, Reason: remove.Expired}).Return(op, nil)

roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerNoAutoscaling.Name, gameRoomIDs[0]).Return(gameRoom, nil)
},
},
},
Expand Down Expand Up @@ -563,7 +554,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerAutoscalingEnabled.Name, gameRoomIDs[0]).Return(gameRoom, nil)

autoscaler.EXPECT().CalculateDesiredNumberOfRooms(gomock.Any(), genericSchedulerAutoscalingEnabled).Return(1, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())
},
},
},
Expand Down Expand Up @@ -920,8 +910,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
Version: genericSchedulerAutoscalingEnabled.Spec.Version,
}
roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerAutoscalingEnabled.Name, gameRoomIDs[0]).Return(gameRoom, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any())

roomStorage.EXPECT().GetRoom(gomock.Any(), genericSchedulerAutoscalingEnabled.Name, gameRoomIDs[0]).Return(gameRoom, nil)
},
},
Expand Down Expand Up @@ -1376,7 +1364,6 @@ func TestSchedulerHealthController_Execute(t *testing.T) {
// findAvailableAndExpiredRooms
roomStorage.EXPECT().GetRoom(gomock.Any(), newScheduler.Name, gameRoomIDs[0]).Return(gameRoom1, nil)
roomStorage.EXPECT().GetRoom(gomock.Any(), newScheduler.Name, gameRoomIDs[1]).Return(gameRoom2, nil)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any()).Times(1)

// GetDesiredNumberOfRooms
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), gomock.Any()).Return(newScheduler, nil)
Expand Down Expand Up @@ -2017,7 +2004,9 @@ func TestCompleteRollingUpdate(t *testing.T) {
instanceStorage.EXPECT().GetAllInstances(gomock.Any(), schedulerV2.Name).Return(instances, nil).MinTimes(1)
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), schedulerV2.Name).Return(schedulerV2, nil).MinTimes(1)
autoscaler.EXPECT().CalculateDesiredNumberOfRooms(gomock.Any(), schedulerV2).Return(cycle.autoscaleDesiredNumberOfRooms, nil).MinTimes(1)
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any()).Times(1)
if cycle.tookAction {
operationManager.EXPECT().AppendOperationEventToExecutionHistory(gomock.Any(), gomock.Any(), gomock.Any()).Times(1)
}

if cycle.currentRoomsInActiveVersion != cycle.currentTotalNumberOfRooms {
schedulerStorage.EXPECT().GetSchedulerWithFilter(gomock.Any(), &filters.SchedulerFilter{
Expand Down
2 changes: 1 addition & 1 deletion internal/core/worker/operationexecution/operation_execution_worker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ func TestSchedulerOperationsExecutionLoop(t *testing.T) {
pendingOpsChan := make(chan string)

operationManager.EXPECT().PendingOperationsChan(gomock.Any(), gomock.Any()).Return(pendingOpsChan)
operationManager.EXPECT().CreateOperation(gomock.Any(), scheduler.Name, &healthcontroller.Definition{}).Return(&operation.Operation{}, nil).MaxTimes(5)
operationManager.EXPECT().CreateOperation(gomock.Any(), scheduler.Name, &healthcontroller.Definition{}).Return(&operation.Operation{}, nil).MaxTimes(6)

ctx, cancel := context.WithCancel(context.Background())

Expand Down

0 comments on commit 785a118

Please sign in to comment.