ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

Update downloader's Etag to last successful act value

Browse Source 
Earlier the client would reset the etag on the downloader
in case of downloader errors and bundle activation failures.
The drawback of this approach is that OPA could potentially download
the same version of a bundle multiple times thereby unnecessarily
adding to network traffic.

This change resolves the issue by allowing the client to set
the etag on the downloader to the last successful activation
etag value in case of failures.

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
This commit is contained in:
Ashutosh Narkar
2021-06-03 16:01:07 -07:00
parent f58815c247
commit 2a6b8a74ca
3 changed files with 78 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
download/download.go
+6 -1
View File
@@ -111,11 +111,16 @@ func (d *Downloader) WithBundlePersistence(persist bool) *Downloader {
return d
}
// ClearCache resets the etag value on the downloader
// ClearCache is deprecated. Use SetCache instead.
func (d *Downloader) ClearCache() {
d.etag = ""
}
// SetCache sets the given etag value on the downloader.
func (d *Downloader) SetCache(etag string) {
d.etag = etag
}
// Start tells the Downloader to begin downloading bundles.
func (d *Downloader) Start(ctx context.Context) {
go d.doStart(ctx)
download/download_test.go
+61 -20
View File
@@ -253,23 +253,13 @@ func TestEtagCachingLifecycle(t *testing.T) {
t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag)
}
// simulate successful bundle activation and check updated etag on the downloader
// simulate downloader error on first bundle download
fixture.server.expCode = 500
fixture.server.expEtag = "some etag value"
_, err := fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
} else if len(fixture.updates) != 1 {
t.Fatal("expected update")
} else if fixture.d.etag != fixture.server.expEtag {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
// simulate downloader error and check etag is cleared
fixture.server.expCode = 500
_, err = fixture.d.oneShot(ctx)
if err == nil {
t.Fatal("Expected error but got nil")
} else if len(fixture.updates) != 2 {
} else if len(fixture.updates) != 1 {
t.Fatal("expected update")
} else if fixture.d.etag != "" {
t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag)
@@ -277,7 +267,17 @@ func TestEtagCachingLifecycle(t *testing.T) {
// simulate successful bundle activation and check updated etag on the downloader
fixture.server.expCode = 0
fixture.server.expEtag = "some new etag value"
_, err = fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
} else if len(fixture.updates) != 2 {
t.Fatal("expected update")
} else if fixture.d.etag != fixture.server.expEtag {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
// simulate another successful bundle activation and check updated etag on the downloader
fixture.server.expEtag = "some etag value - 2"
_, err = fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
@@ -287,16 +287,51 @@ func TestEtagCachingLifecycle(t *testing.T) {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
// simulate bundle activation error and check etag is cleared
// simulate bundle activation error and check etag is set from the last successful activation
fixture.mockBundleActivationError = true
fixture.server.expEtag = "some newer etag value"
fixture.server.expEtag = "some newer etag value - 3"
_, err = fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
} else if len(fixture.updates) != 4 {
t.Fatal("expected update")
} else if fixture.d.etag != "" {
t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag)
} else if fixture.d.etag != "some etag value - 2" {
t.Fatalf("Expected downloader ETag %v but got %v", "some etag value - 2", fixture.d.etag)
}
// simulate successful bundle activation and check updated etag on the downloader
fixture.server.expCode = 0
fixture.mockBundleActivationError = false
_, err = fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
} else if len(fixture.updates) != 5 {
t.Fatal("expected update")
} else if fixture.d.etag != fixture.server.expEtag {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
// simulate downloader error and check etag is set from the last successful activation
fixture.server.expCode = 500
_, err = fixture.d.oneShot(ctx)
if err == nil {
t.Fatal("Expected error but got nil")
} else if len(fixture.updates) != 6 {
t.Fatal("expected update")
} else if fixture.d.etag != fixture.server.expEtag {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
// simulate bundle activation error and check etag is set from the last successful activation
fixture.mockBundleActivationError = true
fixture.server.expCode = 0
_, err = fixture.d.oneShot(ctx)
if err != nil {
t.Fatal("Unexpected:", err)
} else if len(fixture.updates) != 7 {
t.Fatal("expected update")
} else if fixture.d.etag != fixture.server.expEtag {
t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag)
}
}
@@ -393,6 +428,7 @@ type testFixture struct {
server *testServer
updates []Update
mockBundleActivationError bool
etags map[string]string
}
func newTestFixture(t *testing.T) testFixture {
@@ -443,6 +479,7 @@ func newTestFixture(t *testing.T) testFixture {
client: tc,
server: &ts,
updates: []Update{},
etags: make(map[string]string),
}
}
@@ -451,16 +488,20 @@ func (t *testFixture) oneShot(ctx context.Context, u Update) {
t.updates = append(t.updates, u)
if u.Error != nil {
t.d.ClearCache()
etag := t.etags["test/bundle1"]
t.d.SetCache(etag)
return
}
if u.Bundle != nil {
if t.mockBundleActivationError {
t.d.ClearCache()
etag := t.etags["test/bundle1"]
t.d.SetCache(etag)
return
}
}
t.etags["test/bundle1"] = u.ETag
}
type testServer struct {
plugins/bundle/plugin.go
+11 -3
View File
@@ -369,7 +369,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) {
p.log(name).Error("Bundle load failed: %v", u.Error)
p.status[name].SetError(u.Error)
if !p.stopped {
p.downloaders[name].ClearCache()
etag := p.etags[name]
p.downloaders[name].SetCache(etag)
}
return
}
@@ -386,7 +387,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) {
p.log(name).Error("Bundle activation failed: %v", err)
p.status[name].SetError(err)
if !p.stopped {
p.downloaders[name].ClearCache()
etag := p.etags[name]
p.downloaders[name].SetCache(etag)
}
return
}
@@ -399,7 +401,8 @@ func (p *Plugin) process(ctx context.Context, name string, u download.Update) {
p.log(name).Error("Persisting bundle to disk failed: %v", err)
p.status[name].SetError(err)
if !p.stopped {
p.downloaders[name].ClearCache()
etag := p.etags[name]
p.downloaders[name].SetCache(etag)
}
return
}
@@ -617,6 +620,7 @@ func (p *Plugin) getBundlePersistPath() (string, error) {
type bundleLoader interface {
Start(context.Context)
Stop(context.Context)
SetCache(string)
ClearCache()
}
@@ -656,3 +660,7 @@ func (*fileLoader) Stop(context.Context) {
func (*fileLoader) ClearCache() {
}
func (*fileLoader) SetCache(string) {
}