From 21ff03aad38f2ccf3bcce3efceead7c0c0efd584 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 19 Aug 2024 13:14:17 +0200 Subject: [PATCH] orchestrator/updates: fixes incorrect active condition lookup This resolves a bug where the following condition could not be queued, because store.GetActiveCondition() was incorrectly passed a ConditionID in the serverID parameter. --- internal/orchestrator/updates.go | 2 +- internal/orchestrator/updates_test.go | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/orchestrator/updates.go b/internal/orchestrator/updates.go index e3e469d4..47f28edd 100644 --- a/internal/orchestrator/updates.go +++ b/internal/orchestrator/updates.go @@ -606,7 +606,7 @@ func (o *Orchestrator) finalizeCondition(ctx context.Context, cond *rctypes.Cond // Queue up follow on conditions func (o *Orchestrator) queueFollowingCondition(ctx context.Context, cond *rctypes.Condition) error { - active, err := o.repository.GetActiveCondition(ctx, cond.ID) + active, err := o.repository.GetActiveCondition(ctx, cond.Target) if err != nil && errors.Is(err, store.ErrConditionNotFound) { // nothing more to do return nil diff --git a/internal/orchestrator/updates_test.go b/internal/orchestrator/updates_test.go index dd9ba67c..73e47206 100644 --- a/internal/orchestrator/updates_test.go +++ b/internal/orchestrator/updates_test.go @@ -958,26 +958,27 @@ func TestConditionListenersExit(t *testing.T) { } func TestQueueFollowingCondition(t *testing.T) { + serverID := uuid.New() t.Run("no following work", func(t *testing.T) { condID := uuid.New() repo := store.NewMockRepository(t) - repo.On("GetActiveCondition", mock.Anything, condID).Return(nil, store.ErrConditionNotFound).Once() + repo.On("GetActiveCondition", mock.Anything, serverID).Return(nil, store.ErrConditionNotFound).Once() o := &Orchestrator{ logger: logger, repository: repo, } - condArg := &rctypes.Condition{ID: condID} + condArg := &rctypes.Condition{ID: condID, Target: serverID} require.NoError(t, o.queueFollowingCondition(context.TODO(), condArg)) }) t.Run("lookup error", func(t *testing.T) { condID := uuid.New() repo := store.NewMockRepository(t) - repo.On("GetActiveCondition", mock.Anything, condID).Return(nil, errors.New("pound sand")).Once() + repo.On("GetActiveCondition", mock.Anything, serverID).Return(nil, errors.New("pound sand")).Once() o := &Orchestrator{ logger: logger, repository: repo, } - condArg := &rctypes.Condition{ID: condID} + condArg := &rctypes.Condition{ID: condID, Target: serverID} err := o.queueFollowingCondition(context.TODO(), condArg) require.Error(t, err) require.ErrorIs(t, err, errCompleteEvent) @@ -988,11 +989,11 @@ func TestQueueFollowingCondition(t *testing.T) { next := &rctypes.Condition{ ID: condID, Kind: rctypes.Kind("following-kind"), - Target: uuid.New(), + Target: serverID, State: rctypes.Pending, } - condArg := &rctypes.Condition{ID: condID} - repo.On("GetActiveCondition", mock.Anything, condID).Return(next, nil).Once() + condArg := &rctypes.Condition{ID: condID, Target: serverID} + repo.On("GetActiveCondition", mock.Anything, serverID).Return(next, nil).Once() stream := eventsm.NewMockStream(t) subject := "fc-13.servers.following-kind" stream.On("Publish", mock.Anything, subject, mock.Anything).Return(errors.New("pound sand")).Once() @@ -1012,11 +1013,11 @@ func TestQueueFollowingCondition(t *testing.T) { next := &rctypes.Condition{ ID: condID, Kind: rctypes.Kind("following-kind"), - Target: uuid.New(), + Target: serverID, State: rctypes.Active, } - condArg := &rctypes.Condition{ID: condID} - repo.On("GetActiveCondition", mock.Anything, condID).Return(next, nil).Once() + condArg := &rctypes.Condition{ID: condID, Target: serverID} + repo.On("GetActiveCondition", mock.Anything, serverID).Return(next, nil).Once() o := &Orchestrator{ logger: logger, repository: repo, @@ -1030,11 +1031,11 @@ func TestQueueFollowingCondition(t *testing.T) { next := &rctypes.Condition{ ID: condID, Kind: rctypes.Kind("following-kind"), - Target: uuid.New(), + Target: serverID, State: rctypes.Pending, } - condArg := &rctypes.Condition{ID: condID} - repo.On("GetActiveCondition", mock.Anything, condID).Return(next, nil).Once() + condArg := &rctypes.Condition{ID: condID, Target: serverID} + repo.On("GetActiveCondition", mock.Anything, serverID).Return(next, nil).Once() stream := eventsm.NewMockStream(t) subject := "fc-13.servers.following-kind" stream.On("Publish", mock.Anything, subject, mock.Anything).Return(nil).Once()