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

Invalidate host context provider cache when FQDN feature flag is toggled #2461

Merged
merged 11 commits into from
Apr 6, 2023
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)
}