Skip to content

Commit

Permalink
fix(sdk): set worker name max length to 63 characters (#6097)
Browse files Browse the repository at this point in the history
  • Loading branch information
richardlt authored Feb 24, 2022
1 parent e0def48 commit 845efc5
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 115 deletions.
2 changes: 1 addition & 1 deletion engine/api/event/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func Initialize(ctx context.Context, db *gorp.DbMap, cache Store, config *Config
}
// generates an API name. api_foo_bar, only 3 first letters to have a readable status
cdsname = "api_"
for _, v := range strings.Split(namesgenerator.GetRandomNameCDS(0), "_") {
for _, v := range strings.Split(namesgenerator.GetRandomNameCDS(), "_") {
if len(v) > 3 {
cdsname += v[:3]
} else {
Expand Down
24 changes: 12 additions & 12 deletions engine/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,41 +67,41 @@ func configBootstrap(args []string) Configuration {
switch a {
case sdk.TypeAPI:
conf.API = &api.Configuration{}
conf.API.Name = "cds-api-" + namesgenerator.GetRandomNameCDS(0)
conf.API.Name = "cds-api-" + namesgenerator.GetRandomNameCDS()
defaults.SetDefaults(conf.API)
conf.API.Database.Schema = "public"
conf.API.HTTP.Port = 8081
case sdk.TypeUI:
conf.UI = &ui.Configuration{}
conf.UI.Name = "cds-ui-" + namesgenerator.GetRandomNameCDS(0)
conf.UI.Name = "cds-ui-" + namesgenerator.GetRandomNameCDS()
defaults.SetDefaults(conf.UI)
conf.UI.HTTP.Port = 8080
case "migrate":
conf.DatabaseMigrate = &migrateservice.Configuration{}
defaults.SetDefaults(conf.DatabaseMigrate)
conf.DatabaseMigrate.Name = "cds-migrate-" + namesgenerator.GetRandomNameCDS(0)
conf.DatabaseMigrate.Name = "cds-migrate-" + namesgenerator.GetRandomNameCDS()
conf.DatabaseMigrate.ServiceAPI.DB.Schema = "public"
conf.DatabaseMigrate.ServiceCDN.DB.Schema = "cdn"
conf.DatabaseMigrate.HTTP.Port = 8087
case sdk.TypeHatchery + ":local":
conf.Hatchery.Local = &local.HatcheryConfiguration{}
defaults.SetDefaults(conf.Hatchery.Local)
conf.Hatchery.Local.Name = "cds-hatchery-local-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.Local.Name = "cds-hatchery-local-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.Local.HTTP.Port = 8086
case sdk.TypeHatchery + ":kubernetes":
conf.Hatchery.Kubernetes = &kubernetes.HatcheryConfiguration{}
defaults.SetDefaults(conf.Hatchery.Kubernetes)
conf.Hatchery.Kubernetes.Name = "cds-hatchery-kubernetes-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.Kubernetes.Name = "cds-hatchery-kubernetes-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.Kubernetes.HTTP.Port = 8086
case sdk.TypeHatchery + ":marathon":
conf.Hatchery.Marathon = &marathon.HatcheryConfiguration{}
defaults.SetDefaults(conf.Hatchery.Marathon)
conf.Hatchery.Marathon.Name = "cds-hatchery-marathon-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.Marathon.Name = "cds-hatchery-marathon-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.Marathon.HTTP.Port = 8086
case sdk.TypeHatchery + ":openstack":
conf.Hatchery.Openstack = &openstack.HatcheryConfiguration{}
defaults.SetDefaults(conf.Hatchery.Openstack)
conf.Hatchery.Openstack.Name = "cds-hatchery-openstack-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.Openstack.Name = "cds-hatchery-openstack-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.Openstack.HTTP.Port = 8086
case sdk.TypeHatchery + ":swarm":
conf.Hatchery.Swarm = &swarm.HatcheryConfiguration{}
Expand All @@ -111,20 +111,20 @@ func configBootstrap(args []string) Configuration {
Host: "unix:///var/run/docker.sock",
},
}
conf.Hatchery.Swarm.Name = "cds-hatchery-swarm-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.Swarm.Name = "cds-hatchery-swarm-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.Swarm.HTTP.Port = 8086
conf.Hatchery.Swarm.RegistryCredentials = []swarm.RegistryCredential{{
Domain: "docker.io",
}}
case sdk.TypeHatchery + ":vsphere":
conf.Hatchery.VSphere = &vsphere.HatcheryConfiguration{}
defaults.SetDefaults(conf.Hatchery.VSphere)
conf.Hatchery.VSphere.Name = "cds-hatchery-vsphere-" + namesgenerator.GetRandomNameCDS(0)
conf.Hatchery.VSphere.Name = "cds-hatchery-vsphere-" + namesgenerator.GetRandomNameCDS()
conf.Hatchery.VSphere.HTTP.Port = 8086
case sdk.TypeHooks:
conf.Hooks = &hooks.Configuration{}
defaults.SetDefaults(conf.Hooks)
conf.Hooks.Name = "cds-hooks-" + namesgenerator.GetRandomNameCDS(0)
conf.Hooks.Name = "cds-hooks-" + namesgenerator.GetRandomNameCDS()
conf.Hooks.HTTP.Port = 8083
case sdk.TypeVCS:
conf.VCS = &vcs.Configuration{}
Expand All @@ -146,12 +146,12 @@ func configBootstrap(args []string) Configuration {
"gitlab": {URL: "https://gitlab.com", Gitlab: &gitlab},
"gerrit": {URL: "http://localhost:8080", Gerrit: &gerrit},
}
conf.VCS.Name = "cds-vcs-" + namesgenerator.GetRandomNameCDS(0)
conf.VCS.Name = "cds-vcs-" + namesgenerator.GetRandomNameCDS()
conf.VCS.HTTP.Port = 8084
case sdk.TypeRepositories:
conf.Repositories = &repositories.Configuration{}
defaults.SetDefaults(conf.Repositories)
conf.Repositories.Name = "cds-repositories-" + namesgenerator.GetRandomNameCDS(0)
conf.Repositories.Name = "cds-repositories-" + namesgenerator.GetRandomNameCDS()
conf.Repositories.HTTP.Port = 8085
case sdk.TypeCDN:
conf.CDN = &cdn.Configuration{}
Expand Down
2 changes: 1 addition & 1 deletion engine/hatchery/kubernetes/kubernetes_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (k *kubernetesClient) PodDelete(ctx context.Context, ns string, name string

func (k *kubernetesClient) PodList(ctx context.Context, ns string, opts metav1.ListOptions) (*corev1.PodList, error) {
ctx = context.WithValue(ctx, logNS, ns)
log.Info(ctx, "listing pod")
log.Info(ctx, "listing pod in namespace %s", ns)
pods, err := k.client.CoreV1().Pods(ns).List(ctx, opts)
return pods, sdk.WrapError(err, "unable to list pods in namespace %s", ns)
}
Expand Down
9 changes: 1 addition & 8 deletions engine/hatchery/vsphere/hatchery.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/ovh/cds/sdk/cdsclient"
"github.com/ovh/cds/sdk/hatchery"
"github.com/ovh/cds/sdk/namesgenerator"
"github.com/ovh/cds/sdk/slug"
)

// New instanciates a new Hatchery vsphere
Expand Down Expand Up @@ -416,13 +415,7 @@ func (h *HatcheryVSphere) provisioning(ctx context.Context) {
log.Info(ctx, "model %q provisioning: %d/%d", modelPath, mapAlreadyProvisionned[modelPath], number)

for i := 0; i < int(number)-mapAlreadyProvisionned[modelPath]; i++ {
var nameFirstPart = "provision-" + modelPath
if len(nameFirstPart) > maxLength-10 {
nameFirstPart = nameFirstPart[:maxLength-10]
}
var remainingLength = maxLength - len(nameFirstPart) - 1
random := namesgenerator.GetRandomNameCDSWithMaxLength(remainingLength)
workerName := slug.Convert(fmt.Sprintf("%s-%s", nameFirstPart, random))
workerName := namesgenerator.GenerateWorkerName(modelPath, "provision")

h.cacheProvisioning.mu.Lock()
h.cacheProvisioning.pending = append(h.cacheProvisioning.pending, workerName)
Expand Down
26 changes: 2 additions & 24 deletions sdk/hatchery/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/ovh/cds/sdk"
cdslog "github.com/ovh/cds/sdk/log"
"github.com/ovh/cds/sdk/namesgenerator"
"github.com/ovh/cds/sdk/slug"
"github.com/ovh/cds/sdk/telemetry"
"github.com/rockbears/log"
)
Expand Down Expand Up @@ -58,7 +57,7 @@ func workerStarter(ctx context.Context, h Interface, workerNum string, jobs <-ch
continue
}

workerName := generateWorkerName(h.Service().Name, true, m.Name)
workerName := namesgenerator.GenerateWorkerName(m.Name, "register")

atomic.AddInt64(&nbWorkerToStart, 1)
// increment nbRegisteringWorkerModels, but no decrement.
Expand Down Expand Up @@ -168,7 +167,7 @@ func spawnWorkerForJob(ctx context.Context, h Interface, j workerStarterRequest)
})
next()

workerName := generateWorkerName(h.Service().Name, false, modelName)
workerName := namesgenerator.GenerateWorkerName(modelName, "")

ctxSpawnWorker, next := telemetry.Span(ctx, "hatchery.SpawnWorker", telemetry.Tag(telemetry.TagWorker, workerName))
arg := SpawnArguments{
Expand Down Expand Up @@ -226,24 +225,3 @@ 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 '-'
const maxLength = 64

func generateWorkerName(hatcheryName string, isRegister bool, modelName string) string {
prefix := ""
if isRegister {
prefix = "register-"
}

var nameFirstPart = fmt.Sprintf("%s%s", prefix, modelName)
if len(nameFirstPart) > maxLength-10 {
nameFirstPart = nameFirstPart[:maxLength-10]
}
var remainingLength = maxLength - len(nameFirstPart) - 1

random := namesgenerator.GetRandomNameCDSWithMaxLength(remainingLength)
workerName := fmt.Sprintf("%s-%s", nameFirstPart, random)

return slug.Convert(workerName)
}
52 changes: 0 additions & 52 deletions sdk/hatchery/starter_test.go

This file was deleted.

26 changes: 20 additions & 6 deletions sdk/namesgenerator/namesgenerator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"math/rand"
"time"

"github.com/ovh/cds/sdk/slug"
)

func init() {
Expand Down Expand Up @@ -847,7 +849,7 @@ begin:
//

// GetRandomNameCDS generates a random name from the list of adjectives and surnames in this package
func GetRandomNameCDS(retry int) string {
func GetRandomNameCDS() string {
format := rand.Intn(3)
var name string
switch format {
Expand All @@ -858,10 +860,6 @@ func GetRandomNameCDS(retry int) string {
default:
name = fmt.Sprintf("%s_%s", left[rand.Intn(len(left))], right[rand.Intn(len(right))])
}

if retry > 0 {
name = fmt.Sprintf("%s%d", name, rand.Intn(10))
}
return name
}

Expand All @@ -874,9 +872,25 @@ func GetRandomNameCDSWithMaxLength(maxLength int) string {
return s[:maxLength]
}
for {
var s = GetRandomNameCDS(0)
var s = GetRandomNameCDS()
if len(s) <= maxLength {
return s
}
}
}

// A worker name must be 63 char max, without '.' and '_', "/" -> replaced by '-'
const maxWorkerNameLength = 63

func GenerateWorkerName(model, prefix string) string {
nameFirstPart := model
if len(prefix) > 0 {
nameFirstPart = prefix + "-" + model
}
if len(nameFirstPart) > maxWorkerNameLength-10 {
nameFirstPart = nameFirstPart[:maxWorkerNameLength-10]
}
remainingLength := maxWorkerNameLength - len(nameFirstPart) - 1
random := GetRandomNameCDSWithMaxLength(remainingLength)
return slug.Convert(nameFirstPart + "-" + random)
}
57 changes: 46 additions & 11 deletions sdk/namesgenerator/namesgenerator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

func TestNameFormat(t *testing.T) {
name := GetRandomNameCDS(0)
name := GetRandomNameCDS()
if !strings.Contains(name, "_") {
t.Fatalf("Generated name does not contain an underscore")
}
Expand All @@ -16,16 +16,6 @@ func TestNameFormat(t *testing.T) {
}
}

func TestNameRetries(t *testing.T) {
name := GetRandomNameCDS(1)
if !strings.Contains(name, "_") {
t.Fatalf("Generated name does not contain an underscore")
}
if !strings.ContainsAny(name, "0123456789") {
t.Fatalf("Generated name doesn't contain a number")
}
}

func TestGetRandomNameCDSWithMaxLength(t *testing.T) {
type args struct {
maxLength int
Expand Down Expand Up @@ -57,3 +47,48 @@ func TestGetRandomNameCDSWithMaxLength(t *testing.T) {
})
}
}

func Test_GenerateWorkerName(t *testing.T) {
type args struct {
prefix string
model string
}
tests := []struct {
name string
args args
wantPrefix string
}{
{
name: "simple",
args: args{prefix: "register", model: "rust-official-1.41"},
wantPrefix: "register-rust-official-1-41",
},
{
name: "simple special char",
args: args{prefix: "register", model: "shared.infra-rust-official-1.41"},
wantPrefix: "register-shared-infra-rust-official-1-41",
},
{
name: "long hatchery name",
args: args{prefix: "register", model: "shared.infra-rust-official-1.41"},
wantPrefix: "register-shared-infra-rust-official-1-41",
},
{
name: "long model name",
args: args{prefix: "register", model: "shared.infra-rust-official-1.41-xxx-xxx-xxx-xxx"},
wantPrefix: "register-shared-infra-rust-official-1-41-xxx-xxx-xxx",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := GenerateWorkerName(tt.args.model, tt.args.prefix)
if len(got) > 63 {
t.Errorf("len must be < 63() = %d - got: %s", len(got), got)
}

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

0 comments on commit 845efc5

Please sign in to comment.