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

Flag in jobfile.nomad to force pull docker images from registry #639

Closed
adrianlop opened this issue Dec 29, 2015 · 28 comments · Fixed by #2147
Closed

Flag in jobfile.nomad to force pull docker images from registry #639

adrianlop opened this issue Dec 29, 2015 · 28 comments · Fixed by #2147

Comments

@adrianlop
Copy link
Contributor

Hi,

I think it's very useful for staging environments to have a config flag like this one:

[...]
task "taskname" {
            driver = "docker"
            config {
                image = "docker-registry.foo.bar/foo/dockerimage:tag"
                force_pull = true
                port_map {
                    http = 8080
                }
[...]

By force_pull = true I mean that Nomad should always execute docker pull docker-registry.foo.bar/foo/dockerimage:tag before launching the docker image, because in test envs we always work with the 'latest' tag or a milestone tag before having a final docker image version (I think that's the easy way to go with Docker).

What do you think?
Thank you!

@c4milo
Copy link
Contributor

c4milo commented Jan 4, 2016

I hit this need as well, there are small fixes one would like to force push without generating a new tag. Being able to force pulls will allow redeploying the same image including the small fixes.

@adrianlop
Copy link
Contributor Author

ok, I think this is not neccesary since the docker driver has this feature:
docker.cleanup.image: Defaults to true. Changing this to false will prevent Nomad from removing images from stopped tasks.
It's working for me in the tests I made today, but I remember last week I had to run docker rmi 098ac9f6.
Looking at this code

// Cleanup image. This operation may fail if the image is in use by another
// job. That is OK. Will we log a message but continue.
if h.cleanupImage {
err = h.client.RemoveImage(h.imageID)
if err != nil {
containers, err := h.client.ListContainers(docker.ListContainersOptions{
// The image might be in use by a stopped container, so check everything
All: true,
Filters: map[string][]string{
"image": []string{h.imageID},
},
})
if err != nil {
h.logger.Printf("[ERR] driver.docker: failed to query list of containers matching image:%s", h.imageID)
return fmt.Errorf("Failed to query list of containers: %s", err)
}
inUse := len(containers)
if inUse > 0 {
h.logger.Printf("[INFO] driver.docker: image %s is still in use by %d container(s)", h.imageID, inUse)
} else {
return fmt.Errorf("Failed to remove image %s", h.imageID)
}
} else {
h.logger.Printf("[INFO] driver.docker: removed image %s", h.imageID)
}
}
, it kills the container, then removes container, then removes image.

I think what happened to me is that the container stopped but for some reason the docker engine couldn't delete the container, so the image couldn't be deleted neither.
Maybe the reason is that docker returns "OK, I stopped it" but the next step docker rm container happens so fast that it wasn't truly stopped yet?

@dadgar could you please confirm how this works?

thanks!!!

@dadgar
Copy link
Contributor

dadgar commented Jan 20, 2016

@adrianlop: What you described is correct!

@adrianlop
Copy link
Contributor Author

@dadgar thanks. what do you think about adding a delay between these operations?

@dadgar
Copy link
Contributor

dadgar commented Jan 21, 2016

Why would you want a delay? It may be easier to just support the force_pull, I don't think the UX of a delay is very nice.

@adrianlop
Copy link
Contributor Author

you're right, it's better that way.
thanks again man :D

@dadgar
Copy link
Contributor

dadgar commented Mar 22, 2016

Closing as when the tag is latest we check if the cached version is truly the latest.

@dadgar dadgar closed this as completed Mar 22, 2016
@c4milo
Copy link
Contributor

c4milo commented Mar 22, 2016

Nicely done, thanks Alex!

@adrianlop
Copy link
Contributor Author

great, thank you !

@sheldonkwok
Copy link
Contributor

I think that it should be possible to optionally check the cache version on other tags that aren't latest. We have an image tag on specific commits for production workflows but other tags for dev/testing that get updated very frequently on new commits. Latest is helpful for now but it ends up creating an image with the develop tag in the name instead of as an actual tag. Thanks!

@dadgar
Copy link
Contributor

dadgar commented Apr 7, 2016

@sheldonkwok good point!

@dadgar dadgar reopened this Apr 7, 2016
@marceldegraaf
Copy link

Note: if you don't supply a specific tag to Docker, it will assume you mean latest. However, it seems Nomad only force-updates the image when you explicitly add the latest tag to the image name (e.g. redis:latest, instead of just redis).

@iverberk
Copy link
Contributor

Hi Marcel, the code path for specifying and not specifying the latest tag is exactly the same (the tag will always be 'latest'): https://github.com/hashicorp/nomad/blob/master/client/driver/docker.go#L443-L464 . It should therefore always pull the image. Did you reproduce different behaviour?

@marceldegraaf
Copy link

Hm, in that case I'm mistaken. Sorry for adding confusion.

@iverberk
Copy link
Contributor

No problem, just checking!: -)

@skyrocknroll
Copy link

@dadgar We also use two tags . One for qa and one for prod. force_pull = true would be awesome. Is there any ETA ?

@camerondavison
Copy link
Contributor

I guess this didn't make it into 0.4.1?

@diptanu
Copy link
Contributor

diptanu commented Aug 18, 2016

@a86c6f7964 No it didn't. We didn't get time to pick this up.

@djenriquez
Copy link

Will this feature make 0.5.0?

@jippi
Copy link
Contributor

jippi commented Nov 2, 2016

It did not make the RC as far as I can see from the changelog, so next potential target would be 0.5.1. :)

@camerondavison
Copy link
Contributor

camerondavison commented Nov 2, 2016

Darn I didn't get to upgrade to 0.4.1 because I didn't have time to repatch
it again. Hopefully I can upgrade 0.5.0 and can repatch it again. Without
this feature we cannot deploy new images

@djenriquez
Copy link

Hmm bummer, yea, this was also a big one for me; but the workaround according to @dadgar here was to just add in a dummy variable, say a timestamp or something that will force a re-pull of latest.

@jrusiecki
Copy link

+1
Yes. I need it very much. I hope You'll make it soon into release. Regards

@dadgar dadgar modified the milestones: v0.5.3, 0.4.1 Nov 21, 2016
@t3hk0d3
Copy link

t3hk0d3 commented Nov 29, 2016

Is this issue open for PR? Anyone already doing it? Otherwise i'll bother to make PR myself. It looks supereasy.

@dadgar
Copy link
Contributor

dadgar commented Dec 1, 2016

@t3hk0d3 A PR would be appreciated!

@dadgar dadgar modified the milestones: v0.6.0, v0.5.5 Jan 3, 2017
@camerondavison
Copy link
Contributor

@dadgar is that pull request in line with what you would like to see? why did this get pushed out to 6.0?

@dadgar
Copy link
Contributor

dadgar commented Jan 9, 2017

@a86c6f7964 Nope I changed the milestone before I saw your PR. Will look at it today

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.