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

[CLOUD-1981] docker image update script runs in noop mode #297

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

mihaibuzgau
Copy link
Contributor

The --noop flag tells Puppet to determine which resources are out of sync, and to report them without actually synchronizing them. The 'onlyif' and 'unless' commands of an Exec are used in the process of determining whether the Exec is already in sync, therefore they must be run during a --noop Puppet run.

This PR changes the 'onlyif' command for ensure == 'latest' or $image_tab == 'latest' from a script that takes action (downloads images) to a command that only check the state.

Fixes: #241

@davejrt
Copy link
Contributor

davejrt commented Jul 25, 2018

LGTM

@davejrt davejrt merged commit 359b7a6 into puppetlabs:master Jul 25, 2018
@coder-hugo
Copy link
Contributor

This change makes the value latest for the ensure of a docker::image useless since the $image_install is only executed if the image in not already installed (unless => $_image_find). So you will never get a never version of the specified installed unless you change the image_tag

@davejrt
Copy link
Contributor

davejrt commented Jan 8, 2019

@coder-hugo we are aware this was a breaking change. This was reflected in the documentation as well as in the version numbers, and we discussed are reasons in issue #316.

@coder-hugo
Copy link
Contributor

IMHO such a breaking change should be noted within the changelog and not somewhere hidden in the README. I'd expect that most user will just have a look to the changelog before updating a module and they will not read through the whole README to find some breaking changes.

@davejrt
Copy link
Contributor

davejrt commented Jan 9, 2019

We'll take your feedback on board for any future breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants