8000 feat(hatchery/swarm): prerequisiste model value parsed more finely (#… · ovh/cds@905887a · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Commit 905887a

Browse files
maxatomeyesnault
maxatome
authored and
yesnault
committed
feat(hatchery/swarm): prerequisiste model value parsed more finely (#4418)
* SpawnWorker params parsed more finely * New power-user CDS_SERVICE_ARGS option to pass as arg to container Handle such values: CDS_SERVICE_ARGS='--user="John Doe" --details ""' Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
1 parent 9e66dcf commit 905887a

File tree

4 files changed

+318
-40
lines changed

4 files changed

+318
-40
lines changed

‎engine/hatchery/kubernetes/kubernetes.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -396,32 +396,39 @@ func (h *HatcheryKubernetes) SpawnWorker(ctx context.Context, spawnArgs hatchery
396396
for i, serv := range services {
397397
//name= <alias> => the name of the host put in /etc/hosts of the worker
398398
//value= "postgres:latest env_1=blabla env_2=blabla"" => we can add env variables in requirement name
399-
tuple := strings.Split(serv.Value, " ")
400-
img := tuple[0]
399+
img, envm := hatchery.ParseRequirementModel(serv.Value)
401400

402401
servContainer := apiv1.Container{
403402
Name: fmt.Sprintf("service-%d-%s", serv.ID, strings.ToLower(serv.Name)),
404403
Image: img,
405404
}
406405

407-
if len(tuple) > 1 {
408-
servContainer.Env = make([]apiv1.EnvVar, 0, len(tuple)-1)
409-
for _, servEnv := range tuple[1:] {
410-
envSplitted := strings.Split(servEnv, "=")
411-
if len(envSplitted) < 2 {
412-
continue
413-
}
414-
if envSplitted[0] == "CDS_SERVICE_MEMORY" {
415-
servContainer.Resources = apiv1.ResourceRequirements{
416-
Requests: apiv1.ResourceList{
417-
apiv1.ResourceMemory: resource.MustParse(envSplitted[1]),
418-
},
419-
}
420-
continue
421-
}
422-
servContainer.Env = append(servContainer.Env, apiv1.EnvVar{Name: envSplitted[0], Value: envSplitted[1]})
406+
if sm, ok := envm["CDS_SERVICE_MEMORY"]; ok {
407+
mq, err := resource.ParseQuantity(sm)
408+
if err != nil {
409+
log.Warning("hatchery> kubernetes> SpawnWorker> Unable to parse CDS_SERVICE_MEMORY value '%s': %s", sm, err)
410+
continue
411+
}
412+
servContainer.Resources = apiv1.ResourceRequirements{
413+
Requests: apiv1.ResourceList{
414+
apiv1.ResourceMemory: mq,
415+
},
423416
}
417+
delete(envm, "CDS_SERVICE_MEMORY")
424418
}
419+
420+
if sa, ok := envm["CDS_SERVICE_ARGS"]; ok {
421+
servContainer.Args = hatchery.ParseArgs(sa)
422+
delete(envm, "CDS_SERVICE_ARGS")
423+
}
424+
425+
if len(envm) > 0 {
426+
servContainer.Env = make([]apiv1.EnvVar, 0, len(envm))
427+
for key, val := range envm {
428+
servContainer.Env = append(servContainer.Env, apiv1.EnvVar{Name: key, Value: val})
429+
}
430+
}
431+
425432
podSchema.ObjectMeta.Labels[LABEL_SERVICE_JOB_ID] = fmt.Sprintf("%d", spawnArgs.JobID)
426433
podSchema.Spec.Containers = append(podSchema.Spec.Containers, servContainer)
427434
podSchema.Spec.HostAliases[0].Hostnames[i+1] = strings.ToLower(serv.Name)

‎engine/hatchery/swarm/swarm.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -263,31 +263,32 @@ func (h *HatcherySwarm) SpawnWorker(ctx context.Context, spawnArgs hatchery.Spaw
263263
}
264264
//name= <alias> => the name of the host put in /etc/hosts of the worker
265265
//value= "postgres:latest env_1=blabla env_2=blabla" => we can add env variables in requirement name
266-
tuple := strings.Split(r.Value, " ")
267-
img := tuple[0]
268-
env := []string{}
266+
img, envm := hatchery.ParseRequirementModel(r.Value)
267+
269268
serviceMemory := int64(1024)
270-
if len(tuple) > 1 {
271-
for i := 1; i < len(tuple); i++ {
272-
splittedTuple := strings.SplitN(tuple[i], "=", 2)
273-
name := splittedTuple[0]
274-
val := strings.TrimLeft(splittedTuple[1], "\"")
275-
val = strings.TrimRight(val, "\"")
276-
env = append(env, name+"="+val)
277-
}
278-
}
279-
//option for power user : set the service memory with CDS_SERVICE_MEMORY=1024
280-
for _, e := range env {
281-
if strings.HasPrefix(e, "CDS_SERVICE_MEMORY=") {
282-
m := strings.Replace(e, "CDS_SERVICE_MEMORY=", "", -1)
283-
i, err := strconv.Atoi(m)
284-
if err != nil {
285-
log.Warning("hatchery> swarm> SpawnWorker> Unable to parse service option %s : %v", e, err)
286-
continue
287-
}
F438
269+
if sm, ok := envm["CDS_SERVICE_MEMORY"]; ok {
270+
i, err := strconv.ParseUint(sm, 10, 32)
271+
if err != nil {
272+
log.Warning("SpawnWorker> Unable to parse service option CDS_SERVICE_MEMORY=%s : %s", sm, err)
273+
} else {
274+
// too low values are checked in HatcherySwarm.createAndStartContainer() below
288275
serviceMemory = int64(i)
289276
}
290277
}
278+
279+
var cmdArgs []string
280+
if sa, ok := envm["CDS_SERVICE_ARGS"]; ok {
281+
cmdArgs = hatchery.ParseArgs(sa)
282+
}
283+
if cmdArgs == nil {
284+
cmdArgs = []string{}
285+
}
286+
287+
env := make([]string, 0, len(envm))
288+
for key, val := range envm {
289+
env = append(env, key+"="+val)
290+
}
291+
291292
serviceName := r.Name + "-" + name
292293

293294
//labels are used to make container cleanup easier. We "link" the service to its worker this way.
@@ -309,7 +310,7 @@ func (h *HatcherySwarm) SpawnWorker(ctx context.Context, spawnArgs hatchery.Spaw
309310
image: img,
310311
network: network,
311312
networkAlias: r.Name,
312-
cmd: []string{},
313+
cmd: cmdArgs,
313314
env: env,
314315
labels: labels,
315316
memory: serviceMemory,

‎sdk/hatchery/requirement.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package hatchery
2+
3+
import (
4+
"bytes"
5+
"regexp"
6+
"strings"
7+
"unicode"
8+
)
9+
10+
var (
11+
// reSplitParams accepts:
12+
// TEST
13+
// TEST=
14+
// TEST=12
15+
// TEST='12'
16+
// TEST='1\'2'
17+
// TEST="12"
18+
// TEST="1\"2"
19+
// It does not allow spaces around '='.
20+
splitParams = regexp.MustCompile(`([a-zA-Z_]\w+)(?:=('(?:\\.|[^'\\]+)*'|"(?:\\.|[^"\\]+)*"|\S*))?`).FindAllStringSubmatch
21+
22+
// unescapeBackslash allows to unescape a backslash-escaped string.
23+
unescapeBackslash = strings.NewReplacer(`\\`, `\`, `\`, ``).Replace
24+
)
25+
26+
func quoted(s string) bool {
27+
if len(s) >= 2 {
28+
switch s[0] {
29+
case '\'':
30+
return s[len(s)-1] == '\''
31+
case '"':
32+
return s[len(s)-1] == '"'
33+
}
34+
}
35+
return false
36+
}
37+
38+
// ParseRequirementModel parses a requirement model than returns the
39+
// image name and the environment variables.
40+
//
41+
// Example of input:
42+
// "postgres:latest env_1=blabla env_2=blabla env_3 env_4='zip'"
43+
func ParseRequirementModel(rm string) (string, map[string]string) {
44+
var env map[string]string
45+
46+
tuple := strings.SplitN(rm, " ", 2)
47+
img := tuple[0]
48+
49+
if len(tuple) > 1 {
50+
matches := splitParams(tuple[1], -1)
51+
if matches != nil {
52+
env = make(map[string]string, len(matches))
53+
for _, m := range matches {
54+
name, value := m[1], m[2]
55+
if quoted(value) {
56+
value = unescapeBackslash(value[1 : len(value)-1])
57+
}
58+
// non-quoted values cannot be escaped here
59+
env[name] = value
60+
}
61+
}
62+
}
63+
64+
return img, env
65+
}
66+
67+
// ParseArgs splits str on spaces into a slice of strings taking into
68+
// account any quoting (using '' or "") even inside args, and any
69+
// backslash-escaping even without quotes:
70+
// `abc def` → ["abc", "def"]
71+
// ` abc def ` → ["abc", "def"]
72+
// ` '' "" ` → ["", ""]
73+
// ` a'bc' d"e"f ` → ["abc", "def"]
74+
// `'a\bc\'' "def" ` → ["abc'", "def"]
75+
// ` abc\ def ` → ["abc def"]
76+
func ParseArgs(str string) []string {
77+
str = strings.TrimSpace(str)
78+
if str == "" {
79+
return nil
80+
}
81+
82+
var (
83+
quoted rune
84+
cur bytes.Buffer
85+
bs, sp bool
86+
)
87+
88+
var ret []string
89+
for _, r := range str {
90+
if sp {
91+
if unicode.IsSpace(r) {
92+
continue
93+
}
94+
sp = false
95+
}
96+
97+
if bs {
98+
cur.WriteRune(r)
99+
bs = false
100+
continue
101+
}
102+
103+
if r == '\\' {
104+
bs = true
105+
continue
106+
}
107+
108+
// currently quoted
109+
if quoted != 0 {
110+
if r == quoted { // close quoting
111+
quoted = 0
112+
} else {
113+
cur.WriteRune(r)
114+
}
115+
continue
116+
}
117+
118+
switch r {
119+
case '"', '\'': // open quoting
120+
quoted = r
121+
continue
122+
}
123+
124+
if unicode.IsSpace(r) {
125+
ret = append(ret, cur.String())
126+
cur.Truncate(0)
127+
sp = true
128+
continue
129+
}
130+
131+
cur.WriteRune(r)
132+
}
133+
134+
// Last backslash is let as is
135+
if bs {
136+
cur.WriteRune('\\')
137+
}
138+
return append(ret, cur.String())
139+
}

‎sdk/hatchery/requirement_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package hatchery_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
"github.com/ovh/cds/sdk/hatchery"
9+
)
10+
11+
func TestParseRequirementModel(t *testing.T) {
12+
for _, test := range []struct {
13+
in string
14+
expectedImg string
15+
expectedEnv map[string]string
16+
}{
17+
{
18+
in: "",
19+
expectedImg: "",
20+
expectedEnv: nil,
21+
},
22+
{
23+
in: "no:env",
24+
expectedImg: "no:env",
25+
expectedEnv: nil,
26+
},
27+
{
28+
in: "no:env:space ",
29+
expectedImg: "no:env:space",
30+
expectedEnv: nil,
31+
},
32+
{
33+
in: `image_name TEST=abc V1='a "b" c' V2="a 'b' c" V3 V4=`,
34+
expectedImg: "image_name",
35+
expectedEnv: map[string]string{
36+
"TEST": "abc",
37+
`V1`: `a "b" c`,
38+
"V2": "a 'b' c",
39+
"V3": "",
40+
"V4": "",
41+
},
42+
},
43+
{
44+
in: `with_spaces TEST="foo bar" V1=12 V2= V3 V4='1 8'`,
45+
expectedImg: "with_spaces",
46+
expectedEnv: map[string]string{
47+
"TEST": "foo bar",
48+
"V1": "12",
49+
"V2": "",
50+
"V3": "",
51+
"V4": "1 8",
52+
},
53+
},
54+
{
55+
in: `backslash FOO="a\"\b\\c" BAR='a\'\b\\c'`,
56+
expectedImg: "backslash",
57+
expectedEnv: map[string]string{
58+
`FOO`: `a"b\c`,
59+
`BAR`: `a'b\c`,
60+
},
61+
},
62+
} {
63+
img, env := hatchery.ParseRequirementModel(test.in)
64+
65+
assert.Equal(t, test.expectedImg, img,
66+
"image returned by ParseRequirementModel("+test.in+")")
67+
68+
assert.Equal(t, test.expectedEnv, env,
69+
"environment returned by ParseRequirementModel("+test.in+")")
70+
}
71+
}
72+
73+
func TestParseArgs(t *testing.T) {
74+
for _, test := range []struct {
75+
in string
76+
expectedArgs []string
77+
}{
78+
{
79+
in: "",
80+
expectedArgs: nil,
81+
},
82+
{
83+
in: " \t ",
84+
expectedArgs: nil,
85+
},
86+
{
87+
in: " abc \t\n def ghi \t ",
88+
expectedArgs: []string{"abc", "def", "ghi"},
89+
},
90+
{
91+
in: ` abc 'def' "ghi" `,
92+
expectedArgs: []string{"abc", "def", "ghi"},
93+
},
94+
{
95+
in: ` abc d'e'f g"h"i `,
96+
expectedArgs: []string{"abc", "def", "ghi"},
97+
},
98+
{
99+
in: ` abc '' "" `,
100+
expectedArgs: []string{"abc", "", ""},
101+
},
102+
{
103+
in: ` a\\b\c\ 'd\\e"\f\'' "g\\h'\i\"" `,
104+
expectedArgs: []string{`a\bc `, `d\e"f'`, `g\h'i"`},
105+
},
106+
{
107+
// edge case, non closed ", skip it anyway
108+
in: ` "abc `,
109+
expectedArgs: []string{`abc`},
110+
},
111+
{
112+
// edge case, non closed ', skip it anyway
113+
in: ` 'abc `,
114+
expectedArgs: []string{`abc`},
115+
},
116+
{
117+
// edge case, final backslash, keep it as is
118+
in: ` 'abc\`,
119+
expectedArgs: []string{`abc\`},
120+
},
121+
{
122+
// typical use case
123+
in: `--name="Bob Foo" -c user='foo bar' '' ""`,
124+
expectedArgs: []string{`--name=Bob Foo`, `-c`, `user=foo bar`, ``, ``},
125+
},
126+
} {
127+
args := hatchery.ParseArgs(test.in)
128+
129+
assert.Equal(t, test.expectedArgs, args, "ParseArgs("+test.in+")")
130+
}
131+
}

0 commit comments

Comments
 (0)
0