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

Prevent "vault" service to be restarted on update #52

Merged
merged 1 commit into from
Apr 18, 2016

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented Apr 18, 2016

Vault server restart is even more harmful than consul restart (related: sous-chefs/consul#288).

Vault server restart means that Vault becomes sealed and stops process incoming requests. So, regardless of the reason, it should be done only by an operator.

@davidsayers
Copy link

Would it not make more sense to have this a configurable option instead of forcing users one way or the other?

@Ginja
Copy link
Contributor

Ginja commented Apr 18, 2016

Good idea, @legal90. But I also agree with @davidsayers. Would you mind adding an attribute for toggling this? It would give power users the flexibility to come up with their own restart methodology.

default['hashicorp-vault']['service']['restart_on_update'] = false

Also, the Vault service will still restart if one of the config options are changed. Perhaps wrap this property in a conditional with the above attribute?

https://github.com/johnbellone/vault-cookbook/blob/master/recipes/default.rb#L18

@Ginja
Copy link
Contributor

Ginja commented Apr 18, 2016

Actually, I think this can be overridden with the correct poise-service attribute defined:

override['poise-service']['vault']['restart_on_update'] = false

@coderanger, is that right?

@legal90
Copy link
Contributor Author

legal90 commented Apr 18, 2016

Would you mind adding an attribute for toggling this?

OK, I'll do this.
UPDATE: PR was merged as is.

Also, the Vault service will still restart if one of the config options are changed. Perhaps wrap this property in a conditional with the above attribute?

There is reload, not restart. The first one is just sending a SIGHUP, which is safe action and doesn't cause Vault to be sealed.

@Ginja
Copy link
Contributor

Ginja commented Apr 18, 2016

Ah, good catch! Not enough ☕.

@johnbellone
Copy link
Contributor

This makes sense to me.

@johnbellone johnbellone merged commit 9392c33 into sous-chefs:master Apr 18, 2016
@legal90 legal90 deleted the fix-update branch April 18, 2016 14:19
@coderanger
Copy link

I would probably use the never_restart restart option rather than this.

service.options never_restart: true

All providers should support that option to make the restart action a no-op.

@coderanger
Copy link

And yes, restart_on_update can be overridden via an option too but that only covers sending a restart notification when the service definition itself changes.

@lock
Copy link

lock bot commented May 19, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants