cloudfoundry-community / vault-boshrelease

Apache License 2.0
28 stars 35 forks source link

Minor changes to 1.0.0 release #60

Closed ywei2017 closed 6 years ago

ywei2017 commented 6 years ago
ywei2017 commented 6 years ago

Just updated. I did a few round of testing to ensure

But if I reboot the VM, or do a monit restart vault, the vault ends up starting up as sealed. I was reading the bosh docs, that the post-start script doesn't get called with monit restart. That actually suck since the purpose of the unseal keys is that the vault can get started properly if the VM got rebooted for whatever reason. I can't see that being a problem introduced in the 1.0.0+ release.

I will keep on reading more of the docs, and see whether that is even an issue that can be resolved in the bosh release. @MattSurabian @jhunt , please share your thoughts if you encountered similar issues in the past.

MattSurabian commented 6 years ago

Thanks @ywei2017! It's look great, just a few final notes.

MattSurabian commented 6 years ago

@ywei2017 On the monit restart thing... I'm wondering if there's a way to construct the monit file to take advantage of their restart configuration. As far as I know there is no other lifecycle hook in Bosh. The nice thing about solving it all in monit would be that then a user issued monit command would work to unseal as well. Because we have to exec out from the ctl file it's a bit tricky... Definitely not something to solve in this PR but totally worth looking at.

ywei2017 commented 6 years ago

@MattSurabian I most likely will submit a different PR and work with you on that, since I do care about the vault node coming up and ready to use :-)

ywei2017 commented 6 years ago

Hi, @MattSurabian , any other changes you like to see in this PR?

MattSurabian commented 6 years ago

The only thing I still see outstanding is the properties.sh.erb defaults vault_addr=p("vault.addr", "127.0.0.1") instead of vault_addr=p("vault.addr", "http://127.0.0.1:8200") or vault_addr=p("vault.addr", "")

ywei2017 commented 6 years ago

Hi, @MattSurabian , the default value has been corrected. Totally my miss.

Thanks, Yansheng

MattSurabian commented 6 years ago

Yay! Thanks @ywei2017 I'm hoping to spend some time early next week with @jhunt and cut a release with this PR and the new Vault binary in it.

ywei2017 commented 6 years ago

@MattSurabian, that is great. Which version of the binary do you have in mind? I remember seeing a warning that vault unseal will be removed in the next version. I can test out the post-install and make sure it’s compatible with the new binary.

MattSurabian commented 6 years ago

@ywei2017 still good till 0.11 apparently:

matt$ vault version
Vault v0.10.2 ('1543524ffb5f779bc35b7103cc4ce6174e1ec670')
matt$ vault unseal
WARNING! The "vault unseal" command is deprecated. Please use "vault operator
unseal" instead. This command will be removed in Vault 0.11 (or later).
ywei2017 commented 6 years ago

That is great to confirm. Looking forward to the new release.

On Mon, Jun 11, 2018 at 12:27 PM Matthew Surabian notifications@github.com wrote:

@ywei2017 https://github.com/ywei2017 still good till 0.11 apparently:

matt$ vault version Vault v0.10.2 ('1543524ffb5f779bc35b7103cc4ce6174e1ec670') matt$ vault unseal WARNING! The "vault unseal" command is deprecated. Please use "vault operator unseal" instead. This command will be removed in Vault 0.11 (or later).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cloudfoundry-community/vault-boshrelease/pull/60#issuecomment-396321207, or mute the thread https://github.com/notifications/unsubscribe-auth/AdKtiVha4jhtFvP5A49OLXkOWlCK5cnhks5t7qicgaJpZM4UOM5l .