Skip to content

Commit

Permalink
server: Fix panic in /health?bundle=true
Browse files Browse the repository at this point in the history
The issue was that with bundles loaded from the file system we would
not initialize the mutex used for checking bundle status.

This fixes the initialization and prevents the error. Health status
works as expected now.

Fixes: #1703
Signed-off-by: Patrick East <[email protected]>
(cherry picked from commit 3248c2d)
  • Loading branch information
patrick-east committed Aug 30, 2019
1 parent 60003bd commit 6e84c3e
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
3 changes: 1 addition & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type Server struct {
runtime *ast.Term
httpListeners []httpListener
bundleStatuses map[string]*bundlePlugin.Status
bundleStatusMtx *sync.RWMutex
bundleStatusMtx sync.RWMutex
}

// Loop will contain all the calls from the server that we'll be listening on.
Expand Down Expand Up @@ -172,7 +172,6 @@ func (s *Server) Init(ctx context.Context) (*Server, error) {

bp := bundlePlugin.Lookup(s.manager)
if bp != nil {
s.bundleStatusMtx = new(sync.RWMutex)

// initialize statuses to empty defaults for server /health check
s.bundleStatuses = map[string]*bundlePlugin.Status{}
Expand Down
38 changes: 25 additions & 13 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"reflect"
"sort"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -81,7 +80,6 @@ func TestUnversionedGetHealthCheckBundleActivationSingle(t *testing.T) {
// Initialize the server as if a bundle plugin was
// configured on the manager.
f.server.manager.Register(pluginBundle.Name, &pluginBundle.Plugin{})
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
bundleName: &pluginBundle.Status{Name: bundleName},
}
Expand All @@ -106,6 +104,31 @@ func TestUnversionedGetHealthCheckBundleActivationSingle(t *testing.T) {
}
}

func TestUnversionedGetHealthCheckBundleActivationSingleLegacy(t *testing.T) {

// Initialize the server as if there is no bundle plugin

f := newFixture(t)

ctx := context.Background()

err := storage.Txn(ctx, f.server.store, storage.WriteParams, func(txn storage.Transaction) error {
return bundle.LegacyWriteManifestToStore(ctx, f.server.store, txn, bundle.Manifest{
Revision: "a",
})
})

if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

// The heath check should now respond as healthy
req := newReqUnversioned(http.MethodGet, "/health?bundle=true", "")
if err := f.executeRequest(req, 200, `{}`); err != nil {
t.Fatal(err)
}
}

func TestUnversionedGetHealthCheckBundleActivationMulti(t *testing.T) {

f := newFixture(t)
Expand All @@ -118,7 +141,6 @@ func TestUnversionedGetHealthCheckBundleActivationMulti(t *testing.T) {
"b3": {Service: "s3", Resource: "bundle.tar.gz"},
}}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
"b2": {Name: "b2"},
Expand Down Expand Up @@ -187,10 +209,6 @@ func TestInitWithBundlePlugin(t *testing.T) {
t.Error("server.hasBundle should be true")
}

if server.bundleStatusMtx == nil {
t.Error("server.bundleStatusMtx should be initialized")
}

isActivated := server.bundlesActivated()
if isActivated {
t.Error("bundle should not be initialized to activated status")
Expand Down Expand Up @@ -225,10 +243,6 @@ func TestInitWithBundlePluginMultiBundle(t *testing.T) {
t.Error("server.hasBundle should be true")
}

if server.bundleStatusMtx == nil {
t.Error("server.bundleStatusMtx should be initialized")
}

isActivated := server.bundlesActivated()
if isActivated {
t.Error("bundle should not be initialized to activated")
Expand Down Expand Up @@ -1662,7 +1676,6 @@ func TestDataProvenanceSingleBundle(t *testing.T) {
// Initialize as if a bundle plugin is running
bp := pluginBundle.New(&pluginBundle.Config{Name: "b1"}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)
f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
}
Expand Down Expand Up @@ -1774,7 +1787,6 @@ func TestDataProvenanceMultiBundle(t *testing.T) {
}}, f.server.manager)
f.server.manager.Register(pluginBundle.Name, bp)

f.server.bundleStatusMtx = new(sync.RWMutex)
f.server.bundleStatuses = map[string]*pluginBundle.Status{
"b1": {Name: "b1"},
"b2": {Name: "b2"},
Expand Down

0 comments on commit 6e84c3e

Please sign in to comment.