Skip to content

Commit f3e1c60

Browse files
ffelipelimaoffelipelimao
authored
Revert "Prevent concurrent scheduler editions (#692)"
This reverts commit 605717e.
1 parent 605717e commit f3e1c60

File tree

4 files changed

+1
-397
lines changed

4 files changed

+1
-397
lines changed

‎internal/api/handlers/schedulers_handler.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,6 @@ func (h *SchedulersHandler) NewSchedulerVersion(ctx context.Context, request *ap
176176
if errors.Is(err, portsErrors.ErrNotFound) {
177177
return nil, status.Error(codes.NotFound, err.Error())
178178
}
179-
if errors.Is(err, portsErrors.ErrConflict) {
180-
return nil, status.Error(codes.FailedPrecondition, err.Error())
181-
}
182179
return nil, status.Error(codes.Unknown, err.Error())
183180
}
184181
handlerLogger.Debug("finish handling new scheduler version request")
@@ -205,9 +202,6 @@ func (h *SchedulersHandler) PatchScheduler(ctx context.Context, request *api.Pat
205202
if errors.Is(err, portsErrors.ErrInvalidArgument) {
206203
return nil, status.Error(codes.InvalidArgument, err.Error())
207204
}
208-
if errors.Is(err, portsErrors.ErrConflict) {
209-
return nil, status.Error(codes.FailedPrecondition, err.Error())
210-
}
211205

212206
return nil, status.Error(codes.Unknown, err.Error())
213207
}
@@ -223,9 +217,6 @@ func (h *SchedulersHandler) SwitchActiveVersion(ctx context.Context, request *ap
223217

224218
if err != nil {
225219
handlerLogger.Error(fmt.Sprintf("error switching active version %s", request.GetVersion()), zap.Error(err))
226-
if errors.Is(err, portsErrors.ErrConflict) {
227-
return nil, status.Error(codes.FailedPrecondition, err.Error())
228-
}
229220
return nil, status.Error(codes.Unknown, err.Error())
230221
}
231222

‎internal/api/handlers/schedulers_handler_test.go‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -694,8 +694,6 @@ func TestNewSchedulerVersion(t *testing.T) {
694694
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
695695
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
696696

697-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
698-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
699697
operationManager.EXPECT().CreateOperation(gomock.Any(), "scheduler-name-1", gomock.Any()).Return(&operation.Operation{ID: "id-1"}, nil)
700698
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), "scheduler-name-1").Return(currentScheduler, nil)
701699

