puppet --noop fix has broken ensure=>latest #316
Assignees
Comments
|
This seems to be a damned if you do, damned if you don't situation. Without running the docker pull you cannot know that the latest image has changed. |
mleklund
changed the title
|
I haven't been able to replicate this issue. You can see from my logs below that a newer image with the tag latest was downloaded onto my test environment. |
|
Are you running with the patch from #297, i.e. master? I do not see your code running the unless. I do not see how the unless check I realize I am running a non-release version, but am doing that because of the upstart bug from #304. |
|
@mleklund you're correct in that I was running the code from the forge, and not what was in master and I can replicate your issue. I am not aware of a test that is going to be simple, without having to introduce more params and logic and using things like curl to get information out of the registry. I'll open this up for discussion internally as well but it's possible we may have to consider a breaking change in the next release as using the latest tag is generally considered a poor practice, no matter how practical it may be in dev scenarios. We will however be happy to review a PR that can cleanly address the issue. |
|
Closing as we have acknowledged this as a breaking change. We are happy to review PR's that attempt to address the issue without adding too much complexity to the use of the module |
|
@davejrt Is there a reason that the |
|
|
|
@davejrt I meant if the metaparameter See https://puppet.com/docs/puppet/6.0/metaparameter.html#noop What I was wondering was there a particular reason that use of this metaparameter on the exec resource definition instead of removing the behaviour wasn't a valid option for this use case? |
|
I'm not sure if this was tested and didn't work, but we will be open to reviewing any PR's that address the issue. |
|
@davejrt good to know, I'll have a look at putting something together if someone else doesn't get to it first, just wanted to check there wasn't an obvious reason it wasn't used. |
electrofelix
added a commit
to electrofelix/puppetlabs-docker
that referenced
this issue
Apr 3, 2019
Restore the code to update images tagged with 'latest' on each run, moving this to be guarded by a refreshonly meta-parameter that is triggered by a notify resource that is disabled for 'noop'. Because the original code to update images was run via 'onlyif' to indicate if the image was in sync by performing a pull and testing the new digest against the old, this meant that simply applying 'noop' to this exec resource would not work. 'noop' does not affect running the components of a resource to validate if it is in-sync, which is the purpose of the 'onlyif' & 'unless' settings for an exec. To support 'noop' mode, previously the code to update the image on each run was dropped to prevent it accidentally refreshing the local image and then skipping an image restart because there was no update event on a subsequent real run. To resolve, it is necessary to have a prior resource that is used as a proxy, and only when it notifies the image update resource, will the 'onlyif' action be tested to determine if in sync. This prior resource can be disabled from a noop run, and subsequently although puppet records that it would send a refresh event in 'noop' mode, none are actually sent during a 'noop' run thus preventing the image from being updated by a side effect of checking if the notified exec was in-sync. Fixes puppetlabs#316
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What you expected to happen?
I expect docker::image to pull the latest image and update containers when ensure => 'latest' is set.
What happened?
Images are not being updated on linux. This looks to be introduced along with the #297 fix. The unless argument will always return 0 for latest image, so
/usr/local/bin/update_docker_image.shwill never be run.How to reproduce it?
Set ensure => latest on linux and watch nothing happen.
Anything else we need to know?
Versions:
Logs:
The text was updated successfully, but these errors were encountered: