Skip to content

Commit

Permalink
Invalidate host context provider cache when FQDN feature flag is togg…
Browse files Browse the repository at this point in the history
…led (#2461)

* Add callback mechanism to notify when FQDN feature flag is toggled

* Use callback to bust host provider cache

* Moving const inside method

* Fixing typo

* Formatting for better readability

* Fixing comment

* Fixing comment

* Check error from features.Apply

* Simplify test

* Increasing sleep times

* Better cleanup of resources

(cherry picked from commit 0640efd)
  • Loading branch information
ycombinator authored and mergify[bot] committed Apr 6, 2023
1 parent d637c88 commit 5564836
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 1 deletion.
23 changes: 23 additions & 0 deletions internal/pkg/composable/providers/host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,29 @@ func (c *contextProvider) Run(comm corecomp.ContextProviderComm) error {
return errors.New(err, "failed to set mapping", errors.TypeUnexpected)
}

const fqdnFeatureFlagCallbackID = "host_provider"
fqdnFFChangeCh := make(chan struct{})
err = features.AddFQDNOnChangeCallback(
onFQDNFeatureFlagChange(fqdnFFChangeCh),
fqdnFeatureFlagCallbackID,
)
if err != nil {
return fmt.Errorf("unable to add FQDN onChange callback in host provider: %w", err)
}

defer func() {
features.RemoveFQDNOnChangeCallback(fqdnFeatureFlagCallbackID)
close(fqdnFFChangeCh)
}()

// Update context when any host information changes.
for {
t := time.NewTimer(c.CheckInterval)
select {
case <-comm.Done():
t.Stop()
return comm.Err()
case <-fqdnFFChangeCh:
case <-t.C:
}

Expand All @@ -76,6 +92,13 @@ func (c *contextProvider) Run(comm corecomp.ContextProviderComm) error {
}
}

func onFQDNFeatureFlagChange(fqdnFFChangeCh chan struct{}) features.BoolValueOnChangeCallback {
return func(new, old bool) {
// FQDN feature flag was toggled, so notify on channel
fqdnFFChangeCh <- struct{}{}
}
}

// ContextProviderBuilder builds the context provider.
func ContextProviderBuilder(log *logger.Logger, c *config.Config, _ bool) (corecomp.ContextProvider, error) {
p := &contextProvider{
Expand Down
62 changes: 62 additions & 0 deletions internal/pkg/composable/providers/host/host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package host
import (
"context"

"github.com/elastic/elastic-agent/pkg/features"

"sync"
"testing"
"time"
Expand Down Expand Up @@ -82,6 +84,66 @@ func TestContextProvider(t *testing.T) {
assert.Equal(t, next, comm.Current())
}

func TestFQDNFeatureFlagToggle(t *testing.T) {
log, err := logger.New("host_test", false)
require.NoError(t, err)

c, err := config.NewConfigFrom(map[string]interface{}{
// Use a long check interval so we can ensure that any
// calls to hostProvider.fetcher are not happening due
// to the interval timer elapsing. We want such calls
// to happen only due to explicit actions in our
// test below.
"check_interval": 10 * time.Minute,
})
require.NoError(t, err)

builder, _ := composable.Providers.GetContextProvider("host")
provider, err := builder(log, c, true)
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
defer func() {
cancel()
}()
comm := ctesting.NewContextComm(ctx)

// Track the number of times hostProvider.fetcher is called.
numCalled := 0
hostProvider, ok := provider.(*contextProvider)
require.True(t, ok)

hostProvider.fetcher = func() (map[string]interface{}, error) {
numCalled++
return nil, nil
}

// Run the provider
go func() {
err = provider.Run(comm)
}()

// Wait long enough for provider.Run to register
// the FQDN feature flag onChange callback.
time.Sleep(10 * time.Millisecond)

// Trigger the FQDN feature flag callback by
// toggling the FQDN feature flag
err = features.Apply(config.MustNewConfigFrom(map[string]interface{}{
"agent.features.fqdn.enabled": true,
}))
require.NoError(t, err)

// Wait long enough for the FQDN feature flag onChange
// callback to be called.
time.Sleep(10 * time.Millisecond)

// hostProvider.fetcher should be called twice:
// - once, right after the provider is run, and
// - once again, when the FQDN feature flag callback is triggered
require.Equal(t, 2, numCalled)
}

func returnHostMapping(log *logger.Logger) infoFetcher {
i := -1
fetcher := getHostInfo(log)
Expand Down
36 changes: 35 additions & 1 deletion pkg/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ var (
current = Flags{}
)

type BoolValueOnChangeCallback func(new, old bool)

type Flags struct {
mu sync.RWMutex
source *structpb.Struct
fqdn bool

fqdn bool
fqdnCallbacks map[string]BoolValueOnChangeCallback
}

type cfg struct {
Expand Down Expand Up @@ -51,12 +55,42 @@ func (f *Flags) AsProto() *proto.Features {
}
}

// AddFQDNOnChangeCallback takes a callback function that will be called with the new and old values
// of `flags.fqdnEnabled` whenever it changes. It also takes a string ID - this is useful
// in calling `RemoveFQDNOnChangeCallback` to de-register the callback.
func AddFQDNOnChangeCallback(cb BoolValueOnChangeCallback, id string) error {
current.mu.Lock()
defer current.mu.Unlock()

// Initialize callbacks map if necessary.
if current.fqdnCallbacks == nil {
current.fqdnCallbacks = map[string]BoolValueOnChangeCallback{}
}

current.fqdnCallbacks[id] = cb
return nil
}

// RemoveFQDNOnChangeCallback removes the callback function associated with the given ID (originally
// returned by `AddFQDNOnChangeCallback` so that function will be no longer be called when
// `flags.fqdnEnabled` changes.
func RemoveFQDNOnChangeCallback(id string) {
current.mu.Lock()
defer current.mu.Unlock()

delete(current.fqdnCallbacks, id)
}

// setFQDN sets the value of the FQDN flag in Flags.
func (f *Flags) setFQDN(newValue bool) {
f.mu.Lock()
defer f.mu.Unlock()

oldValue := f.fqdn
f.fqdn = newValue
for _, cb := range f.fqdnCallbacks {
cb(newValue, oldValue)
}
}

// setSource sets the source from he given cfg.
Expand Down
37 changes: 37 additions & 0 deletions pkg/features/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package features
import (
"testing"

"github.com/stretchr/testify/require"

"github.com/elastic/elastic-agent/internal/pkg/config"
)

Expand Down Expand Up @@ -79,3 +81,38 @@ agent:
})
}
}

func TestFQDNCallbacks(t *testing.T) {
cb1Called, cb2Called := false, false

err := AddFQDNOnChangeCallback(func(new, old bool) {
cb1Called = true
}, "cb1")
require.NoError(t, err)

err = AddFQDNOnChangeCallback(func(new, old bool) {
cb2Called = true
}, "cb2")
require.NoError(t, err)

defer func() {
// Cleanup in case we don't get to the end of
// this test successfully.
if _, exists := current.fqdnCallbacks["cb1"]; exists {
RemoveFQDNOnChangeCallback("cb1")
}
if _, exists := current.fqdnCallbacks["cb2"]; exists {
RemoveFQDNOnChangeCallback("cb2")
}
}()

require.Len(t, current.fqdnCallbacks, 2)
current.setFQDN(false)
require.True(t, cb1Called)
require.True(t, cb2Called)

RemoveFQDNOnChangeCallback("cb1")
require.Len(t, current.fqdnCallbacks, 1)
RemoveFQDNOnChangeCallback("cb2")
require.Len(t, current.fqdnCallbacks, 0)
}

0 comments on commit 5564836

Please sign in to comment.