Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk): set worker name max length to 63 characters #6097

Merged
merged 1 commit into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
}
})
}
}