From 845efc5b44bc47b4b15116068451f0c6b6edf076 Mon Sep 17 00:00:00 2001 From: Richard LT Date: Thu, 24 Feb 2022 12:22:49 +0100 Subject: [PATCH] fix(sdk): set worker name max length to 63 characters (#6097) --- engine/api/event/event.go | 2 +- engine/config.go | 24 ++++---- .../hatchery/kubernetes/kubernetes_client.go | 2 +- engine/hatchery/vsphere/hatchery.go | 9 +-- sdk/hatchery/starter.go | 26 +-------- sdk/hatchery/starter_test.go | 52 ----------------- sdk/namesgenerator/namesgenerator.go | 26 +++++++-- sdk/namesgenerator/namesgenerator_test.go | 57 +++++++++++++++---- 8 files changed, 83 insertions(+), 115 deletions(-) delete mode 100644 sdk/hatchery/starter_test.go diff --git a/engine/api/event/event.go b/engine/api/event/event.go index e70961154f..ac1449c7a8 100644 --- a/engine/api/event/event.go +++ b/engine/api/event/event.go @@ -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 { diff --git a/engine/config.go b/engine/config.go index 54935f855d..b928dd23a1 100644 --- a/engine/config.go +++ b/engine/config.go @@ -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{} @@ -111,7 +111,7 @@ 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", @@ -119,12 +119,12 @@ func configBootstrap(args []string) Configuration { 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{} @@ -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{} diff --git a/engine/hatchery/kubernetes/kubernetes_client.go b/engine/hatchery/kubernetes/kubernetes_client.go index 71833ad13a..29ddba7f5a 100644 --- a/engine/hatchery/kubernetes/kubernetes_client.go +++ b/engine/hatchery/kubernetes/kubernetes_client.go @@ -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) } diff --git a/engine/hatchery/vsphere/hatchery.go b/engine/hatchery/vsphere/hatchery.go index 11479c1c92..72cf9edfd2 100644 --- a/engine/hatchery/vsphere/hatchery.go +++ b/engine/hatchery/vsphere/hatchery.go @@ -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 @@ -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) diff --git a/sdk/hatchery/starter.go b/sdk/hatchery/starter.go index f20850f3af..3ed5ea7b3b 100644 --- a/sdk/hatchery/starter.go +++ b/sdk/hatchery/starter.go @@ -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" ) @@ -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. @@ -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{ @@ -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) -} diff --git a/sdk/hatchery/starter_test.go b/sdk/hatchery/starter_test.go deleted file mode 100644 index e544afcbef..0000000000 --- a/sdk/hatchery/starter_test.go +++ /dev/null @@ -1,52 +0,0 @@ -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 - wantPrefix string - }{ - { - name: "simple", - args: args{hatcheryName: "p999-prod", isRegister: true, model: "rust-official-1.41"}, - wantPrefix: "register-rust-official-1-41-", - }, - { - name: "simple special char", - args: args{hatcheryName: "p999/prod", isRegister: true, model: "shared.infra-rust-official-1.41"}, - wantPrefix: "register-shared-infra-rust-official-1-41-", - }, - { - name: "long hatchery name", - args: args{hatcheryName: "p999-prod-xxxx-xxxx-xxxx-xxxx-xxxx", isRegister: true, model: "shared.infra-rust-official-1.41"}, - wantPrefix: "register-shared-infra-rust-official-1-41-", - }, - { - name: "long model name", - args: args{hatcheryName: "hname", isRegister: true, model: "shared.infra-rust-official-1.41-xxx-xxx-xxx-xxx"}, - wantPrefix: "register-shared-infra-rust-official-1-41-xxx-xxx-xxx-x-", - }, - } - 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.wantPrefix) { - t.Errorf("generateWorkerName() = %v, want prefix : %v", got, tt.wantPrefix) - } - }) - } -} diff --git a/sdk/namesgenerator/namesgenerator.go b/sdk/namesgenerator/namesgenerator.go index 2f9fb141fb..5fe083b6f3 100644 --- a/sdk/namesgenerator/namesgenerator.go +++ b/sdk/namesgenerator/namesgenerator.go @@ -6,6 +6,8 @@ import ( "fmt" "math/rand" "time" + + "github.com/ovh/cds/sdk/slug" ) func init() { @@ -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 { @@ -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 } @@ -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) +} diff --git a/sdk/namesgenerator/namesgenerator_test.go b/sdk/namesgenerator/namesgenerator_test.go index ec6f476333..718832b9b8 100644 --- a/sdk/namesgenerator/namesgenerator_test.go +++ b/sdk/namesgenerator/namesgenerator_test.go @@ -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") } @@ -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 @@ -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) + } + }) + } +}