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(image-builder):Refactor ensembler image naming convention #368

Conversation

deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Jan 30, 2024

Context

This fix/enhancement revises the image naming convention for both ensembler service and ensembler job images built by the image builder:

Before

{dockerRegistry}/{projectName}-{ensemblerName}-{runID}-service:{ensemblerID}

or

{dockerRegistry}/{projectName}-{ensemblerName}-{runID}-job:{ensemblerID}

After

{dockerRegistry}/{projectName}/ensembler-services/{ensemblerName}:{runID}

or

{dockerRegistry}/{projectName}/ensembler-jobs/{ensemblerName}:{runID}

By removing the runID from the image name, we now allow all images built for the same ensembler to be stored within the same image repository as opposed to storing them in separate repositories. Furthermore, since all built images of the same ensembler will now be stored in the same repository, we now tag them with their specific runID-s in order to distinguish between different versions of the same ensembler.

This PR contains changes similar to what is implemented for Merlin in caraml-dev/merlin#526.

Modifications

  • api/turing/imagebuilder/ensembler.go - Change in template string for the ensembler image name
  • api/turing/imagebuilder/imagebuilder.go - Change of image tag from ensemblerID to runID

@deadlycoconuts deadlycoconuts added the enhancement New feature or request label Jan 30, 2024
@deadlycoconuts deadlycoconuts self-assigned this Jan 30, 2024
@deadlycoconuts deadlycoconuts marked this pull request as ready for review February 1, 2024 09:48
deadlycoconuts added a commit to caraml-dev/merlin that referenced this pull request Feb 15, 2024
# 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
```
Copy link
Contributor

@leonlnj leonlnj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@deadlycoconuts deadlycoconuts merged commit b4ff131 into caraml-dev:main Feb 15, 2024
12 checks passed
@deadlycoconuts deadlycoconuts deleted the fix_turing_ensembler_docker_image_name branch May 7, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants