Skip to content

Commit df1757e

Browse files
committed
test(runs): cover settings applied at run creation
Signed-off-by: davidlin20dev <davidlin20.dev@gmail.com>
1 parent cb8e3ca commit df1757e

2 files changed

Lines changed: 173 additions & 0 deletions

File tree

runs/service/run_service_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ import (
1313
"time"
1414

1515
"connectrpc.com/connect"
16+
"github.com/flyteorg/flyte/v2/gen/go/flyteidl2/settings"
1617
"github.com/golang/protobuf/proto"
1718
"github.com/stretchr/testify/assert"
1819
"github.com/stretchr/testify/mock"
1920
"github.com/stretchr/testify/require"
21+
"google.golang.org/protobuf/encoding/protojson"
2022
"google.golang.org/protobuf/types/known/timestamppb"
2123

2224
"github.com/flyteorg/flyte/v2/flytestdlib/storage"
@@ -36,6 +38,32 @@ import (
3638
"github.com/flyteorg/flyte/v2/runs/repository/models"
3739
)
3840

41+
// noSettings returns a settings repo reporting no stored rows, so run creation
42+
// resolves to empty settings and the applier changes nothing.
43+
func noSettings(t *testing.T) *repoMocks.SettingsRepo {
44+
m := &repoMocks.SettingsRepo{}
45+
m.On("GetSettingsByKeys", mock.Anything, mock.Anything).Return(nil, nil)
46+
return m
47+
}
48+
49+
// settingsWithQueue returns a settings repo holding one org-level row that sets the
50+
// default queue. The row key must match what fetchLevels asks for, or the lookup
51+
// aligns to nothing.
52+
func settingsWithQueue(t *testing.T, org, queue string) *repoMocks.SettingsRepo {
53+
t.Helper()
54+
data, err := protojson.Marshal(&settings.Settings{
55+
Run: &settings.RunSettings{
56+
DefaultQueue: &settings.StringSetting{State: stateValue, StringValue: queue},
57+
},
58+
})
59+
require.NoError(t, err)
60+
61+
m := &repoMocks.SettingsRepo{}
62+
m.On("GetSettingsByKeys", mock.Anything, mock.Anything).
63+
Return([]*models.Settings{{Key: models.EncodeSettingsKey(org, "", ""), Data: data, Version: 1}}, nil)
64+
return m
65+
}
66+
3967
// newMockProjectClientAlwaysOK returns a mock ProjectServiceClient whose GetProject always succeeds.
4068
func newMockProjectClientAlwaysOK(t *testing.T) *projectMocks.ProjectServiceClient {
4169
pc := projectMocks.NewProjectServiceClient(t)
@@ -546,6 +574,7 @@ func TestCreateRunResponseIncludesMetadataAndStatus(t *testing.T) {
546574

547575
svc := &RunService{
548576
repo: repo,
577+
settingsRepo: noSettings(t),
549578
actionsClient: actionsClient,
550579
projectClient: newMockProjectClientAlwaysOK(t),
551580
storagePrefix: "s3://flyte-data",
@@ -1068,6 +1097,7 @@ func TestCreateRun_WritesEmptyInputsProto(t *testing.T) {
10681097

10691098
svc := &RunService{
10701099
repo: repo,
1100+
settingsRepo: noSettings(t),
10711101
actionsClient: actionsClient,
10721102
projectClient: newMockProjectClientAlwaysOK(t),
10731103
storagePrefix: "s3://flyte-data",
@@ -1126,6 +1156,7 @@ func TestCreateRun_ResponseUsesRunModel(t *testing.T) {
11261156

11271157
svc := &RunService{
11281158
repo: repo,
1159+
settingsRepo: noSettings(t),
11291160
actionsClient: actionsClient,
11301161
projectClient: newMockProjectClientAlwaysOK(t),
11311162
storagePrefix: "s3://flyte-data",
@@ -1190,6 +1221,7 @@ func TestCreateRun_TriggerFire_CarriesRunSpecEnvVars(t *testing.T) {
11901221

11911222
svc := &RunService{
11921223
repo: repo,
1224+
settingsRepo: noSettings(t),
11931225
actionsClient: actionsClient,
11941226
projectClient: newMockProjectClientAlwaysOK(t),
11951227
storagePrefix: "s3://flyte-data",
@@ -1287,6 +1319,7 @@ func TestCreateRun_ActionIDUsesRunName(t *testing.T) {
12871319

12881320
svc := &RunService{
12891321
repo: repo,
1322+
settingsRepo: noSettings(t),
12901323
actionsClient: actionsClient,
12911324
projectClient: newMockProjectClientAlwaysOK(t),
12921325
storagePrefix: "s3://flyte-data",
@@ -1377,6 +1410,7 @@ func TestCreateRun_PreservesInputContextAndRawDataPath(t *testing.T) {
13771410

13781411
svc := &RunService{
13791412
repo: repo,
1413+
settingsRepo: noSettings(t),
13801414
actionsClient: actionsClient,
13811415
projectClient: newMockProjectClientAlwaysOK(t),
13821416
storagePrefix: "s3://flyte-data",
@@ -1434,6 +1468,65 @@ func TestCreateRun_PreservesInputContextAndRawDataPath(t *testing.T) {
14341468
require.NoError(t, err)
14351469
}
14361470

1471+
// TestCreateRun_AppliesSettingsQueue proves the applier is actually wired into run
1472+
// creation: the request names no queue, and the value stored on the run comes from
1473+
// the org's settings row.
1474+
func TestCreateRun_AppliesSettingsQueue(t *testing.T) {
1475+
actionRepo := &repoMocks.ActionRepo{}
1476+
taskRepo := &repoMocks.TaskRepo{}
1477+
actionsClient := actionsconnectmocks.NewActionsServiceClient(t)
1478+
repo := &repoMocks.Repository{}
1479+
store := &storageMocks.ComposedProtobufStore{}
1480+
dataStore := &storage.DataStore{ComposedProtobufStore: store}
1481+
1482+
repo.On("ActionRepo").Return(actionRepo)
1483+
repo.On("TaskRepo").Return(taskRepo)
1484+
1485+
svc := &RunService{
1486+
repo: repo,
1487+
settingsRepo: settingsWithQueue(t, "org", "fast-queue"),
1488+
actionsClient: actionsClient,
1489+
projectClient: newMockProjectClientAlwaysOK(t),
1490+
storagePrefix: "s3://flyte-data",
1491+
dataStore: dataStore,
1492+
}
1493+
1494+
req := &workflow.CreateRunRequest{
1495+
Id: &workflow.CreateRunRequest_RunId{
1496+
RunId: &common.RunIdentifier{
1497+
Org: "org",
1498+
Project: "proj",
1499+
Domain: "dev",
1500+
Name: "rq-123",
1501+
},
1502+
},
1503+
InputWrapper: &workflow.CreateRunRequest_Inputs{Inputs: &task.Inputs{}},
1504+
Task: &workflow.CreateRunRequest_TaskSpec{
1505+
TaskSpec: &task.TaskSpec{},
1506+
},
1507+
}
1508+
1509+
store.On("WriteProtobuf", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
1510+
taskRepo.On("CreateTaskSpec", mock.Anything, mock.Anything).Return(nil).Once()
1511+
1512+
actionRepo.On("CreateAction", mock.Anything, mock.MatchedBy(func(m *models.Run) bool {
1513+
var rs task.RunSpec
1514+
_ = proto.Unmarshal(m.RunSpec, &rs)
1515+
return rs.GetQueue() == "fast-queue"
1516+
}), mock.Anything).Return(&models.Run{
1517+
Project: "proj",
1518+
Domain: "dev",
1519+
Name: "rq-123",
1520+
}, nil).Once()
1521+
1522+
actionsClient.On("Enqueue", mock.Anything, mock.MatchedBy(func(req *connect.Request[actions.EnqueueRequest]) bool {
1523+
return req.Msg.GetRunSpec().GetQueue() == "fast-queue"
1524+
})).Return(connect.NewResponse(&actions.EnqueueResponse{}), nil).Once()
1525+
1526+
_, err := svc.CreateRun(context.Background(), connect.NewRequest(req))
1527+
require.NoError(t, err)
1528+
}
1529+
14371530
func TestCreateRun_WithOffloadedInputData(t *testing.T) {
14381531
actionRepo := &repoMocks.ActionRepo{}
14391532
taskRepo := &repoMocks.TaskRepo{}
@@ -1447,6 +1540,7 @@ func TestCreateRun_WithOffloadedInputData(t *testing.T) {
14471540

14481541
svc := &RunService{
14491542
repo: repo,
1543+
settingsRepo: noSettings(t),
14501544
actionsClient: actionsClient,
14511545
projectClient: newMockProjectClientAlwaysOK(t),
14521546
storagePrefix: "s3://flyte-data",
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,80 @@
11
package service
2+
3+
import (
4+
"testing"
5+
6+
"github.com/flyteorg/flyte/v2/gen/go/flyteidl2/settings"
7+
"github.com/flyteorg/flyte/v2/gen/go/flyteidl2/task"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func runSettings(queue *settings.StringSetting, concurrency *settings.Int64Setting) *settings.Settings {
12+
return &settings.Settings{
13+
Run: &settings.RunSettings{DefaultQueue: queue, MaxActionConcurrency: concurrency},
14+
}
15+
}
16+
17+
func TestApplyRunSettings(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
spec *task.RunSpec
21+
resolved *settings.Settings
22+
wantQueue string
23+
wantConcurrency uint32
24+
}{
25+
{
26+
name: "empty queue takes the settings value",
27+
spec: &task.RunSpec{},
28+
resolved: runSettings(&settings.StringSetting{State: stateValue, StringValue: "fast-queue"}, nil),
29+
wantQueue: "fast-queue",
30+
},
31+
{
32+
name: "an explicit queue wins over settings",
33+
spec: &task.RunSpec{Queue: "user-queue"},
34+
resolved: runSettings(&settings.StringSetting{State: stateValue, StringValue: "fast-queue"}, nil),
35+
wantQueue: "user-queue",
36+
},
37+
{
38+
name: "a queue in INHERIT contributes nothing",
39+
spec: &task.RunSpec{},
40+
resolved: runSettings(&settings.StringSetting{State: stateInherit, StringValue: "fast-queue"}, nil),
41+
wantQueue: "",
42+
},
43+
{
44+
name: "zero concurrency takes the settings value",
45+
spec: &task.RunSpec{},
46+
resolved: runSettings(nil, &settings.Int64Setting{State: stateValue, IntValue: 5}),
47+
wantConcurrency: 5,
48+
},
49+
{
50+
name: "an explicit concurrency wins over settings",
51+
spec: &task.RunSpec{MaxActionConcurrency: 3},
52+
resolved: runSettings(nil, &settings.Int64Setting{State: stateValue, IntValue: 5}),
53+
wantConcurrency: 3,
54+
},
55+
{
56+
name: "concurrency in UNSET contributes nothing",
57+
spec: &task.RunSpec{},
58+
resolved: runSettings(nil, &settings.Int64Setting{State: stateUnset, IntValue: 5}),
59+
wantConcurrency: 0,
60+
},
61+
{
62+
name: "no settings at all",
63+
spec: &task.RunSpec{},
64+
resolved: &settings.Settings{},
65+
},
66+
{
67+
name: "nil spec does not panic",
68+
spec: nil,
69+
resolved: runSettings(&settings.StringSetting{State: stateValue, StringValue: "fast-queue"}, nil),
70+
},
71+
}
72+
73+
for _, tt := range tests {
74+
t.Run(tt.name, func(t *testing.T) {
75+
applyRunSettings(tt.spec, tt.resolved)
76+
assert.Equal(t, tt.wantQueue, tt.spec.GetQueue())
77+
assert.Equal(t, tt.wantConcurrency, tt.spec.GetMaxActionConcurrency())
78+
})
79+
}
80+
}

0 commit comments

Comments
 (0)