Skip to content

Commit

Permalink
Restart deployment on config diff
Browse files Browse the repository at this point in the history
  • Loading branch information
pkosiec committed Mar 6, 2023
1 parent 74cb901 commit 286ee49
Show file tree
Hide file tree
Showing 15 changed files with 372 additions and 27 deletions.
10 changes: 10 additions & 0 deletions cmd/botkube/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,21 @@ func run(ctx context.Context) error {
})
}

restarter := reloader.NewRestarter(
logger.WithField(componentLogFieldKey, "Restarter"),
k8sCli,
conf.ConfigWatcher.Deployment,
conf.Settings.ClusterName,
func(msg string) error {
return notifier.SendPlaintextMessage(ctx, notifiers, msg)
},
)
cfgReloader := reloader.Get(
remoteCfgEnabled,
logger.WithField(componentLogFieldKey, "Config Updater"),
configUpdaterInterval,
deployClient,
restarter,
statusReporter,
)
errGroup.Go(func() error {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ require (
github.com/pkg/errors v0.9.1
github.com/pmezard/go-difflib v1.0.0
github.com/prometheus/client_golang v1.14.0
github.com/r3labs/diff/v3 v3.0.1
github.com/sanity-io/litter v1.5.5
github.com/segmentio/analytics-go v3.1.0+incompatible
github.com/sha1sum/aws_signing_client v0.0.0-20200229211254-f7815c59d5c1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1674,6 +1674,8 @@ github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1
github.com/prometheus/procfs v0.8.0 h1:ODq8ZFEaYeCaZOJlZZdJA2AbQR98dSHSM1KW/You5mo=
github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0uaxHdg830/4=
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
github.com/r3labs/diff/v3 v3.0.1 h1:CBKqf3XmNRHXKmdU7mZP1w7TV0pDyVCis1AUHtA4Xtg=
github.com/r3labs/diff/v3 v3.0.1/go.mod h1:f1S9bourRbiM66NskseyUdo0fTmEE0qKrikYJX63dgo=
github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/reflog/dateconstraints v0.2.1/go.mod h1:Ax8AxTBcJc3E/oVS2hd2j7RDM/5MDtuPwuR7lIHtPLo=
Expand Down
4 changes: 4 additions & 0 deletions helm/botkube/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ spec:
value: "{{.Release.Namespace}}"
- name: BOTKUBE_SETTINGS_LIFECYCLE__SERVER_DEPLOYMENT_NAME
value: "{{ include "botkube.fullname" . }}"
- name: BOTKUBE_CONFIG__WATCHER_DEPLOYMENT_NAMESPACE
value: "{{.Release.Namespace}}"
- name: BOTKUBE_CONFIG__WATCHER_DEPLOYMENT_NAME
value: "{{ include "botkube.fullname" . }}"
{{- if .Values.config.provider.endpoint }}
- name: CONFIG_PROVIDER_ENDPOINT
value: {{ .Values.config.provider.endpoint }}
Expand Down
4 changes: 2 additions & 2 deletions internal/config/reloader/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import (
)

// Get returns Reloader based on remoteCfgEnabled flag.
func Get(remoteCfgEnabled bool, log logrus.FieldLogger, interval time.Duration, deployCli DeploymentClient, resVerHolders ...ResourceVersionHolder) Reloader {
func Get(remoteCfgEnabled bool, log logrus.FieldLogger, interval time.Duration, deployCli DeploymentClient, restarter *Restarter, resVerHolders ...ResourceVersionHolder) Reloader {
if remoteCfgEnabled {
return NewRemote(log, interval, deployCli, resVerHolders...)
return NewRemote(log, interval, deployCli, restarter, resVerHolders...)
}

return NewNoopReloader()
Expand Down
161 changes: 161 additions & 0 deletions internal/config/reloader/reloader_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package reloader

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/kubeshop/botkube/internal/loggerx"
"github.com/kubeshop/botkube/pkg/config"
)

func TestRemote_ProcessConfig(t *testing.T) {
testCases := []struct {
Name string

InitialCfg config.Config
InitialResVer int

NewConfig config.Config
NewResVer int

ExpectedErrMessage string
ExpectedCfg config.Config
ExpectedCfgDiff configDiff
ExpectedResVer int
}{
{
Name: "Resource Version equal",
InitialCfg: config.Config{},
InitialResVer: 2,
NewConfig: config.Config{},
NewResVer: 2,
ExpectedCfg: config.Config{},
ExpectedResVer: 2,
ExpectedCfgDiff: configDiff{},
},
{
Name: "Resource Version lower",
InitialCfg: config.Config{},
InitialResVer: 2,
NewConfig: config.Config{},
NewResVer: 1,

ExpectedErrMessage: "current config version (2) is newer than the latest one (1)",
ExpectedCfg: config.Config{},
ExpectedResVer: 2,
ExpectedCfgDiff: configDiff{},
},
{
Name: "Resource Version higher but config the same",
InitialCfg: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: false,
},
},
},
InitialResVer: 2,
NewConfig: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: false,
},
},
},
NewResVer: 3,

ExpectedErrMessage: "",
ExpectedCfg: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: false,
},
},
},
ExpectedResVer: 3,
ExpectedCfgDiff: configDiff{},
},
{
Name: "Different config, should restart",
InitialCfg: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: false,
},
"second": {
Enabled: false,
},
},
},
InitialResVer: 2,
NewConfig: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: true,
},
"second": {
Enabled: false,
},
},
},
NewResVer: 3,

