fix: remove old configurations from conf store (#41)

* fix: remove old configurations from conf store
dollarshaveclub · May 4, 2020 · 37cbced · 37cbced
1 parent d7adde8
commit 37cbced
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 2 deletions.
diff --git a/e2e/docker-compose-async.yml b/e2e/docker-compose-async.yml
@@ -12,7 +12,7 @@ services:
     environment:
       GUARDIAN_FLAG_REDIS_ADDRESS: "redis:6379"
       GUARDIAN_FLAG_LOG_LEVEL: "debug"
-      GUARDIAN_FLAG_CONF_UPDATE_INTERVAL: "1s"
+      GUARDIAN_FLAG_CONF_UPDATE_INTERVAL: "500ms"
       GUARDIAN_FLAG_SYNCHRONOUS: "false"
     depends_on:
     - redis

diff --git a/e2e/docker-compose-sync.yml b/e2e/docker-compose-sync.yml
@@ -12,7 +12,7 @@ services:
     environment:
       GUARDIAN_FLAG_REDIS_ADDRESS: "redis:6379"
       GUARDIAN_FLAG_LOG_LEVEL: "debug"
-      GUARDIAN_FLAG_CONF_UPDATE_INTERVAL: "1s"
+      GUARDIAN_FLAG_CONF_UPDATE_INTERVAL: "500ms"
       GUARDIAN_FLAG_SYNCHRONOUS: "true"
     depends_on:
     - redis

diff --git a/pkg/guardian/redis_conf_store.go b/pkg/guardian/redis_conf_store.go
@@ -604,6 +604,7 @@ func (rs *RedisConfStore) UpdateCachedConf() {
 		rs.conf.reportOnly = *fetched.reportOnly
 	}
 
+	rs.conf.routeRateLimits = make(map[url.URL]Limit, len(fetched.routeLimitCounts))
 	for url, count := range fetched.routeLimitCounts {
 		duration, _ := fetched.routeLimitDurations[url]
 		enabled, _ := fetched.routeRateLimitsEnabled[url]
@@ -618,6 +619,7 @@ func (rs *RedisConfStore) UpdateCachedConf() {
 		}
 	}
 
+	rs.conf.jails = make(map[url.URL]Jail, len(fetched.jailLimitCounts))
 	for url, count := range fetched.jailLimitCounts {
 		duration, _ := fetched.jailLimitDurations[url]
 		enabled, _ := fetched.jailLimitsEnabled[url]

diff --git a/pkg/guardian/redis_conf_store_test.go b/pkg/guardian/redis_conf_store_test.go
@@ -370,6 +370,13 @@ func TestConfStoreAddRemoveRouteRateLimits(t *testing.T) {
 		t.Errorf("expected: %v, received: %v", fooBarLimit, got)
 	}
 
+	// Ensure configuration cache is updated after a confSyncInterval
+	c.UpdateCachedConf()
+	cachedItem := c.GetRouteRateLimit(*fooBarURL)
+	if !cmp.Equal(cachedItem, fooBarLimit) {
+		t.Errorf("expected: %v, received: %v", fooBarLimit, cachedItem)
+	}
+
 	var urls []url.URL
 	urls = append(urls, *fooBarURL)
 	if err := c.RemoveRouteRateLimits(urls); err != nil {
@@ -382,6 +389,13 @@ func TestConfStoreAddRemoveRouteRateLimits(t *testing.T) {
 		t.Fatalf("expected error fetching route limit which didn't exist")
 	}
 
+	// Ensure configuration cache is updated after a confSyncInterval
+	c.UpdateCachedConf()
+	cachedItem = c.GetRouteRateLimit(*fooBarURL)
+	if !cmp.Equal(cachedItem, Limit{}) {
+		t.Errorf("expected: %v, received: %v", Limit{}, cachedItem)
+	}
+
 	got, err = c.FetchRouteRateLimit(*fooBazURL)
 	if err != nil {
 		t.Fatalf("got error: %v", err)
@@ -522,6 +536,13 @@ func TestConfStoreAddRemoveJails(t *testing.T) {
 		t.Errorf("expected: %v, received: %v", fooBarJail, got)
 	}
 
+	// Ensure configuration cache is updated after a confSyncInterval
+	c.UpdateCachedConf()
+	cachedItem := c.GetJail(*fooBarURL)
+	if !cmp.Equal(cachedItem, fooBarJail) {
+		t.Errorf("expected: %v, received: %v", fooBarJail, cachedItem)
+	}
+
 	var urls []url.URL
 	urls = append(urls, *fooBarURL)
 	if err := c.RemoveJails(urls); err != nil {
@@ -534,6 +555,13 @@ func TestConfStoreAddRemoveJails(t *testing.T) {
 		t.Fatalf("expected error fetching route limit which didn't exist")
 	}
 
+	// Ensure configuration cache is updated after a confSyncInterval
+	c.UpdateCachedConf()
+	cachedItem = c.GetJail(*fooBarURL)
+	if !cmp.Equal(cachedItem, Jail{}) {
+		t.Errorf("expected: %v, received: %v", Jail{}, cachedItem)
+	}
+
 	got, err = c.FetchJail(*fooBazURL)
 	if err != nil {
 		t.Fatalf("got error: %v", err)