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

Smart reload #5934

Closed
jsirex opened this issue Dec 6, 2018 · 6 comments
Closed

Smart reload #5934

jsirex opened this issue Dec 6, 2018 · 6 comments
Labels
Type:Additional Discussion Type: Feature Issues that describe a new desired feature

Comments

@jsirex
Copy link
Contributor

jsirex commented Dec 6, 2018

This a "really for discussion" issue.

UPD: Refactored feature request

There are cases when I need a decider to reload or restart service. For example core/consul has reloadable configuration: https://www.consul.io/docs/agent/options.html#reloadable-configuration.
But not all config options are supported. For example:

  1. if I change dns-port I must restart service - but service will be reloaded (reload hook exist)
  2. if I change log-level reload will work as expected.

I believe there are many similar cases when we prefer restart in particular case.

We can notify service to restart when specific config files have changed. Restart should honor update strategy to prevent whole cluster down.

# plan.sh example
# given a package with reload hook defined
pkg_svc_notify_restart=(
  config/network.cfg # changes to this file or
  hook/run           # or this hook or
  files/ca.cert       # of this file trigger restart
)

If change didn't touch $pkg_svc_notify_restart files - default behavior used: reload if hook defined, restart otherwise.
Supervisor can memorize sha256sum for $pkg_svc_notify_restart files after load and listen for ring or config updates. If sha256sum changes - then file changed.

How to use:
The easiest way is to split configuration into multiple files and watch some of them.
This is common approach for most linux distribution and packaging systems: for example /etc/consul.d/.
If splitting configuration is not possible, you can create and watch dummy file(s) with required options. For example:

# plan.sh
pkg_svc_notify_restart=(config/restart_file)


# config/nginx.config
worker_processes       {{cfg.nginx.worker_processes}};
daemon                 off;
pid		       {{pkg.svc_var_path}}/nginx.pid;
....


# config/restart_file
{{cfg.nginx.http.listen.port}}
{{#each bind.backends as|backend| ~}}
{{backend.sys.ip}}
{{/each}}

PS. It's time for me to learn rust :)

@HT154
Copy link

HT154 commented Dec 11, 2018

I recently came across a need for something like this as well. I have situation where a subset of my service's config files can (and should) be live-reloaded, but changes in other files require a full restart. Using cfg/svc/etc. template variables to gate this would work for my use case

@jsirex jsirex changed the title Smart reload, smart service Smart reload Dec 16, 2018
@jsirex
Copy link
Contributor Author

jsirex commented Dec 16, 2018

I refactored initial request to make it simpler.
Now this feature relates only to #5305

@jsirex
Copy link
Contributor Author

jsirex commented Jan 8, 2019

@chefsalim, can someone have this get prioritized?

Lack of this feature forces us to develop ugly workarounds to prevent whole clusters restart on minor config change.
Thanks.

@jsirex
Copy link
Contributor Author

jsirex commented Jan 24, 2019

Managed to implement smart reload logic by bash as proof of concept: habitat-sh/builder#885

@jsirex
Copy link
Contributor Author

jsirex commented Jan 25, 2019

UPD: Another option is to provider helper method for all configs and hook files:

...
restart="{{svcFileChanged "config/that-config.json"}}"
..
if [ "$restart" == "true" ]; then
 echo "Restarting service"
  kill $PID
fi

@jsirex
Copy link
Contributor Author

jsirex commented Nov 6, 2019

I think this can be closed, because lifecycle hooks were reworked:

  • if hooks/ changed - restart
  • if config/ - reconfigure or restart.

So I can put options in hooks/run to force restart

@jsirex jsirex closed this as completed Nov 6, 2019
@christophermaier christophermaier added Type: Feature Issues that describe a new desired feature and removed C-feature labels Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Additional Discussion Type: Feature Issues that describe a new desired feature
Projects
None yet
Development

No branches or pull requests

4 participants