-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
Upgrade and Downgrade with url #118
Conversation
10d8191
to
99a8d11
Compare
Very similar to #114 , but I think this change is simpler Tested in our staging+production setups, works fine: can upgrade, downgrade and if the correct version is already installed it will not attempt to restart. We just upgrade the masters first (hiera-data for the consul-master-role set to the new consul version) and after that we upgrade all agents (setting the environment to the correnct consul version) |
@@ -10,7 +10,7 @@ | |||
$package_ensure = 'latest' | |||
$ui_package_name = 'consul_ui' | |||
$ui_package_ensure = 'latest' | |||
$version = '0.5.0' | |||
$version = hiera('consul::version', '0.5.0') # ensure defaults resolve from hiera as well |
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.
It is not advised to call Hiera from a Module ...
Best practices are to use the Role / Profile pattern to compose nodes definition and make Hiera calls from these classes which are business specific anyway .
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.
In our puppet setup we have various roles that use the Consul module. We don't want to write the Consul version in each of the roles (or their hiera counterpart), but want to set them once per environment (staging, production etc). The old code forces the user of the Consul module to explicitly pass the version for Consul to the module in order to create the correct download url, whereas the actual version in the init.pp would be set from hiera. This seems highly counter-intuitive to me. If I set consul::version in Hiera, I would expect that version to be downloaded and used. We don't have the concept of Profiles.
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'm also going to veto an explicit heira lookup here.
I don't understand why $consul::version
in hiera would not be picked up using the native hiera bindings?
It may be counter-intuitive to you, but this is the documented way that variable precedence takes place:
https://docs.puppetlabs.com/hiera/1/puppet.html#automatic-parameter-lookup
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.
$consul::version
works fine
but in init.pp we have defaults:
$download_url = "https://dl.bintray.com/mitchellh/consul/${version}_${os}_${arch}.zip",
$ui_download_url = "https://dl.bintray.com/mitchellh/consul/${version}_web_ui.zip",
These defaults apparently do not get populated by hiera. Eg if you have in hiera:
consul::version: 0.5.1
and in params.pp $version = '0.5.0'
this means your url will become https://dl.bintray.com/mitchellh/consul/0.5.0_web_ui.zip
instead of 0.5.1 (which is used in other places because init.pp:version is correctly resolved from hiera)
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.
Something must be weird with your hiera setup then.
We use hiera exactly like you mention:
Eg in hiera
consul::version: 0.5.1
and in params.pp
$version = '0.5.0'
and the download url becomes:
https://dl.bintray.com/mitchellh/consul/0.5.1_web_ui.zip
Edit:
Oh, do you mean those urls as params in init.pp?
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.
Oh, do you mean those urls as params in init.pp?
Not sure what you mean, but end-result is that if I only set consul::version in hiera, my download comes from the version-number as set in params. I'll test it again!
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.
Are we still blocked on this? Can we keep this the way it was (without hiera) and merge it?
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.
@thijsterlouw I think the problem you are experiencing with hiera is fixed here #129
This functionality should be handled now by #202 |
This allows users to upgrade (and downgrade) their consul version, even when using the 'url' install_method.
It also notifies the service, so it gets properly restarted.