Skip to content

Commit d21537d

Browse files
authored
fix(api): rename worker model (#5183)
* fix(api): rename worker model Signed-off-by: Yvonnick Esnault <yvonnick.esnault@corp.ovh.com>
1 parent 2497dfa commit d21537d

File tree

4 files changed

+231
-14
lines changed

4 files changed

+231
-14
lines changed

engine/api/action/dao.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package action
22

33
import (
44
"context"
5+
"fmt"
56
"strings"
67

78
"github.com/go-gorp/gorp"
@@ -197,11 +198,14 @@ func GetRequirementsDistinctBinary(db gorp.SqlExecutor) (sdk.RequirementList, er
197198
func GetRequirementsTypeModelAndValueStartBy(ctx context.Context, db gorp.SqlExecutor, value string) ([]sdk.Requirement, error) {
198199
rs := []sdk.Requirement{}
199200

201+
// if value equals Debian9, the regex should match "Debian9" and "Debian9 --foo", but not "Debian9-Foo"
202+
reg := fmt.Sprintf("^%s(?!\\S)", value)
203+
200204
query := gorpmapping.NewQuery(`
201205
SELECT *
202206
FROM action_requirement
203-
WHERE type = 'model' AND value LIKE $1
204-
`).Args(value + "%")
207+
WHERE type = 'model' AND (value ~ $1)
208+
`).Args(reg)
205209

206210
if err := gorpmapping.GetAll(ctx, db, query, &rs); err != nil {
207211
return nil, sdk.WrapError(err, "cannot get requirements")

engine/api/pipeline/pipeline.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ func LoadByWorkerModel(ctx context.Context, db gorp.SqlExecutor, model *sdk.Mode
9393
var query gorpmapping.Query
9494

9595
isSharedInfraModel := model.GroupID == group.SharedInfraGroup.ID
96-
modelNamePattern := model.Group.Name + "/" + model.Name + "%"
96+
modelNamePatternWithGroup := model.Group.Name + "/" + model.Name
97+
98+
modelNamePattern1 := fmt.Sprintf("^%s(?!\\S)", model.Name)
99+
modelNamePattern2 := fmt.Sprintf("^%s(?!\\S)", modelNamePatternWithGroup)
97100

98101
if isSharedInfraModel {
99102
query = gorpmapping.NewQuery(`
@@ -104,8 +107,8 @@ func LoadByWorkerModel(ctx context.Context, db gorp.SqlExecutor, model *sdk.Mode
104107
JOIN pipeline ON pipeline.id = pipeline_stage.pipeline_id
105108
JOIN project ON project.id = pipeline.project_id
106109
WHERE action_requirement.type = 'model'
107-
AND (action_requirement.value LIKE $1 OR action_requirement.value LIKE $2)
108-
`).Args(model.Name+"%", modelNamePattern)
110+
AND (action_requirement.value ~ $1 OR action_requirement.value ~ $2)
111+
`).Args(modelNamePattern1, modelNamePattern2)
109112
} else {
110113
query = gorpmapping.NewQuery(`
111114
SELECT DISTINCT pipeline.*, project.projectkey AS projectKey
@@ -115,13 +118,13 @@ func LoadByWorkerModel(ctx context.Context, db gorp.SqlExecutor, model *sdk.Mode
115118
JOIN pipeline ON pipeline.id = pipeline_stage.pipeline_id
116119
JOIN project ON project.id = pipeline.project_id
117120
WHERE action_requirement.type = 'model'
118-
AND action_requirement.value LIKE $1
119-
`).Args(modelNamePattern)
121+
AND action_requirement.value ~ $1
122+
`).Args(modelNamePattern2)
120123
}
121124

122125
var dbPips Pipelines
123126
if err := gorpmapping.GetAll(ctx, db, query, &dbPips); err != nil {
124-
return nil, sdk.WrapError(err, "unable to load pipelines linked to worker model pattern %s", modelNamePattern)
127+
return nil, sdk.WrapError(err, "unable to load pipelines linked to worker model pattern %s", modelNamePattern2)
125128
}
126129

127130
return dbPips.Cast(), nil
@@ -132,7 +135,10 @@ func LoadByWorkerModelAndGroupIDs(ctx context.Context, db gorp.SqlExecutor, mode
132135
var query gorpmapping.Query
133136

134137
isSharedInfraModel := model.GroupID == group.SharedInfraGroup.ID
135-
modelNamePattern := model.Group.Name + "/" + model.Name + "%"
138+
modelNamePatternWithGroup := model.Group.Name + "/" + model.Name
139+
140+
modelNamePattern1 := fmt.Sprintf("^%s(?!\\S)", model.Name)
141+
modelNamePattern2 := fmt.Sprintf("^%s(?!\\S)", modelNamePatternWithGroup)
136142

137143
if isSharedInfraModel {
138144
query = gorpmapping.NewQuery(`
@@ -143,13 +149,13 @@ func LoadByWorkerModelAndGroupIDs(ctx context.Context, db gorp.SqlExecutor, mode
143149
JOIN pipeline ON pipeline.id = pipeline_stage.pipeline_id
144150
JOIN project ON project.id = pipeline.project_id
145151
WHERE action_requirement.type = 'model'
146-
AND (action_requirement.value LIKE $1 OR action_requirement.value LIKE $2)
152+
AND (action_requirement.value ~ $1 OR action_requirement.value ~ $2)
147153
AND project.id IN (
148154
SELECT project_group.project_id
149155
FROM project_group
150156
WHERE project_group.group_id = ANY(string_to_array($3, ',')::int[])
151157
)
152-
`).Args(model.Name+"%", modelNamePattern, gorpmapping.IDsToQueryString(groupIDs))
158+
`).Args(modelNamePattern1, modelNamePattern2, gorpmapping.IDsToQueryString(groupIDs))
153159
} else {
154160
query = gorpmapping.NewQuery(`
155161
SELECT DISTINCT pipeline.*, project.projectkey AS projectKey
@@ -159,19 +165,19 @@ func LoadByWorkerModelAndGroupIDs(ctx context.Context, db gorp.SqlExecutor, mode
159165
JOIN pipeline ON pipeline.id = pipeline_stage.pipeline_id
160166
JOIN project ON project.id = pipeline.project_id
161167
WHERE action_requirement.type = 'model'
162-
AND action_requirement.value LIKE $1
168+
AND action_requirement.value ~ $1
163169
AND project.id IN (
164170
SELECT project_group.project_id
165171
FROM project_group
166172
WHERE
167173
project_group.group_id = ANY(string_to_array($2, ',')::int[])
168174
)
169-
`).Args(modelNamePattern, gorpmapping.IDsToQueryString(groupIDs))
175+
`).Args(modelNamePattern2, gorpmapping.IDsToQueryString(groupIDs))
170176
}
171177

172178
var pips Pipelines
173179
if err := gorpmapping.GetAll(ctx, db, query, &pips); err != nil {
174-
return nil, sdk.WrapError(err, "unable to load pipelines linked to worker model pattern %s", modelNamePattern)
180+
return nil, sdk.WrapError(err, "unable to load pipelines linked to worker model pattern %s", modelNamePattern2)
175181
}
176182

177183
return pips.Cast(), nil

engine/api/pipeline/pipeline_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ func TestLoadByWorkerModel(t *testing.T) {
200200

201201
model1 := sdk.Model{Name: sdk.RandomString(10), Group: g1, GroupID: g1.ID}
202202
model2 := sdk.Model{Name: sdk.RandomString(10), Group: g2, GroupID: g2.ID}
203+
model3 := sdk.Model{Name: model1.Name[:5], Group: g1, GroupID: g1.ID}
203204

204205
projectKey := sdk.RandomString(10)
205206
proj := assets.InsertTestProject(t, db, cache, projectKey, projectKey)
@@ -267,6 +268,13 @@ func TestLoadByWorkerModel(t *testing.T) {
267268
assert.Equal(t, pip1.Name, pips[0].Name)
268269
assert.Equal(t, pip2.Name, pips[1].Name)
269270

271+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, &model3)
272+
assert.NoError(t, err)
273+
274+
if !assert.Equal(t, 0, len(pips)) {
275+
t.FailNow()
276+
}
277+
270278
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, &model2)
271279
assert.NoError(t, err)
272280

@@ -275,6 +283,10 @@ func TestLoadByWorkerModel(t *testing.T) {
275283
}
276284
assert.Equal(t, pip3.Name, pips[0].Name)
277285

286+
pips, err = pipeline.LoadByWorkerModelAndGroupIDs(context.TODO(), db, &model3, []int64{})
287+
assert.NoError(t, err)
288+
assert.Equal(t, 0, len(pips))
289+
278290
pips, err = pipeline.LoadByWorkerModelAndGroupIDs(context.TODO(), db, &model1, []int64{})
279291
assert.NoError(t, err)
280292
assert.Equal(t, 0, len(pips))

engine/api/workermodel/crud_test.go

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@ package workermodel_test
22

33
import (
44
"context"
5+
"fmt"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
89
"github.com/stretchr/testify/require"
910

1011
"github.com/ovh/cds/engine/api/bootstrap"
12+
"github.com/ovh/cds/engine/api/group"
13+
"github.com/ovh/cds/engine/api/pipeline"
1114
"github.com/ovh/cds/engine/api/test"
1215
"github.com/ovh/cds/engine/api/test/assets"
1316
"github.com/ovh/cds/engine/api/workermodel"
@@ -122,6 +125,198 @@ func TestUpdateModel(t *testing.T) {
122125
cpy.GroupID = g2.ID
123126
res, err = workermodel.Update(context.TODO(), db, res, cpy)
124127
require.Error(t, err)
128+
129+
}
130+
131+
// create a worker model aaa
132+
// a pipeline use worker model aaa-foo
133+
// rename worker model to aaa-bar
134+
// the pipeline should keep the name aaa-foo
135+
func TestUpdateModelInPipeline(t *testing.T) {
136+
db, cache, end := test.SetupPG(t, bootstrap.InitiliazeDB)
137+
defer end()
138+
139+
g1 := assets.InsertTestGroup(t, db, sdk.RandomString(10))
140+
u, _ := assets.InsertLambdaUser(t, db)
141+
142+
pattern := sdk.ModelPattern{
143+
Name: sdk.RandomString(10),
144+
Type: sdk.Docker,
145+
Model: sdk.ModelCmds{
146+
Cmd: "pattern cmd",
147+
Shell: "pattern shell",
148+
},
149+
}
150+
require.NoError(t, workermodel.InsertPattern(db, &pattern))
151+
152+
model1Name := sdk.RandomString(10)
153+
model1, err := workermodel.Create(context.TODO(), db, sdk.Model{
154+
Type: sdk.Docker,
155+
Name: model1Name,
156+
Group: g1,
157+
GroupID: g1.ID,
158+
ModelDocker: sdk.ModelDocker{},
159+
}, u)
160+
require.NoError(t, err)
161+
162+
model1NameFoo := model1Name + "-foo"
163+
model1Foo, err := workermodel.Create(context.TODO(), db, sdk.Model{
164+
Type: sdk.Docker,
165+
Name: model1NameFoo,
166+
Group: g1,
167+
GroupID: g1.ID,
168+
ModelDocker: sdk.ModelDocker{},
169+
}, u)
170+
require.NoError(t, err)
171+
172+
projectKey := sdk.RandomString(10)
173+
proj := assets.InsertTestProject(t, db, cache, projectKey, projectKey)
174+
175+
require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{
176+
GroupID: g1.ID,
177+
ProjectID: proj.ID,
178+
Role: sdk.PermissionReadWriteExecute,
179+
}))
180+
181+
pip1 := sdk.Pipeline{ProjectID: proj.ID, ProjectKey: proj.Key, Name: sdk.RandomString(10)}
182+
test.NoError(t, pipeline.InsertPipeline(db, &pip1))
183+
job1 := sdk.Job{
184+
Enabled: true,
185+
Action: sdk.Action{
186+
Enabled: true,
187+
Requirements: []sdk.Requirement{{
188+
Type: sdk.ModelRequirement,
189+
Name: fmt.Sprintf("%s/%s-foo --privileged", g1.Name, model1.Name),
190+
Value: fmt.Sprintf("%s/%s-foo --privileged", g1.Name, model1.Name),
191+
}},
192+
},
193+
}
194+
test.NoError(t, pipeline.InsertJob(db, &job1, 0, &pip1))
195+
196+
model1FooLoad, err := workermodel.LoadByNameAndGroupID(db, model1Foo.Name, g1.ID)
197+
require.NoError(t, err)
198+
199+
pips, err := pipeline.LoadByWorkerModel(context.TODO(), db, model1FooLoad)
200+
assert.NoError(t, err)
201+
require.Equal(t, 1, len(pips))
202+
203+
model1Load, err := workermodel.LoadByIDWithClearPassword(db, model1.ID)
204+
require.NoError(t, err)
205+
206+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, model1Load)
207+
assert.NoError(t, err)
208+
require.Equal(t, 0, len(pips))
209+
210+
// Test rename worker model
211+
res, err := workermodel.Update(context.TODO(), db, model1Load, sdk.Model{
212+
Type: sdk.Docker,
213+
Name: model1Name + "-bar",
214+
PatternName: pattern.Name,
215+
GroupID: g1.ID,
216+
ModelDocker: sdk.ModelDocker{
217+
Private: true,
218+
Password: sdk.PasswordPlaceholder,
219+
},
220+
})
221+
require.NoError(t, err)
222+
223+
model2Load, err := workermodel.LoadByIDWithClearPassword(db, res.ID)
224+
require.NoError(t, err)
225+
226+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, model1Load)
227+
assert.NoError(t, err)
228+
require.Equal(t, 0, len(pips))
229+
230+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, model2Load)
231+
assert.NoError(t, err)
232+
require.Equal(t, 0, len(pips))
233+
234+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, model1FooLoad)
235+
assert.NoError(t, err)
236+
require.Equal(t, 1, len(pips))
237+
238+
}
239+
240+
func TestUpdateModelInPipelineSimple(t *testing.T) {
241+
db, cache, end := test.SetupPG(t, bootstrap.InitiliazeDB)
242+
defer end()
243+
244+
g1 := assets.InsertTestGroup(t, db, sdk.RandomString(10))
245+
u, _ := assets.InsertLambdaUser(t, db)
246+
247+
pattern := sdk.ModelPattern{
248+
Name: sdk.RandomString(10),
249+
Type: sdk.Docker,
250+
Model: sdk.ModelCmds{
251+
Cmd: "pattern cmd",
252+
Shell: "pattern shell",
253+
},
254+
}
255+
require.NoError(t, workermodel.InsertPattern(db, &pattern))
256+
257+
model1Name := sdk.RandomString(10)
258+
model1, err := workermodel.Create(context.TODO(), db, sdk.Model{
259+
Type: sdk.Docker,
260+
Name: model1Name,
261+
Group: g1,
262+
GroupID: g1.ID,
263+
ModelDocker: sdk.ModelDocker{},
264+
}, u)
265+
require.NoError(t, err)
266+
267+
projectKey := sdk.RandomString(10)
268+
proj := assets.InsertTestProject(t, db, cache, projectKey, projectKey)
269+
270+
require.NoError(t, group.InsertLinkGroupProject(context.TODO(), db, &group.LinkGroupProject{
271+
GroupID: g1.ID,
272+
ProjectID: proj.ID,
273+
Role: sdk.PermissionReadWriteExecute,
274+
}))
275+
276+
pip1 := sdk.Pipeline{ProjectID: proj.ID, ProjectKey: proj.Key, Name: sdk.RandomString(10)}
277+
test.NoError(t, pipeline.InsertPipeline(db, &pip1))
278+
job1 := sdk.Job{
279+
Enabled: true,
280+
Action: sdk.Action{
281+
Enabled: true,
282+
Requirements: []sdk.Requirement{{
283+
Type: sdk.ModelRequirement,
284+
Name: fmt.Sprintf("%s/%s --privileged", g1.Name, model1.Name),
285+
Value: fmt.Sprintf("%s/%s --privileged", g1.Name, model1.Name),
286+
}},
287+
},
288+
}
289+
test.NoError(t, pipeline.InsertJob(db, &job1, 0, &pip1))
290+
291+
model1Load, err := workermodel.LoadByIDWithClearPassword(db, model1.ID)
292+
require.NoError(t, err)
293+
294+
pips, err := pipeline.LoadByWorkerModel(context.TODO(), db, model1Load)
295+
assert.NoError(t, err)
296+
require.Equal(t, 1, len(pips))
297+
298+
model1NameFoo := model1Name + "-foo"
299+
res, err := workermodel.Update(context.TODO(), db, model1Load, sdk.Model{
300+
Type: sdk.Docker,
301+
Name: model1NameFoo,
302+
PatternName: pattern.Name,
303+
GroupID: g1.ID,
304+
ModelDocker: sdk.ModelDocker{
305+
Private: true,
306+
Password: sdk.PasswordPlaceholder,
307+
},
308+
})
309+
require.NoError(t, err)
310+
311+
model1FooLoad, err := workermodel.LoadByIDWithClearPassword(db, res.ID)
312+
require.NoError(t, err)
313+
314+
pips, err = pipeline.LoadByWorkerModel(context.TODO(), db, model1FooLoad)
315+
assert.NoError(t, err)
316+
require.Equal(t, 1, len(pips))
317+
318+
pip, err := pipeline.LoadPipelineByID(context.TODO(), db, pips[0].ID, true)
319+
require.Equal(t, fmt.Sprintf("%s/%s-foo --privileged", g1.Name, model1.Name), pip.Stages[0].Jobs[0].Action.Requirements[0].Value)
125320
}
126321

127322
func TestCopyModelTypeData(t *testing.T) {

0 commit comments

Comments
 (0)