Fix PluginHealthCheckJob (#18927)

* Fix PluginHealthCheckJob

We were incorrectly calling to start PluginHealthCheckJob
without initializing the plugins environment.

To fix this, we move the code inside initPlugins right after
the environment is initialized.

To respond to config changes, we call it again from pluginsEnvironment.Shutdown
which gets called from ConfigListener when plugins are disabled. And initPlugins
is anyways called again from ConfigListener which plugins are enabled,
so we can avoid checking for PluginSettings.Enable again in the call.

We also rename the method to better indicate its nature.

During this, we also uncover and fix another bug where disabling
plugins would not shut down plugins at all because we were calling
s.GetPluginsEnvironment() directly which returns nil if plugins
were disabled. The approach we follow is to manually acquire the lock
whenever we need access to the struct ignoring config setting.
We fix that as well.

https://community-daily.mattermost.com/boards/workspace/zyoahc9uapdn3xdptac6jb69ic/285b80a3-257d-41f6-8cf4-ed80ca9d92e5/495cdb4d-c13a-4992-8eb9-80cfee2819a4?c=6ef6178c-3512-4e57-8edd-1d2b66a09c9e
```release-note
NONE
```

* Fix test

```release-note
NONE
```
This commit is contained in:
Agniva De Sarker
2021-11-04 08:54:03 +05:30
committed by GitHub
parent 5ad7fb5b6f
commit 83b3090eb9
3 changed files with 10 additions and 20 deletions

View File

@@ -180,6 +180,7 @@ func (s *Server) initPlugins(c *request.Context, pluginDir, webappPluginDir stri
s.PluginsLock.RUnlock()
if pluginsEnvironment != nil || !*s.Config().PluginSettings.Enable {
s.syncPluginsActiveState()
pluginsEnvironment.TogglePluginHealthCheckJob(*s.Config().PluginSettings.EnableHealthCheck)
return
}
@@ -208,6 +209,8 @@ func (s *Server) initPlugins(c *request.Context, pluginDir, webappPluginDir stri
s.PluginsEnvironment = env
s.PluginsLock.Unlock()
s.PluginsEnvironment.TogglePluginHealthCheckJob(*s.Config().PluginSettings.EnableHealthCheck)
if err := s.syncPlugins(); err != nil {
mlog.Error("Failed to sync plugins from the file store", mlog.Err(err))
}
@@ -327,7 +330,10 @@ func (s *Server) syncPlugins() *model.AppError {
}
func (s *Server) ShutDownPlugins() {
pluginsEnvironment := s.GetPluginsEnvironment()
// Acquiring lock manually, as plugins might be disabled. See GetPluginsEnvironment.
s.PluginsLock.RLock()
pluginsEnvironment := s.PluginsEnvironment
s.PluginsLock.RUnlock()
if pluginsEnvironment == nil {
return
}

View File

@@ -571,20 +571,6 @@ func NewServer(options ...Option) (*Server, error) {
s.EmailService.InitEmailBatching()
})
// Start plugin health check job
pluginsEnvironment := s.PluginsEnvironment
if pluginsEnvironment != nil {
pluginsEnvironment.InitPluginHealthCheckJob(*s.Config().PluginSettings.Enable && *s.Config().PluginSettings.EnableHealthCheck)
}
s.AddConfigListener(func(_, c *model.Config) {
s.PluginsLock.RLock()
pluginsEnvironment := s.PluginsEnvironment
s.PluginsLock.RUnlock()
if pluginsEnvironment != nil {
pluginsEnvironment.InitPluginHealthCheckJob(*s.Config().PluginSettings.Enable && *c.PluginSettings.EnableHealthCheck)
}
})
logCurrentVersion := fmt.Sprintf("Current version is %v (%v/%v/%v/%v)", model.CurrentVersion, model.BuildNumber, model.BuildDate, model.BuildHash, model.BuildHashEnterprise)
mlog.Info(
logCurrentVersion,

View File

@@ -332,9 +332,7 @@ func (env *Environment) RestartPlugin(id string) error {
// Shutdown deactivates all plugins and gracefully shuts down the environment.
func (env *Environment) Shutdown() {
if env.pluginHealthCheckJob != nil {
env.pluginHealthCheckJob.Cancel()
}
env.TogglePluginHealthCheckJob(false)
var wg sync.WaitGroup
env.registeredPlugins.Range(func(key, value interface{}) bool {
@@ -507,8 +505,8 @@ func newRegisteredPlugin(bundle *model.BundleInfo) registeredPlugin {
return registeredPlugin{State: state, BundleInfo: bundle}
}
// InitPluginHealthCheckJob starts a new job if one is not running and is set to enabled, or kills an existing one if set to disabled.
func (env *Environment) InitPluginHealthCheckJob(enable bool) {
// TogglePluginHealthCheckJob starts a new job if one is not running and is set to enabled, or kills an existing one if set to disabled.
func (env *Environment) TogglePluginHealthCheckJob(enable bool) {
// Config is set to enable. No job exists, start a new job.
if enable && env.pluginHealthCheckJob == nil {
mlog.Debug("Enabling plugin health check job", mlog.Duration("interval_s", HealthCheckInterval))