search

saltstack-formulas / vault-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
15 stars 59 forks source link

Fix systemd unit file, file backend, generating self-signed cert, upgrade process #22

Closed dafyddj closed 6 years ago

dafyddj commented 6 years ago

This is a bit of an overhaul of the formula, fixing many issues, incl. the two open issues #18 #19.

The main thrust of the changes are to add tests for running in production mode (which didn't work) and that the binary is upgraded on version change (also didn't work; manual test only due to limits in Test Kitchen, unfortunately). Generating a self-signed cert also didn't work.

Also, use many more Salt builtin states rather than cmd.runs, and tidy up requisites.

More information can be found in the individual commits.

johnkeates commented 6 years ago

I usually like different changes in different PRs but in this case it's a rather intertwined piece of housekeeping and splitting elements off makes not a whole lot of sense.

dafyddj commented 6 years ago

Yes, once I'd started it was hard to know where to stop, without leaving things still broken.

johnkeates commented 6 years ago

One of the things that make splitting this type of fixing difficult is the connection between CI and configuration; you can't test something that isn't there yet, and you can't make something without testing (well, you can, but then what's the point).

dafyddj commented 6 years ago

Not sure why, but Travis doesn't seem to be building/testing the PR.

myoung34 commented 6 years ago

@dafyddj My guess for travis is that its a side effect of the massive outage GH just experienced and is still experiencing: https://blog.github.com/2018-10-21-october21-incident-report/

johnkeates commented 6 years ago

Yeah, that's probably it. Travis isn't showing this PR at all. Webhooks etc. are suspended by GH so that makes sense.

myoung34 commented 6 years ago

Im mostly impressed that tests were added and it passed. im going to go ahead and merge it as is, and test it later since I need to upgrade our vault. if i find any issues ill make those additional changes in a new PR to go ahead and get progress started given your amount of effort.

Thanks for this!

dafyddj commented 6 years ago

That's great. Thanks