Skip to content

Commit

Permalink
fix(image-builder): Refactor model image naming convention (#526)
Browse files Browse the repository at this point in the history
# Description
This fix/enhancement revises the image naming convention for both model
and batch job images built by the image builder to make them consistent
with the convention used in Turing:

### Before
```
{dockerRegistry}/{projectName}-{modelName}:{versionID}
```

### After
```
{dockerRegistry}/{projectName}/pyfunc-models/{modelName}:{versionID}
```
or 
```
{dockerRegistry}/{projectName}/batch-jobs/{modelName}:{versionID}
```

**_This PR contains changes similar to what is implemented for Turing in
caraml-dev/turing#368

# Modifications
- `api/pkg/imagebuilder/model_service_imagebuilder.go` - Change in
template string for the pyfunc model service image name
- `api/pkg/imagebuilder/prediction_job_imagebuilder.go` - Change in
template string for the batch job image name

# Tests
- [x] Deploying PyFunc models

# Checklist
- [x] Added PR label
- [x] Added unit test, integration, and/or e2e tests
- [x] Tested locally
- [ ] Updated documentation
- [ ] Update Swagger spec if the PR introduce API changes
- [ ] Regenerated Golang and Python client if the PR introduces API
changes

# Release Notes
```release-note
NONE
```
  • Loading branch information
deadlycoconuts authored Feb 15, 2024
1 parent e0964c2 commit 6838844
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 21 deletions.
38 changes: 19 additions & 19 deletions api/pkg/imagebuilder/imagebuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -391,7 +391,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: config,
},
{
Expand Down Expand Up @@ -453,7 +453,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -482,7 +482,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: configWithSa,
},
{
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -587,7 +587,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: Config{
BuildNamespace: testBuildNamespace,
BaseImage: cfg.BaseImageConfig{
Expand Down Expand Up @@ -670,7 +670,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -718,7 +718,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: Config{
BuildNamespace: testBuildNamespace,
BaseImage: cfg.BaseImageConfig{
Expand Down Expand Up @@ -806,7 +806,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
"--cache=true",
"--compressed-caching=false",
"--snapshot-mode=redo",
Expand Down Expand Up @@ -856,7 +856,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: Config{
BuildNamespace: config.BuildNamespace,
BaseImage: cfg.BaseImageConfig{
Expand Down Expand Up @@ -936,7 +936,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -987,7 +987,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantCreateJob: nil,
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: config,
},
{
Expand Down Expand Up @@ -1048,7 +1048,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -1102,7 +1102,7 @@ func TestBuildImage(t *testing.T) {
},
wantCreateJob: nil,
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: config,
},
{
Expand Down Expand Up @@ -1163,7 +1163,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -1266,7 +1266,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -1317,7 +1317,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: fmt.Sprintf("%s-%s-%s", project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: config,
},
{
Expand Down Expand Up @@ -1383,7 +1383,7 @@ func TestBuildImage(t *testing.T) {
fmt.Sprintf("--build-arg=BASE_IMAGE=%s", config.BaseImage.ImageName),
fmt.Sprintf("--build-arg=MODEL_DEPENDENCIES_URL=%s", modelDependenciesURL),
fmt.Sprintf("--build-arg=MODEL_ARTIFACTS_URL=%s/model", testArtifactURI),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--destination=%s", fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID)),
fmt.Sprintf("--context-sub-path=%s", config.BaseImage.BuildContextSubPath),
"--cache=true",
"--compressed-caching=false",
Expand Down Expand Up @@ -1434,7 +1434,7 @@ func TestBuildImage(t *testing.T) {
Status: batchv1.JobStatus{},
},
wantDeleteJobName: "",
wantImageRef: fmt.Sprintf("%s/%s-%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
wantImageRef: fmt.Sprintf("%s/%s/pyfunc-models/%s:%s", config.DockerRegistry, project.Name, model.Name, modelVersion.ID),
config: config,
},
}
Expand Down
2 changes: 1 addition & 1 deletion api/pkg/imagebuilder/model_service_imagebuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ func (n *modelServiceNameGenerator) generateBuilderJobName(project mlp.Project,

// generateDockerImageName generate docker image name of model service
func (n *modelServiceNameGenerator) generateDockerImageName(project mlp.Project, model *models.Model) string {
return fmt.Sprintf("%s/%s-%s", n.dockerRegistry, project.Name, model.Name)
return fmt.Sprintf("%s/%s/pyfunc-models/%s", n.dockerRegistry, project.Name, model.Name)
}
2 changes: 1 addition & 1 deletion api/pkg/imagebuilder/prediction_job_imagebuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ func (n *predictionJobNameGenerator) generateBuilderJobName(project mlp.Project,

// generateDockerImageName generate the name of docker image of prediction job that will be created from given model
func (n *predictionJobNameGenerator) generateDockerImageName(project mlp.Project, model *models.Model) string {
return fmt.Sprintf("%s/%s-%s-job", n.dockerRegistry, project.Name, model.Name)
return fmt.Sprintf("%s/%s/batch-jobs/%s", n.dockerRegistry, project.Name, model.Name)
}

0 comments on commit 6838844

Please sign in to comment.