From fff95e79929d023f7b4a6ea8888e42fd5f258ba7 Mon Sep 17 00:00:00 2001
From: Owen Cliffe <owen.cliffe@oracle.com>
Date: Sat, 7 Jul 2018 10:37:19 +0100
Subject: [PATCH] Clean up/make consistent the APIs for registering core
 components,  make Docker an optional component  at compile time (#1111)

---
 api/agent/agent.go                     | 11 +++++++----
 api/agent/agent_test.go                |  1 +
 api/agent/drivers/docker/docker.go     |  6 ++++++
 api/agent/drivers/register.go          | 26 ++++++++++++++++++++++++++
 api/datastore/datastore.go             |  6 +++---
 api/datastore/sql/dbhelper/dbhelper.go |  6 +++---
 api/datastore/sql/mysql/mysql.go       |  2 +-
 api/datastore/sql/postgres/postgres.go |  2 +-
 api/datastore/sql/sql.go               |  4 ++--
 api/datastore/sql/sqlite/sqlite.go     |  2 +-
 api/logs/log.go                        |  6 +++---
 api/logs/s3/s3.go                      |  2 +-
 api/server/defaultexts/defaultexts.go  |  1 +
 api/server/server_test.go              |  1 +
 test/fn-system-tests/system_test.go    |  6 +++++-
 15 files changed, 62 insertions(+), 20 deletions(-)
 create mode 100644 api/agent/drivers/register.go

diff --git a/api/agent/agent.go b/api/agent/agent.go
index c57e92c07..bd4b1ef66 100644
--- a/api/agent/agent.go
+++ b/api/agent/agent.go
@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/fnproject/fn/api/agent/drivers"
-	"github.com/fnproject/fn/api/agent/drivers/docker"
 	"github.com/fnproject/fn/api/agent/protocol"
 	"github.com/fnproject/fn/api/common"
 	"github.com/fnproject/fn/api/id"
@@ -142,7 +141,11 @@ func New(da CallHandler, options ...AgentOption) Agent {
 	logrus.Infof("agent starting cfg=%+v", a.cfg)
 
 	if a.driver == nil {
-		a.driver = NewDockerDriver(&a.cfg)
+		d, err := NewDockerDriver(&a.cfg)
+		if err != nil {
+			logrus.WithError(err).Fatal("failed to create docker driver ")
+		}
+		a.driver = d
 	}
 
 	a.resources = NewResourceTracker(&a.cfg)
@@ -201,8 +204,8 @@ func WithCallOverrider(fn CallOverrider) AgentOption {
 }
 
 // NewDockerDriver creates a default docker driver from agent config
-func NewDockerDriver(cfg *AgentConfig) *docker.DockerDriver {
-	return docker.NewDocker(drivers.Config{
+func NewDockerDriver(cfg *AgentConfig) (drivers.Driver, error) {
+	return drivers.New("docker", drivers.Config{
 		DockerNetworks:       cfg.DockerNetworks,
 		ServerVersion:        cfg.MinDockerVersion,
 		PreForkPoolSize:      cfg.PreForkPoolSize,
diff --git a/api/agent/agent_test.go b/api/agent/agent_test.go
index 385920984..8159cfc50 100644
--- a/api/agent/agent_test.go
+++ b/api/agent/agent_test.go
@@ -16,6 +16,7 @@ import (
 	"testing"
 	"time"
 
+	_ "github.com/fnproject/fn/api/agent/drivers/docker"
 	"github.com/fnproject/fn/api/id"
 	"github.com/fnproject/fn/api/logs"
 	"github.com/fnproject/fn/api/models"
diff --git a/api/agent/drivers/docker/docker.go b/api/agent/drivers/docker/docker.go
index a26e1b010..0b3d8a929 100644
--- a/api/agent/drivers/docker/docker.go
+++ b/api/agent/drivers/docker/docker.go
@@ -646,3 +646,9 @@ func (w *waitResult) wait(ctx context.Context) (status string, err error) {
 }
 
 var _ drivers.Driver = &DockerDriver{}
+
+func init() {
+	drivers.Register("docker", func(config drivers.Config) (drivers.Driver, error) {
+		return NewDocker(config), nil
+	})
+}
diff --git a/api/agent/drivers/register.go b/api/agent/drivers/register.go
new file mode 100644
index 000000000..05025a920
--- /dev/null
+++ b/api/agent/drivers/register.go
@@ -0,0 +1,26 @@
+package drivers
+
+import (
+	"fmt"
+	"github.com/sirupsen/logrus"
+)
+
+type DriverFunc func(config Config) (Driver, error)
+
+var drivers = make(map[string]DriverFunc)
+
+// Register adds  a container driver by name to this process
+func Register(name string, driverFunc DriverFunc) {
+	logrus.Infof("Registering container driver '%s'", name)
+	drivers[name] = driverFunc
+}
+
+// New Instantiates a driver by name
+func New(driverName string, config Config) (Driver, error) {
+	driverFunc, ok := drivers[driverName]
+
+	if !ok {
+		return nil, fmt.Errorf("agent driver \"%s\" is not registered", driverName)
+	}
+	return driverFunc(config)
+}
diff --git a/api/datastore/datastore.go b/api/datastore/datastore.go
index 0d6b02d20..b840e42a2 100644
--- a/api/datastore/datastore.go
+++ b/api/datastore/datastore.go
@@ -44,8 +44,8 @@ type Provider interface {
 
 var providers []Provider
 
-// AddProvider globally registers a data store provider
-func AddProvider(provider Provider) {
-	logrus.Infof("Adding DataStore provider %s", provider)
+// Register globally registers a data store provider
+func Register(provider Provider) {
+	logrus.Infof("Registering data store provider '%s'", provider)
 	providers = append(providers, provider)
 }
diff --git a/api/datastore/sql/dbhelper/dbhelper.go b/api/datastore/sql/dbhelper/dbhelper.go
index 76bc854b1..45ac3611e 100644
--- a/api/datastore/sql/dbhelper/dbhelper.go
+++ b/api/datastore/sql/dbhelper/dbhelper.go
@@ -10,9 +10,9 @@ import (
 
 var sqlHelpers []Helper
 
-//Add registers a new SQL helper
-func Add(helper Helper) {
-	logrus.Infof("Registering DB helper %s", helper)
+//Register registers a new SQL helper
+func Register(helper Helper) {
+	logrus.Infof("Registering sql helper '%s'", helper)
 	sqlHelpers = append(sqlHelpers, helper)
 }
 
diff --git a/api/datastore/sql/mysql/mysql.go b/api/datastore/sql/mysql/mysql.go
index 14c314f69..cc590a464 100644
--- a/api/datastore/sql/mysql/mysql.go
+++ b/api/datastore/sql/mysql/mysql.go
@@ -57,5 +57,5 @@ func (mysqlHelper) IsDuplicateKeyError(err error) bool {
 }
 
 func init() {
-	dbhelper.Add(mysqlHelper(0))
+	dbhelper.Register(mysqlHelper(0))
 }
diff --git a/api/datastore/sql/postgres/postgres.go b/api/datastore/sql/postgres/postgres.go
index 8a32f3fc6..1284a5a89 100644
--- a/api/datastore/sql/postgres/postgres.go
+++ b/api/datastore/sql/postgres/postgres.go
@@ -59,5 +59,5 @@ func (postgresHelper) IsDuplicateKeyError(err error) bool {
 }
 
 func init() {
-	dbhelper.Add(postgresHelper(0))
+	dbhelper.Register(postgresHelper(0))
 }
diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go
index 360a5fa07..ce69971d1 100644
--- a/api/datastore/sql/sql.go
+++ b/api/datastore/sql/sql.go
@@ -1443,6 +1443,6 @@ func (ds *SQLStore) Close() error {
 }
 
 func init() {
-	datastore.AddProvider(sqlDsProvider(0))
-	logs.AddProvider(sqlLogsProvider(0))
+	datastore.Register(sqlDsProvider(0))
+	logs.Register(sqlLogsProvider(0))
 }
diff --git a/api/datastore/sql/sqlite/sqlite.go b/api/datastore/sql/sqlite/sqlite.go
index 4f961f2c8..0dd14efb2 100644
--- a/api/datastore/sql/sqlite/sqlite.go
+++ b/api/datastore/sql/sqlite/sqlite.go
@@ -70,5 +70,5 @@ func (sqliteHelper) IsDuplicateKeyError(err error) bool {
 }
 
 func init() {
-	dbhelper.Add(sqliteHelper(0))
+	dbhelper.Register(sqliteHelper(0))
 }
diff --git a/api/logs/log.go b/api/logs/log.go
index 231baa913..45db2f632 100644
--- a/api/logs/log.go
+++ b/api/logs/log.go
@@ -24,9 +24,9 @@ type Provider interface {
 
 var providers []Provider
 
-// AddProvider globally registers a new LogStore provider
-func AddProvider(pf Provider) {
-	logrus.Infof("Adding log provider %s", pf)
+// Register globally registers a new LogStore provider
+func Register(pf Provider) {
+	logrus.Infof("Registering log provider '%s'", pf)
 	providers = append(providers, pf)
 }
 
diff --git a/api/logs/s3/s3.go b/api/logs/s3/s3.go
index 961f33a3d..847a92bd3 100644
--- a/api/logs/s3/s3.go
+++ b/api/logs/s3/s3.go
@@ -469,5 +469,5 @@ func makeKeys(names []string) []tag.Key {
 }
 
 func init() {
-	logs.AddProvider(s3StoreProvider(0))
+	logs.Register(s3StoreProvider(0))
 }
diff --git a/api/server/defaultexts/defaultexts.go b/api/server/defaultexts/defaultexts.go
index d9e3a011a..26274d25e 100644
--- a/api/server/defaultexts/defaultexts.go
+++ b/api/server/defaultexts/defaultexts.go
@@ -5,6 +5,7 @@ package defaultexts
 
 import (
 	// import all datastore/log/mq modules for runtime config
+	_ "github.com/fnproject/fn/api/agent/drivers/docker"
 	_ "github.com/fnproject/fn/api/datastore/sql"
 	_ "github.com/fnproject/fn/api/datastore/sql/mysql"
 	_ "github.com/fnproject/fn/api/datastore/sql/postgres"
diff --git a/api/server/server_test.go b/api/server/server_test.go
index 2a1b93c79..c78374f8e 100644
--- a/api/server/server_test.go
+++ b/api/server/server_test.go
@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/fnproject/fn/api/agent"
+	_ "github.com/fnproject/fn/api/agent/drivers/docker"
 	"github.com/fnproject/fn/api/datastore"
 	"github.com/fnproject/fn/api/datastore/sql"
 	_ "github.com/fnproject/fn/api/datastore/sql/sqlite"
diff --git a/test/fn-system-tests/system_test.go b/test/fn-system-tests/system_test.go
index 638a62e13..f811bc2a8 100644
--- a/test/fn-system-tests/system_test.go
+++ b/test/fn-system-tests/system_test.go
@@ -259,8 +259,12 @@ func SetUpPureRunnerNode(ctx context.Context, nodeNum int) (*server.Server, erro
 	}
 
 	// customer driver that overrides generic docker driver
+	d, err := agent.NewDockerDriver(cfg)
+	if err != nil {
+		return nil, err
+	}
 	drv := &customDriver{
-		drv: agent.NewDockerDriver(cfg),
+		drv: d,
 	}
 
 	// inner agent for pure-runners