Updating staging file download to use the version and symlink

[hopperd]

This is the same approach used by consul-template to allow for the version of consul to be upgraded/downgraded accordingly by downloading the artifact zip to a versioned folder and just symlinking the binary to the appropriate one.

This is also similar to how homebrew on Mac works as well.

I'm sure there are a few pieces missing, like service restart and such is probably desired to have on there as well, just didn't want to dive too deep if we didn't think this is a viable solution.

[hopperd]

@solarkennedy and @aj-jester can you guys give some thoughts on this?

[aj-jester]

@Split3 hey, first of all, great solution! so simple than an of the others 😃

One thing I'm noticing is even though consul -v shows the correct version, the rest of the cluster still sees the older version (0.5.2 or whatever the previous version was). I think we need to add notify => $consul::notify_service to the file resource "${consul::bin_dir}/consul" so consul gets restarted whenever the symlink gets updated. Also can we add a test for this? 😁

Can you verify this behavior on your end as well?

Before upgrade:

# consul -v
Consul v0.5.2
Consul Protocol: 2 (Understands back to: 1)

# consul members
Node        Address             Status  Type    Build  Protocol  DC
v-xhpez1yg  192.168.50.71:8301  alive   server  0.5.2  2         development

After upgrade:

# consul -v
Consul v0.6.0
Consul Protocol: 3 (Understands back to: 1)

# consul members
Node        Address             Status  Type    Build  Protocol  DC
v-xhpez1yg  192.168.50.71:8301  alive   server  0.5.2  2         development

After restart:

# consul members
Node        Address             Status  Type    Build  Protocol  DC
v-xhpez1yg  192.168.50.71:8301  alive   server  0.6.0  2         development

[hopperd]

Thanks @aj-jester, yes we will need the notify to restart accordingly as I mentioned in the first comment, just wanted to get us all to take a first look at it to ensure it was something we'd consider before fine tuning all the pieces together.

So yes I also verified that the notify is needed, I can add that in now.

As for tests, we currently already test that the proper file is downloaded, so we could also add the necessary test for the symlink, are there any others that would be desired?

[hopperd]

@aj-jester Tests added for the file along with ensuring that it should notify

[aj-jester]

@Split3 ah! didn't see the updated comment 😊

From my side this looks good!

🚄 LGTM

[hopperd]

@solarkennedy you ok to merge this in?

[hopperd]

@aj-jester are you ok to merge this in or should we wait for @solarkennedy to provide some feedback?

[florian-zeidler]

@Split3 When I use this patch on CentOS 6.1 Vagrant with Puppet 3.8.4 I get following warning:

==> default: Warning: Scope(Class[Consul::Install]): Could not look up qualified variable '::staging::path'; class ::staging has not been evaluated

It seems that you did forget to include ::staging.
Besides that, the change works fine.

[aj-jester]

@Split3 I'm ok to merge this in. Of course if @solarkennedy has any feedback we can always open up another PR 😄

[hopperd]

@das-vinculum hmm, I didn't get that warning, but yes I can include that as well, let me add that in.

[hopperd]

@das-vinculum change is in if you'd like to verify on your side

[solarkennedy]

Thanks for this. I also am not really sure where best to do the notify_service logic. I kinda like to have that stuff in the init.pp so anything in the install class can notify the service (or not). This wfm.

[aj-jester]

@solarkennedy thats a good idea.

[florian-zeidler]

@Split3 The warning is now gone, thank you very much.