Pull missing docker images automatically#191
Conversation
d6d27d5 to
b3d6858
Compare
b3d6858 to
17b5afa
Compare
| out, err := exec.CommandContext(ctx, "docker", "image", "inspect", "--format", "{{.Id}}", "--", image).CombinedOutput() | ||
| if err != nil { | ||
| return "", fmt.Errorf("error inspecting docker image (try `docker pull %q` to fix this): %s", image, bytes.TrimSpace(out)) | ||
| // An error could mean the image just didn't exist, so we need to pull it first. |
There was a problem hiding this comment.
How do we determine that? What if the error is "docker: command does not exist"? We then log "Pulling image..." which would fail.
Is there are a way to determine whether we should pull the image or return the error?
There was a problem hiding this comment.
The logic
- tries to get the digest
- on fail, tries to pull
- tries to get the digest again
- on fail, it's not related to the pull and we return the error
There was a problem hiding this comment.
Yeah, I know that, but the "on fail" could be caused by anything, right?
If "tries to get the digest" fails because docker is not installed, then we run "docker pull" and print more stuff to stderr.
In other words: can't we check the output returned by "docker image inspect" to see if it contains "No such image" (or something) and only then try to pull?
| } | ||
| }() | ||
| errScanner := bufio.NewScanner(stderr) | ||
| go func() { |
There was a problem hiding this comment.
Instead of doing this whole dance with StdoutPipe/StderrPipe and the scanners, I think we can simplify this by moving some of that into the logger:
stdout := logger.InfoPipe()
pullCmd.Stdout = stdoutThe InfoPipe and WarnPipe methods could return a prefixed io.Writer that only logs if the verbose mode is turned on.
Take a look at this method to get some ideas:
src-cli/cmd/src/actions_exec_logger.go
Lines 144 to 160 in 1266471
|
I reworked the log streaming, PTAL :) |
mrnugget
left a comment
There was a problem hiding this comment.
Approving, but left some comments with suggestions.
Also, did you test it in different cases?
src action exec -f action-with-docker-images-that-cannot-be-pulled.jsonsrc action exec -f action-with-docker-images-that-can-be-pulled.jsonsrc -v action exec -f action-with-docker-images-that-cannot-be-pulled.jsonsrc -v action exec -f action-with-docker-images-that-can-be-pulled.json
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
# Conflicts: # cmd/src/actions_exec.go # cmd/src/actions_exec_logger.go
|
I finished this one up and seems to work well for me, can I get another review after I merged master into this branch? :) |
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
Closes #183