Skip to content

Commit

Permalink
Skip apply empty changes in the cache provider
Browse files Browse the repository at this point in the history
Change-Id: Icaf1ffe34e75c320d4efbb428f831deb8784cd11
  • Loading branch information
tjamet committed Aug 14, 2024
1 parent 089744c commit 29191e2
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ func main() {
case "txt":
r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, cfg.TXTEncryptEnabled, []byte(cfg.TXTEncryptAESKey))
case "aws-sd":
r, err = registry.NewAWSSDRegistry(p.(*awssd.AWSSDProvider), cfg.TXTOwnerID)
r, err = registry.NewAWSSDRegistry(p, cfg.TXTOwnerID)
default:
log.Fatalf("unknown registry: %s", cfg.Registry)
}
Expand Down
11 changes: 11 additions & 0 deletions provider/cached_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/plan"
)
Expand Down Expand Up @@ -41,15 +42,23 @@ type CachedProvider struct {

func (c *CachedProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
if c.needRefresh() {
log.Info("Records cache provider: refreshing records list cache")
c.cache, c.err = c.Provider.Records(ctx)
if c.err != nil {
log.Errorf("Records cache provider: list records failed: %v", c.err)
}
c.lastRead = time.Now()
cachedRecordsCallsTotal.WithLabelValues("false").Inc()
} else {
log.Info("Records cache provider: using records list from cache")
cachedRecordsCallsTotal.WithLabelValues("true").Inc()
}
return c.cache, c.err
}
func (c *CachedProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
if !changes.HasChanges() {
return nil
}
c.Reset()
cachedApplyChangesCallsTotal.Inc()
return c.Provider.ApplyChanges(ctx, changes)
Expand All @@ -63,8 +72,10 @@ func (c *CachedProvider) Reset() {

func (c *CachedProvider) needRefresh() bool {
if c.cache == nil || c.err != nil {
log.Debug("Records cache provider is not initialized")
return true
}
log.Debug("Records cache last Read: ", c.lastRead, "expiration: ", c.RefreshDelay, " provider expiration:", c.lastRead.Add(c.RefreshDelay), "expired: ", time.Now().After(c.lastRead.Add(c.RefreshDelay)))
return time.Now().After(c.lastRead.Add(c.RefreshDelay))
}

Expand Down
28 changes: 27 additions & 1 deletion provider/cached_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,39 @@ func TestCachedProviderForcesCacheRefreshOnUpdate(t *testing.T) {
_, err := provider.Records(context.Background())
require.NoError(t, err)

t.Run("When the caching time frame is exceeded", func(t *testing.T) {
t.Run("When empty changes are applied", func(t *testing.T) {
testProvider.records = recordsNotCalled(t)
testProvider.applyChanges = func(ctx context.Context, changes *plan.Changes) error {
return nil
}
err := provider.ApplyChanges(context.Background(), &plan.Changes{})
assert.NoError(t, err)
t.Run("Next call to Records is cached", func(t *testing.T) {
testProvider.applyChanges = applyChangesNotCalled(t)
testProvider.records = func(ctx context.Context) ([]*endpoint.Endpoint, error) {
return []*endpoint.Endpoint{{DNSName: "new.domain.fqdn"}}, nil
}
endpoints, err := provider.Records(context.Background())

assert.NoError(t, err)
require.NotNil(t, endpoints)
require.Len(t, endpoints, 1)
require.NotNil(t, endpoints[0])
assert.Equal(t, "domain.fqdn", endpoints[0].DNSName)
})
})

t.Run("When changes are applied", func(t *testing.T) {
testProvider.records = recordsNotCalled(t)
testProvider.applyChanges = func(ctx context.Context, changes *plan.Changes) error {
return nil
}
err := provider.ApplyChanges(context.Background(), &plan.Changes{
Create: []*endpoint.Endpoint{
{DNSName: "hello.world"},
},
})
assert.NoError(t, err)
t.Run("Next call to Records is not cached", func(t *testing.T) {
testProvider.applyChanges = applyChangesNotCalled(t)
testProvider.records = func(ctx context.Context) ([]*endpoint.Endpoint, error) {
Expand Down

0 comments on commit 29191e2

Please sign in to comment.