-
Notifications
You must be signed in to change notification settings - Fork 654
Use recently built image instead of latest for src #2649
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
base: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| func (c *apiClient) LocalImageID(ctx context.Context, ref string) (string, error) { | ||
| console.Debugf("=== APIClient.LocalImageTag %s", ref) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was expecting this debug line to say APIClient.LocalImageID 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found a way to run this that produces the incorrect hash message 🤔
| func BuildAddLabelsAndSchemaToImage(ctx context.Context, dockerClient command.Command, image string, labels map[string]string, bundledSchemaFile string, progressOutput string) error { | ||
| dockerfile := "FROM " + image + "\n" | ||
| dockerfile += "COPY " + bundledSchemaFile + " .cog\n" | ||
| if strings.HasPrefix("image", "r8.im") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean?
| if strings.HasPrefix("image", "r8.im") { | |
| if strings.HasPrefix(image , "r8.im") { |
Summary
We have been investigating a caching issue that has lead to versions of cog including an old version of the
srcfolder when runningcog buildorcog pushon a model that already has a version. We are fairly sure that in certain circumstances, when theFROM r8.im/<username>/<model-name>command is run, it is pulling the latest from the upstream registry instead of using the recently built image from the local machine. To disambiguate, we should directly pass the SHA of the image on the local machine instead of letting docker just look forlatest, as it would do by default.Test Plan
A script,
./script/test-e2e-rebuild, was introduced, that simply makes a small change to thepredict.pyfunction and tries to rebuild the image. It then checks that the hash of thepredict.pybuilt into the docker image is the same as the one in thepredict.pysource directory for building the image.To verify the correctness of this change, build
cogfrom source on themainbranch. Then checkout this branch and run the test. The exit code of the script should be 1. Rebuildcogfrom this branch, and re-run the test. The exit code of the script should be 0.