@@ -734,8 +732,6 @@ func TestNewSchedulerVersion(t *testing.T) {
734732
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
735733
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
736734

737-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
738-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
739735
operationManager.EXPECT().CreateOperation(gomock.Any(), "scheduler-name-1", gomock.Any()).Return(&operation.Operation{ID: "id-1"}, nil)
740736
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), "scheduler-name-1").DoAndReturn(func(_ context.Context, _ string) (*entities.Scheduler, error) {
741737
dbScheduler := scheduler.NewDBScheduler(currentScheduler)
@@ -767,13 +763,9 @@ func TestNewSchedulerVersion(t *testing.T) {
767763
t.Run("fails when scheduler does not exists", func(t *testing.T) {
768764
mockCtrl := gomock.NewController(t)
769765
schedulerStorage := mockports.NewMockSchedulerStorage(mockCtrl)
770-
operationManager := mock.NewMockOperationManager(mockCtrl)
771766
roomStorage := mockports.NewMockRoomStorage(mockCtrl)
772-
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
773-
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
767+
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, nil, nil, roomStorage)
774768

775-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
776-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
777769
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), "scheduler-name-1").Return(nil, errors.NewErrNotFound("err"))
778770

779771
mux := runtime.NewServeMux()
@@ -807,8 +799,6 @@ func TestNewSchedulerVersion(t *testing.T) {
807799
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
808800
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
809801

810-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
811-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
812802
operationManager.EXPECT().CreateOperation(gomock.Any(), "scheduler-name-1", gomock.Any()).Return(nil, errors.NewErrUnexpected("storage offline"))
813803
schedulerStorage.EXPECT().GetScheduler(gomock.Any(), "scheduler-name-1").Return(currentScheduler, nil)
814804

@@ -845,8 +835,6 @@ func TestSwitchActiveVersion(t *testing.T) {
845835
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
846836
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
847837

848-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
849-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
850838
operationManager.EXPECT().CreateOperation(gomock.Any(), "scheduler-name-1", gomock.Any()).Return(&operation.Operation{ID: "id-1"}, nil)
851839

852840
mux := runtime.NewServeMux()
@@ -876,8 +864,6 @@ func TestSwitchActiveVersion(t *testing.T) {
876864
schedulerCache := mockports.NewMockSchedulerCache(mockCtrl)
877865
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, schedulerCache, operationManager, roomStorage)
878866

879-
operationManager.EXPECT().ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
880-
operationManager.EXPECT().ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").Return([]*operation.Operation{}, nil)
881867
operationManager.EXPECT().CreateOperation(gomock.Any(), "scheduler-name-1", gomock.Any()).Return(nil, errors.NewErrUnexpected("internal error"))
882868

883869
mux := runtime.NewServeMux()
@@ -1182,16 +1168,6 @@ func TestPatchScheduler(t *testing.T) {
11821168
operationManager := mock.NewMockOperationManager(mockCtrl)
11831169
schedulerManager := schedulers.NewSchedulerManager(schedulerStorage, nil, operationManager, nil)
11841170

1185-
operationManager.EXPECT().
1186-
ListSchedulerPendingOperations(gomock.Any(), "scheduler-name-1").
1187-
Return([]*operation.Operation{}, nil).
1188-
AnyTimes()
1189-
1190-
operationManager.EXPECT().
1191-
ListSchedulerActiveOperations(gomock.Any(), "scheduler-name-1").
1192-
Return([]*operation.Operation{}, nil).
1193-
AnyTimes()
1194-
11951171
schedulerStorage.EXPECT().
11961172
GetScheduler(gomock.Any(), "scheduler-name-1").
11971173
Return(testCase.Mocks.GetSchedulerReturn, testCase.Mocks.GetSchedulerError).

‎internal/core/services/schedulers/scheduler_manager.go‎

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"context"
2727
"errors"
2828
"fmt"
29-
"slices"
3029

3130
newversion "github.com/topfreegames/maestro/internal/core/operations/schedulers/newversion"
3231
"github.com/topfreegames/maestro/internal/core/operations/schedulers/switchversion"
@@ -145,14 +144,6 @@ func (s *SchedulerManager) CreateNewSchedulerVersionAndEnqueueSwitchVersion(ctx
145144
}
146145

147146
func (s *SchedulerManager) PatchSchedulerAndCreateNewSchedulerVersionOperation(ctx context.Context, schedulerName string, patchMap map[string]interface{}) (*operation.Operation, error) {
148-
ongoing, err := s.hasOngoingVersionOperation(ctx, schedulerName)
149-
if err != nil {
150-
return nil, portsErrors.NewErrUnexpected("failed to check for ongoing version operations: %s", err.Error())
151-
}
152-
if ongoing != "" {
153-
return nil, portsErrors.NewErrConflict("cannot patch scheduler: there is already an ongoing version operation (id: %s) for scheduler %s", ongoing, schedulerName)
154-
}
155-
156147
scheduler, err := s.schedulerStorage.GetScheduler(ctx, schedulerName)
157148
if err != nil {
158149
if errors.Is(err, portsErrors.ErrNotFound) {
@@ -197,14 +188,6 @@ func (s *SchedulerManager) GetSchedulerVersions(ctx context.Context, schedulerNa
197188
}
198189

199190
func (s *SchedulerManager) EnqueueNewSchedulerVersionOperation(ctx context.Context, scheduler *entities.Scheduler) (*operation.Operation, error) {
200-
ongoing, err := s.hasOngoingVersionOperation(ctx, scheduler.Name)
201-
if err != nil {
202-
return nil, fmt.Errorf("failed to check for ongoing version operations: %w", err)
203-
}
204-
if ongoing != "" {
205-
return nil, portsErrors.NewErrConflict("cannot create new scheduler version: there is already an ongoing version operation (id: %s) for scheduler %s", ongoing, scheduler.Name)
206-
}
207-
208191
currentScheduler, err := s.schedulerStorage.GetScheduler(ctx, scheduler.Name)
209192
if err != nil {
210193
return nil, fmt.Errorf("no scheduler found, can not create new version for inexistent scheduler: %w", err)
@@ -227,14 +210,6 @@ func (s *SchedulerManager) EnqueueNewSchedulerVersionOperation(ctx context.Conte
227210
}
228211

229212
func (s *SchedulerManager) EnqueueSwitchActiveVersionOperation(ctx context.Context, schedulerName, newVersion string) (*operation.Operation, error) {
230-
ongoing, err := s.hasOngoingVersionOperation(ctx, schedulerName)
231-
if err != nil {
232-
return nil, fmt.Errorf("failed to check for ongoing version operations: %w", err)
233-
}
234-
if ongoing != "" {
235-
return nil, portsErrors.NewErrConflict("cannot switch active version: there is already an ongoing version operation (id: %s) for scheduler %s", ongoing, schedulerName)
236-
}
237-
238213
opDef := &switchversion.Definition{NewActiveVersion: newVersion}
239214
op, err := s.operationManager.CreateOperation(ctx, schedulerName, opDef)
240215
if err != nil {
@@ -353,37 +328,3 @@ func (s *SchedulerManager) newSchedulerInfo(ctx context.Context, scheduler *enti
353328
entities.WithAutoscalingInfo(scheduler.Autoscaling),
354329
), nil
355330
}
356-
357-
func (s *SchedulerManager) hasOngoingVersionOperation(ctx context.Context, schedulerName string) (string, error) {
358-
pending, err := s.operationManager.ListSchedulerPendingOperations(ctx, schedulerName)
359-
if err != nil {
360-
return "", fmt.Errorf("failed to list pending operations: %w", err)
361-
}
362-
363-
for _, op := range pending {
364-
if slices.Contains([]string{newversion.OperationName, switchversion.OperationName}, op.DefinitionName) {
365-
s.logger.Debug("found pending version operation",
366-
zap.String("operationID", op.ID),
367-
zap.String("operationType", op.DefinitionName),
368-
zap.String("schedulerName", schedulerName))
369-
return op.ID, nil
370-
}
371-
}
372-
373-
active, err := s.operationManager.ListSchedulerActiveOperations(ctx, schedulerName)
374-
if err != nil {
375-
return "", fmt.Errorf("failed to list active operations: %w", err)
376-
}
377-
378-
for _, op := range active {
379-
if slices.Contains([]string{newversion.OperationName, switchversion.OperationName}, op.DefinitionName) {
380-
s.logger.Debug("found active version operation",
381-
zap.String("operationID", op.ID),
382-
zap.String("operationType", op.DefinitionName),
383-
zap.String("schedulerName", schedulerName))
384-
return op.ID, nil
385-
}
386-
}
387-
388-
return "", nil
389-
}

0 commit comments

Comments
 (0)