Skip to content

Commit

Permalink
fix(hatchery): fix worker model name for all hatcheries (#5013)
Browse files Browse the repository at this point in the history
* fix(hatchery): fix worker model name for all hatcheries

Signed-off-by: Yvonnick Esnault <[email protected]>
  • Loading branch information
yesnault committed Feb 26, 2020
1 parent b8c5eea commit c22cd8f
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 10 deletions.
7 changes: 0 additions & 7 deletions engine/hatchery/kubernetes/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,6 @@ func (h *HatcheryKubernetes) SpawnWorker(ctx context.Context, spawnArgs hatchery
label = "register"
}

podName := spawnArgs.WorkerName
// Kubernetes pod name must not be > 63 chars
if len(podName) > 63 {
podName = podName[:60]
}
podName = strings.Replace(podName, ".", "-", -1)

var logJob string
if spawnArgs.JobID > 0 {
logJob = fmt.Sprintf("for workflow job %d,", spawnArgs.JobID)
Expand Down
2 changes: 1 addition & 1 deletion sdk/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ const (
// StatusIsTerminated returns if status is terminated (nothing related to building or waiting, ...)
func StatusIsTerminated(status string) bool {
switch status {
case StatusBuilding, StatusWaiting, "": // A stage does not have status when he's waiting a previous stage
case StatusPending, StatusBuilding, StatusWaiting, "": // A stage does not have status when he's waiting a previous stage
return false
default:
return true
Expand Down
41 changes: 39 additions & 2 deletions sdk/hatchery/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func workerStarter(ctx context.Context, h Interface, workerNum string, jobs <-ch
// this counter is reset with func workerRegister
atomic.AddInt64(&nbRegisteringWorkerModels, 1)
arg := SpawnArguments{
WorkerName: fmt.Sprintf("register-%s-%s-%s", h.Service().Name, strings.Replace(strings.ToLower(m.Name), "/", "-", -1), strings.Replace(namesgenerator.GetRandomNameCDS(0), "_", "-", -1)),
WorkerName: generateWorkerName(h.Service().Name, true, m.Name),
Model: m,
RegisterOnly: true,
HatcheryName: h.Service().Name,
Expand Down Expand Up @@ -171,7 +171,7 @@ func spawnWorkerForJob(ctx context.Context, h Interface, j workerStarterRequest)

_, next = observability.Span(ctxJob, "hatchery.SpawnWorker")
arg := SpawnArguments{
WorkerName: fmt.Sprintf("%s-%s-%s", h.Service().Name, strings.Replace(strings.ToLower(modelName), "/", "-", -1), strings.Replace(namesgenerator.GetRandomNameCDS(0), "_", "-", -1)),
WorkerName: generateWorkerName(h.Service().Name, false, modelName),
Model: j.model,
JobID: j.id,
Requirements: j.requirements,
Expand Down Expand Up @@ -225,3 +225,40 @@ func spawnWorkerForJob(ctx context.Context, h Interface, j workerStarterRequest)
}
return true // ok for this job
}

// a worker name must be 60 char max, without '.' and '_' -> replaced by '-'
func generateWorkerName(hatcheryName string, isRegister bool, model string) string {
prefix := ""
if isRegister {
prefix = "register-"
}

maxLength := 63
hName := strings.Replace(strings.ToLower(hatcheryName), "/", "-", -1) + "-"
modelName := strings.Replace(strings.ToLower(model), "/", "-", -1)
random := strings.Replace(namesgenerator.GetRandomNameCDS(0), "_", "-", -1)
workerName := strings.Replace(fmt.Sprintf("%s%s-%s-%s", prefix, hatcheryName, modelName, random), ".", "-", -1)

if len(workerName) <= maxLength {
return workerName
}
if len(hName) > 10 {
hName = ""
}
workerName = fmt.Sprintf("%s%s%s-%s", prefix, hName, modelName, random)
if len(workerName) <= maxLength {
return workerName
}
if len(modelName) > 15 {
modelName = modelName[:15]
}
workerName = fmt.Sprintf("%s%s%s-%s", prefix, hName, modelName, random)
if len(workerName) <= maxLength {
return workerName
}

if len(workerName) > maxLength {
return workerName[:maxLength]
}
return workerName
}
47 changes: 47 additions & 0 deletions sdk/hatchery/starter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package hatchery

import (
"strings"
"testing"
)

func Test_generateWorkerName(t *testing.T) {
type args struct {
hatcheryName string
isRegister bool
model string
}
tests := []struct {
name string
args args
want string
}{
{
name: "simple",
args: args{hatcheryName: "p999-prod", isRegister: true, model: "shared.infra-rust-official-1.41"},
want: "register-p999-prod-shared.infra-ru",
},
{
name: "long hatchery name",
args: args{hatcheryName: "p999-prod-xxxx-xxxx-xxxx-xxxx-xxxx", isRegister: true, model: "shared.infra-rust-official-1.41"},
want: "register-shared.infra-ru",
},
{
name: "long model name",
args: args{hatcheryName: "hname", isRegister: true, model: "shared.infra-rust-official-1.41-xxx-xxx-xxx-xxx"},
want: "register-hname-shared.infra-ru",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := generateWorkerName(tt.args.hatcheryName, tt.args.isRegister, tt.args.model)
if len(got) > 64 {
t.Errorf("len must be < 64() = %d - got:%s", len(got), got)
}

if !strings.HasPrefix(got, tt.want) {
t.Errorf("generateWorkerName() = %v, want prefix : %v", got, tt.want)
}
})
}
}

0 comments on commit c22cd8f

Please sign in to comment.