From c1a1f8bb1bfbd5f84e84f80dfb6894980196a402 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 28 Sep 2020 16:05:15 -0400 Subject: [PATCH] Separate out docker dep into package (#482) This also then removes support for docker-based containing starting (for now) from platforms other than windows/darwin/linux as there are build issues on several. --- go.mod | 2 +- go.sum | 4 +- internal/cmd/base/servers.go | 18 +++++-- internal/cmd/commands/dev/dev.go | 5 ++ internal/db/db.go | 87 ++------------------------------ internal/db/testing.go | 6 ++- internal/docker/docker.go | 18 +++++++ internal/docker/supported.go | 76 ++++++++++++++++++++++++++++ internal/oplog/oplog_test.go | 2 +- internal/oplog/testing.go | 69 ++++++------------------- 10 files changed, 142 insertions(+), 145 deletions(-) create mode 100644 internal/docker/docker.go create mode 100644 internal/docker/supported.go diff --git a/go.mod b/go.mod index 6dabbe371..07da272ec 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/golang/protobuf v1.4.2 github.com/google/go-cmp v0.5.2 github.com/grpc-ecosystem/grpc-gateway/v2 v2.0.0-beta.5 - github.com/hashicorp/dbassert v0.0.0-20200602142727-a6709b436ad6 + github.com/hashicorp/dbassert v0.0.0-20200928193136-88d5f2091187 github.com/hashicorp/errwrap v1.0.0 github.com/hashicorp/go-cleanhttp v0.5.1 github.com/hashicorp/go-hclog v0.14.1 diff --git a/go.sum b/go.sum index 080ea97a6..6a668ecd1 100644 --- a/go.sum +++ b/go.sum @@ -531,8 +531,8 @@ github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBt github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/consul/sdk v0.3.0/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= -github.com/hashicorp/dbassert v0.0.0-20200602142727-a6709b436ad6 h1:Nejn3Kli0VXWf26vr9XQEGDjw5CELP8rxOwbG4ruFa8= -github.com/hashicorp/dbassert v0.0.0-20200602142727-a6709b436ad6/go.mod h1:+B5eZq7vXqN4Gxkgjm4ub1bwG6OTcYzFL0r+bMvWorg= +github.com/hashicorp/dbassert v0.0.0-20200928193136-88d5f2091187 h1:KULp/ad4VAihhHelqgAy7610xMVEsCIXQ9Tu+uK6Asg= +github.com/hashicorp/dbassert v0.0.0-20200928193136-88d5f2091187/go.mod h1:+B5eZq7vXqN4Gxkgjm4ub1bwG6OTcYzFL0r+bMvWorg= github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index b20efd3a7..5561b7d41 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/cmd/config" "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/docker" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/sdk/strutil" @@ -447,21 +448,30 @@ func (b *Server) CreateDevDatabase(dialect string, opt ...Option) error { switch b.DatabaseUrl { case "": - c, url, container, err = db.InitDbInDocker(dialect) + c, url, container, err = docker.StartDbInDocker(dialect) // In case of an error, run the cleanup function. If we pass all errors, c should be set to a noop // function before returning from this method defer func() { if !opts.withSkipDatabaseDestruction { - if err := c(); err != nil { - b.Logger.Error("error cleaning up docker container", "error", err) + if c != nil { + if err := c(); err != nil { + b.Logger.Error("error cleaning up docker container", "error", err) + } } } }() - + if err == docker.ErrDockerUnsupported { + return err + } if err != nil { return fmt.Errorf("unable to start dev database with dialect %s: %w", dialect, err) } + _, err := db.InitStore(dialect, c, url) + if err != nil { + return fmt.Errorf("unable to initialize dev database with dialect %s: %w", dialect, err) + } + b.DevDatabaseCleanupFunc = c b.DatabaseUrl = url diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index a0fd045c2..d286af1bb 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/internal/cmd/commands/dev/dev.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/boundary/internal/auth/password" "github.com/hashicorp/boundary/internal/cmd/base" "github.com/hashicorp/boundary/internal/cmd/config" + "github.com/hashicorp/boundary/internal/docker" "github.com/hashicorp/boundary/internal/host/static" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/servers/controller" @@ -374,6 +375,10 @@ func (c *Command) Run(args []string) int { opts = append(opts, base.WithSkipDatabaseDestruction()) } if err := c.CreateDevDatabase("postgres", opts...); err != nil { + if err == docker.ErrDockerUnsupported { + c.UI.Error("Automatically starting a Docker container running Postgres is not currently supported on this platform. Please use -database-url to pass in a URL (or an env var or file reference to a URL) for connecting to an existing empty database.") + return 1 + } c.UI.Error(fmt.Errorf("Error creating dev database container: %w", err).Error()) return 1 } diff --git a/internal/db/db.go b/internal/db/db.go index 8f07fe65e..b940752ab 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -1,20 +1,21 @@ package db import ( - "database/sql" "errors" "fmt" "os" - "strings" - "sync" "github.com/golang-migrate/migrate/v4" "github.com/hashicorp/boundary/internal/db/migrations" + "github.com/hashicorp/boundary/internal/docker" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" "github.com/jinzhu/gorm" "github.com/lib/pq" - "github.com/ory/dockertest/v3" +) + +var ( + StartDbInDocker = docker.StartDbInDocker ) type DbType int @@ -60,69 +61,6 @@ func Migrate(connectionUrl string, migrationsDirectory string) error { return nil } -// InitDbInDocker initializes the data store within docker or an existing -func InitDbInDocker(dialect string) (cleanup func() error, retURL, container string, err error) { - c, url, container, err := StartDbInDocker(dialect) - if err != nil { - return func() error { return nil }, "", "", fmt.Errorf("could not start docker: %w", err) - } - if _, err := InitStore(dialect, c, url); err != nil { - return func() error { return nil }, "", "", fmt.Errorf("error initializing store: %w", err) - } - return c, url, container, nil -} - -var ( - mx = sync.Mutex{} -) - -// StartDbInDocker -func StartDbInDocker(dialect string) (cleanup func() error, retURL, container string, err error) { - // TODO: Debug what part of this method is actually causing race condition issues with our test and fix. - mx.Lock() - defer mx.Unlock() - pool, err := dockertest.NewPool("") - if err != nil { - return func() error { return nil }, "", "", fmt.Errorf("could not connect to docker: %w", err) - } - - var resource *dockertest.Resource - var url string - switch dialect { - case "postgres": - resource, err = pool.Run("postgres", "12", []string{"POSTGRES_PASSWORD=password", "POSTGRES_DB=boundary"}) - url = "postgres://postgres:password@localhost:%s?sslmode=disable" - default: - panic(fmt.Sprintf("unknown dialect %q", dialect)) - } - if err != nil { - return func() error { return nil }, "", "", fmt.Errorf("could not start resource: %w", err) - } - - cleanup = func() error { - return cleanupDockerResource(pool, resource) - } - - url = fmt.Sprintf(url, resource.GetPort("5432/tcp")) - - if err := pool.Retry(func() error { - db, err := sql.Open(dialect, url) - if err != nil { - return fmt.Errorf("error opening %s dev container: %w", dialect, err) - } - - if err := db.Ping(); err != nil { - return err - } - defer db.Close() - return nil - }); err != nil { - return func() error { return nil }, "", "", fmt.Errorf("could not connect to docker: %w", err) - } - - return cleanup, url, resource.Container.Name, nil -} - // InitStore will execute the migrations needed to initialize the store. It // returns true if migrations actually ran; false if we were already current. func InitStore(dialect string, cleanup func() error, url string) (bool, error) { @@ -164,21 +102,6 @@ func InitStore(dialect string, cleanup func() error, url string) (bool, error) { return true, mErr.ErrorOrNil() } -// cleanupDockerResource will clean up the dockertest resources (postgres) -func cleanupDockerResource(pool *dockertest.Pool, resource *dockertest.Resource) error { - var err error - for i := 0; i < 10; i++ { - err = pool.Purge(resource) - if err == nil { - return nil - } - } - if strings.Contains(err.Error(), "No such container") { - return nil - } - return fmt.Errorf("Failed to cleanup local container: %s", err) -} - func GetGormLogFormatter(log hclog.Logger) func(values ...interface{}) (messages []interface{}) { return func(values ...interface{}) (messages []interface{}) { if len(values) > 2 && values[0].(string) == "log" { diff --git a/internal/db/testing.go b/internal/db/testing.go index 4c362a508..9b5729c2f 100644 --- a/internal/db/testing.go +++ b/internal/db/testing.go @@ -20,13 +20,17 @@ import ( // setup the tests (initialize the database one-time and intialized testDatabaseURL). Do not close the returned db. func TestSetup(t *testing.T, dialect string) (*gorm.DB, string) { - cleanup, url, _, err := InitDbInDocker(dialect) + cleanup, url, _, err := StartDbInDocker(dialect) if err != nil { t.Fatal(err) } t.Cleanup(func() { assert.NoError(t, cleanup(), "Got error cleaning up db in docker.") }) + _, err = InitStore(dialect, cleanup, url) + if err != nil { + t.Fatal(err) + } db, err := gorm.Open(dialect, url) if err != nil { t.Fatal(err) diff --git a/internal/docker/docker.go b/internal/docker/docker.go new file mode 100644 index 000000000..f6394377a --- /dev/null +++ b/internal/docker/docker.go @@ -0,0 +1,18 @@ +package docker + +import ( + "errors" + "sync" +) + +var ( + StartDbInDocker func(string) (func() error, string, string, error) = startDbInDockerUnsupported + + ErrDockerUnsupported = errors.New("docker is not currently supported on this platform") + + mx = sync.Mutex{} +) + +func startDbInDockerUnsupported(dialect string) (cleanup func() error, retURL, container string, err error) { + return nil, "", "", ErrDockerUnsupported +} diff --git a/internal/docker/supported.go b/internal/docker/supported.go new file mode 100644 index 000000000..cebff6063 --- /dev/null +++ b/internal/docker/supported.go @@ -0,0 +1,76 @@ +// +build linux darwin windows + +package docker + +import ( + "database/sql" + "fmt" + "strings" + + "github.com/ory/dockertest/v3" +) + +func init() { + StartDbInDocker = startDbInDockerSupported +} + +func startDbInDockerSupported(dialect string) (cleanup func() error, retURL, container string, err error) { + // TODO: Debug what part of this method is actually causing race condition issues with our test and fix. + mx.Lock() + defer mx.Unlock() + pool, err := dockertest.NewPool("") + if err != nil { + return func() error { return nil }, "", "", fmt.Errorf("could not connect to docker: %w", err) + } + + var resource *dockertest.Resource + var url string + switch dialect { + case "postgres": + resource, err = pool.Run("postgres", "12", []string{"POSTGRES_PASSWORD=password", "POSTGRES_DB=boundary"}) + url = "postgres://postgres:password@localhost:%s?sslmode=disable" + default: + panic(fmt.Sprintf("unknown dialect %q", dialect)) + } + if err != nil { + return func() error { return nil }, "", "", fmt.Errorf("could not start resource: %w", err) + } + + cleanup = func() error { + return cleanupDockerResource(pool, resource) + } + + url = fmt.Sprintf(url, resource.GetPort("5432/tcp")) + + if err := pool.Retry(func() error { + db, err := sql.Open(dialect, url) + if err != nil { + return fmt.Errorf("error opening %s dev container: %w", dialect, err) + } + + if err := db.Ping(); err != nil { + return err + } + defer db.Close() + return nil + }); err != nil { + return func() error { return nil }, "", "", fmt.Errorf("could not connect to docker: %w", err) + } + + return cleanup, url, resource.Container.Name, nil +} + +// cleanupDockerResource will clean up the dockertest resources (postgres) +func cleanupDockerResource(pool *dockertest.Pool, resource *dockertest.Resource) error { + var err error + for i := 0; i < 10; i++ { + err = pool.Purge(resource) + if err == nil { + return nil + } + } + if strings.Contains(err.Error(), "No such container") { + return nil + } + return fmt.Errorf("Failed to cleanup local container: %s", err) +} diff --git a/internal/oplog/oplog_test.go b/internal/oplog/oplog_test.go index 0a3535b3e..c5d55d180 100644 --- a/internal/oplog/oplog_test.go +++ b/internal/oplog/oplog_test.go @@ -17,7 +17,7 @@ import ( ) // setup the tests (initialize the database one-time and intialized testDatabaseURL) -func setup(t *testing.T) (func(), *gorm.DB) { +func setup(t *testing.T) (func() error, *gorm.DB) { t.Helper() require := require.New(t) cleanup, url, err := testInitDbInDocker(t) diff --git a/internal/oplog/testing.go b/internal/oplog/testing.go index 7bc497718..5f6696668 100644 --- a/internal/oplog/testing.go +++ b/internal/oplog/testing.go @@ -2,26 +2,24 @@ package oplog import ( "crypto/rand" - "database/sql" - "fmt" - "strings" "testing" "github.com/golang-migrate/migrate/v4" + "github.com/hashicorp/boundary/internal/docker" "github.com/hashicorp/boundary/internal/oplog/oplog_test" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/go-kms-wrapping/wrappers/aead" "github.com/hashicorp/go-uuid" "github.com/jinzhu/gorm" - "github.com/ory/dockertest/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func testCleanup(t *testing.T, cleanupFunc func(), db *gorm.DB) { +func testCleanup(t *testing.T, cleanupFunc func() error, db *gorm.DB) { t.Helper() - cleanupFunc() - err := db.Close() + err := cleanupFunc() + assert.NoError(t, err) + err = db.Close() assert.NoError(t, err) } @@ -54,37 +52,14 @@ func testId(t *testing.T) string { return id } -// testInitDbInDocker initializes postgres within dockertest for the unit tests -func testInitDbInDocker(t *testing.T) (cleanup func(), retURL string, err error) { +func testInitDbInDocker(t *testing.T) (cleanup func() error, retURL string, err error) { t.Helper() - pool, err := dockertest.NewPool("") - require.NoErrorf(t, err, "could not connect to docker: %w", err) - - resource, err := pool.Run("postgres", "12", []string{"POSTGRES_PASSWORD=password", "POSTGRES_DB=boundary"}) - require.NoErrorf(t, err, "could not start resource: %w", err) - - c := func() { - err := testCleanupResource(t, pool, resource) - assert.NoError(t, err) + cleanup, retURL, _, err = docker.StartDbInDocker("postgres") + if err != nil { + t.Fatal(err) } - - url := fmt.Sprintf("postgres://postgres:password@localhost:%s?sslmode=disable", resource.GetPort("5432/tcp")) - - err = pool.Retry(func() error { - db, err := sql.Open("postgres", url) - if err != nil { - return fmt.Errorf("error opening postgres dev container: %w", err) - } - - if err := db.Ping(); err != nil { - return err - } - defer db.Close() - return nil - }) - require.NoErrorf(t, err, "could not connect to docker: %w", err) - testInitStore(t, c, url) - return c, url, nil + testInitStore(t, cleanup, retURL) + return } // testWrapper initializes an AEAD wrapping.Wrapper for testing the oplog @@ -101,30 +76,16 @@ func testWrapper(t *testing.T) wrapping.Wrapper { } // testInitStore will execute the migrations needed to initialize the store for tests -func testInitStore(t *testing.T, cleanup func(), url string) { +func testInitStore(t *testing.T, cleanup func() error, url string) { t.Helper() // run migrations m, err := migrate.New("file://../db/migrations/postgres", url) require.NoError(t, err, "Error creating migrations") if err := m.Up(); err != nil && err != migrate.ErrNoChange { - cleanup() + if err := cleanup(); err != nil { + t.Fatalf("error cleaning up after migration failure: %v", err) + } require.NoError(t, err, "Error running migrations") } } - -// testCleanupResource will clean up the dockertest resources (postgres) -func testCleanupResource(t *testing.T, pool *dockertest.Pool, resource *dockertest.Resource) error { - t.Helper() - var err error - for i := 0; i < 10; i++ { - err = pool.Purge(resource) - if err == nil { - return nil - } - } - if strings.Contains(err.Error(), "No such container") { - return nil - } - return fmt.Errorf("Failed to cleanup local container: %s", err) -}