Skip to content
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
1 change: 1 addition & 0 deletions .nextchanges/air/container-name-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Align multi-image container name validation with backend services: allow hyphens and up to 32 characters, and reject uppercase letters and underscores. ([#7154](https://github.com/databricks/cli/pull/7154))
8 changes: 4 additions & 4 deletions cmd/air/runconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var uuidRe = regexp.MustCompile(`^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[
// workspace checks performed by the backend.
var unityCatalogImageRe = regexp.MustCompile(`^[^.:\s]+\.[^.:\s]+\.[^.:\s]+:[^:\s]+$`)

var containerNameRe = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
var containerNameRe = regexp.MustCompile(`^[a-z]([-a-z0-9]{0,30}[a-z0-9])?$`)

var secretTemplateRe = regexp.MustCompile(`^\{\{secrets/([^/{}]+)/([^/{}]+)\}\}$`)

Expand Down Expand Up @@ -228,7 +228,7 @@ func (c *runConfig) validate() error {
// staged beside it because PuPr workspaces cannot consume the DEVELOPMENT API
// field yet.
type containerConfig struct {
Name string `yaml:"name" help:"Unique container name: at most 16 letters, digits, or underscores, not starting with a digit." required:"yes"`
Name string `yaml:"name" help:"Unique container name: 1–32 lowercase letters, digits, or hyphens; start with a letter and end with a letter or digit." required:"yes"`
Command *string `yaml:"command" help:"Shell command run on every rank assigned to this container." required:"yes"`
Ranks []int `yaml:"ranks" help:"Zero-based node ranks assigned to this container." required:"yes"`
UnityCatalogImage string `yaml:"unity_catalog_image" help:"Unity Catalog image as <catalog>.<schema>.<image>:<tag>." required:"yes"`
Expand Down Expand Up @@ -256,8 +256,8 @@ func (c *runConfig) validateContainers() error {
for i := range c.Containers {
container := &c.Containers[i]
prefix := fmt.Sprintf("containers[%d]", i)
if !containerNameRe.MatchString(container.Name) || len(container.Name) > 16 {
return fmt.Errorf("%s.name must be at most 16 letters, digits, or underscores and must not start with a digit, got %q", prefix, container.Name)
if !containerNameRe.MatchString(container.Name) {
return fmt.Errorf("%s.name must start with a lowercase letter, contain only lowercase letters, digits, or '-', end with a letter or digit, and be at most 32 characters, got %q", prefix, container.Name)
}
if _, exists := names[container.Name]; exists {
return fmt.Errorf("%s.name %q is duplicated; container names must be unique", prefix, container.Name)
Expand Down
44 changes: 44 additions & 0 deletions cmd/air/runconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,50 @@ func TestMultiImageExample(t *testing.T) {
assert.Len(t, cfg.Containers, 2)
}

func TestLoadRunConfigContainerNames(t *testing.T) {
for _, tc := range []struct {
name string
valid bool
}{
{"a", true},
{"trainer1", true},
{"data-proc", true},
{"a--b", true},
{strings.Repeat("a", 17), true},
{strings.Repeat("a", 32), true},
{"", false},
{"Trainer", false},
{"data_proc", false},
{"_trainer", false},
{"1trainer", false},
{"-trainer", false},
{"trainer-", false},
{"data.proc", false},
{"data proc", false},
{strings.Repeat("a", 33), false},
} {
t.Run(tc.name, func(t *testing.T) {
cfg, err := loadRunConfig(writeConfig(t, fmt.Sprintf(`
experiment_name: roles
compute:
accelerator_type: GPU_1xH100
num_accelerators: 1
containers:
- name: %q
command: echo train
ranks: [0]
unity_catalog_image: main.ml.trainer:v1
`, tc.name)))
if tc.valid {
require.NoError(t, err)
assert.Equal(t, tc.name, cfg.Containers[0].Name)
} else {
require.ErrorContains(t, err, "containers[0].name must start with a lowercase letter")
}
})
}
}

func TestLoadRunConfigContainerValidation(t *testing.T) {
base := `
experiment_name: roles
Expand Down
Loading