ExpectedErrMessage: "",
ExpectedCfg: config.Config{
Actions: map[string]config.Action{
"test": {
Enabled: true,
},
"second": {
Enabled: false,
},
},
},
ExpectedResVer: 3,
ExpectedCfgDiff: configDiff{shouldRestart: true},
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
resVerHldr := &sampleResVerHolder{testCase.InitialResVer}
remoteReloader := RemoteConfigReloader{
log: loggerx.NewNoop(),
interval: time.Minute,
deployCli: nil,
resVerHolders: []ResourceVersionHolder{resVerHldr},
latestCfg: testCase.InitialCfg,
resVersion: testCase.InitialResVer,
}

cfgDiff, err := remoteReloader.processNewConfig(testCase.NewConfig, testCase.NewResVer)
if testCase.ExpectedErrMessage != "" {
require.Error(t, err)
assert.EqualError(t, err, testCase.ExpectedErrMessage)
assert.Equal(t, testCase.ExpectedCfg, remoteReloader.latestCfg)
return
}

require.NoError(t, err)
assert.Equal(t, testCase.ExpectedCfg, remoteReloader.latestCfg)
assert.Equal(t, testCase.ExpectedResVer, remoteReloader.resVersion)
assert.Equal(t, testCase.ExpectedResVer, resVerHldr.resVer)
assert.Equal(t, testCase.ExpectedCfgDiff, cfgDiff)
})
}
}

type sampleResVerHolder struct {
resVer int
}

func (s *sampleResVerHolder) SetResourceVersion(version int) {
s.resVer = version
}

func (s *sampleResVerHolder) GetResourceVersion(version int) {
s.resVer = version
}
71 changes: 62 additions & 9 deletions internal/config/reloader/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package reloader
import (
"context"
"fmt"
"strings"
"time"

"github.com/r3labs/diff/v3"
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v3"

Expand All @@ -18,12 +20,13 @@ type DeploymentClient interface {
}

// NewRemote returns new ConfigUpdater.
func NewRemote(log logrus.FieldLogger, interval time.Duration, deployCli DeploymentClient, resVerHolders ...ResourceVersionHolder) *RemoteConfigReloader {
func NewRemote(log logrus.FieldLogger, interval time.Duration, deployCli DeploymentClient, restarter *Restarter, resVerHolders ...ResourceVersionHolder) *RemoteConfigReloader {
return &RemoteConfigReloader{
log: log,
interval: interval,
deployCli: deployCli,
resVerHolders: resVerHolders,
restarter: restarter,
}
}

Expand All @@ -36,6 +39,7 @@ type RemoteConfigReloader struct {
resVersion int

deployCli DeploymentClient
restarter *Restarter
}

func (u *RemoteConfigReloader) Do(ctx context.Context) error {
Expand All @@ -58,18 +62,23 @@ func (u *RemoteConfigReloader) Do(ctx context.Context) error {
u.log.Error(wrappedErr.Error())
continue
}

if resVer == u.resVersion {
u.log.Debugf("Config version (%d) is the same as the latest one. Skipping...", resVer)
cfgDiff, err := u.processNewConfig(cfg, resVer)
if err != nil {
wrappedErr := fmt.Errorf("while processing new config: %w", err)
u.log.Error(wrappedErr.Error())
continue
}

// TODO: check diff

if !cfgDiff.shouldRestart {
continue
}

u.latestCfg = cfg
u.setResourceVersionForAll(resVer)
u.log.Debugf("Successfully set newer config version (%d)", resVer)
u.log.Info("Reloading configuration...")
err = u.restarter.Do(ctx)
if err != nil {
u.log.Errorf("while restarting the app: %s", err.Error())
return fmt.Errorf("while restarting the app: %w", err)
}
}
}
}
Expand All @@ -89,6 +98,50 @@ func (u *RemoteConfigReloader) queryConfig(ctx context.Context) (config.Config,
return latestCfg, deploy.ResourceVersion, nil
}

type configDiff struct {
shouldRestart bool
}

func (u *RemoteConfigReloader) processNewConfig(newCfg config.Config, newResVer int) (configDiff, error) {
if newResVer == u.resVersion {
u.log.Debugf("Config version (%d) is the same as the latest one. Skipping...", newResVer)
return configDiff{}, nil
}
if newResVer < u.resVersion {
return configDiff{}, fmt.Errorf("current config version (%d) is newer than the latest one (%d)", u.resVersion, newResVer)
}
u.setResourceVersionForAll(newResVer)

changelog, err := diff.Diff(u.latestCfg, newCfg, diff.DisableStructValues(), diff.AllowTypeMismatch(true))
if err != nil {
return configDiff{}, fmt.Errorf("while diffing configs: %w", err)
}

if len(changelog) == 0 {
u.log.Debugf("Config with higher version (%d) is the same as the latest one. No need to reload config", newResVer)
return configDiff{}, nil
}

var paths []string
for _, change := range changelog {
paths = append(paths, fmt.Sprintf(`- "%s"`, strings.Join(change.Path, ".")))
}
u.log.Debugf("detected config changes on paths:\n%s", strings.Join(paths, "\n"))

// TODO: check if notifications are enabled and if so:
// - update notifications for a given channel (this needs a global state)
// - send message to a given channel (this needs a rework for the notifier executor)
// - updating notifications should happen after ConfigMap update, not before
// - same for remote config reloader
// - do not restart the app

u.latestCfg = newCfg
u.log.Debugf("Successfully set newer config version (%d). Config should be reloaded soon", newResVer)
return configDiff{
shouldRestart: true,
}, nil
}

func (u *RemoteConfigReloader) setResourceVersionForAll(resVersion int) {
u.resVersion = resVersion
for _, h := range u.resVerHolders {
Expand Down
Loading

0 comments on commit 286ee49

Please sign in to comment.