Skip to content

Conversation

@peterbroadhurst
Copy link
Contributor

@peterbroadhurst peterbroadhurst commented Dec 8, 2022

Summary

This PR implements file-listener based dynamic configuration reload for FireFly Core.

  • Allows listening for changes to a config file in a Kubernetes configmap or secret
  • Performs analysis on the configuration to determine what namespaces and plugins have changed
    • Note: Does not currently support reloading API server configuration
  • Does not affect any namespaces that are unaffected by changes
  • Restarts all namespaces affected by plugin changes, or direct configuration changes
  • Adds namespaces that did not previously exist
  • Removes namespaces that have been removed

Depends on hyperledger/firefly-common#40 making it into a release before this can be merged

Implementation detail

The namespace.Manager code has been refactored with the following significant changes:

  • The initialization code has been decomposed into blocks that can be re-run during config reload
  • A strong separation is created between configuration load+validation, and runtime initialization
  • The individual maps of namespaceManager.plugins and namespaceManager.pluginNames has been replaced with a single map of plugins - which depending on the category will have a link to the specific plugin type
  • A new dynamic configuration reload function is connected to a file listener, which tries to be self-explanatory as follows:
    func (nm *namespaceManager) configReloaded(ctx context.Context) {
    // Get Viper to dump the whole new config, with everything resolved across env vars
    // and the config file etc.
    // We use this to detect if anything has changed.
    rawConfig := nm.dumpRootConfig()
    // Build the new set of plugins from the config (including those that are unchanged)
    allPluginsInNewConf, err := nm.loadPlugins(ctx, rawConfig)
    if err != nil {
    log.L(ctx).Errorf("Failed to initialize plugins after config reload: %s", err)
    return
    }
    // Analyze the new list to see which plugins need to be updated,
    // so we load the namespaces against the correct list of plugins
    availablePlugins, updatedPlugins, pluginsToStop := nm.analyzePluginChanges(ctx, allPluginsInNewConf)
    // Build the new set of namespaces (including those that are unchanged)
    allNewNamespaces, err := nm.loadNamespaces(ctx, rawConfig, availablePlugins)
    if err != nil {
    log.L(ctx).Errorf("Failed to load namespaces after config reload: %s", err)
    return
    }
    // From this point we need to block any API calls resolving namespaces,
    // until the reload is complete
    nm.nsMux.Lock()
    defer nm.nsMux.Unlock()
    // Stop all defunct namespaces
    availableNS, updatedNamespaces := nm.stopDefunctNamespaces(ctx, availablePlugins, allNewNamespaces)
    // Stop all defunct plugins - now the namespaces using them are all stopped
    nm.stopDefunctPlugins(ctx, pluginsToStop)
    // Update the new lists
    nm.plugins = availablePlugins
    nm.namespaces = availableNS
    // Only initialize updated plugins
    if err = nm.initPlugins(updatedPlugins); err != nil {
    log.L(ctx).Errorf("Failed to initialize plugins after config reload: %s", err)
    nm.cancelCtx() // stop the world
    return
    }
    // Only initialize the updated namespaces (which includes all that depend on above plugins)
    if err = nm.initNamespaces(ctx, updatedNamespaces); err != nil {
    log.L(ctx).Errorf("Failed to initialize namespaces after config reload: %s", err)
    nm.cancelCtx() // stop the world
    return
    }
    // Now finally we can start all the new things
    if err = nm.startNamespacesAndPlugins(updatedNamespaces, updatedPlugins); err != nil {
    log.L(ctx).Errorf("Failed to initialize namespaces after config reload: %s", err)
    nm.cancelCtx() // stop the world
    return
    }
    }

Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2022

Codecov Report

Merging #1113 (0214e15) into main (0697425) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 0214e15 differs from pull request most recent head f23122e. Consider uploading reports for the commit f23122e to get more accurate results

@@             Coverage Diff             @@
##              main    #1113      +/-   ##
===========================================
- Coverage   100.00%   99.97%   -0.03%     
===========================================
  Files          303      303              
  Lines        19799    19528     -271     
===========================================
- Hits         19799    19524     -275     
- Misses           0        4       +4     
Impacted Files Coverage Δ
internal/coreconfig/coreconfig.go 100.00% <ø> (ø)
cmd/firefly.go 100.00% <100.00%> (ø)
internal/apiserver/route_spi_post_reset.go 100.00% <100.00%> (ø)
internal/blockchain/ethereum/ethereum.go 100.00% <100.00%> (ø)
internal/namespace/config.go 100.00% <100.00%> (ø)
internal/namespace/configreload.go 100.00% <100.00%> (ø)
internal/namespace/manager.go 100.00% <100.00%> (ø)
internal/orchestrator/persistence_events.go 96.29% <0.00%> (-3.71%) ⬇️
internal/metrics/metrics.go 97.43% <0.00%> (-2.57%) ⬇️
internal/events/aggregator_rewind.go 99.34% <0.00%> (-0.66%) ⬇️
... and 28 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@peterbroadhurst peterbroadhurst marked this pull request as ready for review December 9, 2022 21:41
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Copy link
Contributor

@nguyer nguyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I'll see if I can work out the conflicts.

Comment on lines +358 to +363
## config

|Key|Description|Type|Default Value|
|---|-----------|----|-------------|
|autoReload|Monitor the configuration file for changes, and automatically add/remove/reload namespaces and plugins|`boolean`|`<nil>`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config for the config 🤯

Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
@nguyer nguyer merged commit b69d4f2 into hyperledger:main Jan 13, 2023
@nguyer nguyer deleted the config-listener branch January 13, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants