From 123a5e15e9d4594ecfaea66832d7e383df5ed218 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 22 Mar 2023 16:37:26 +0100 Subject: [PATCH] Acquire lock prior to deploy (#270) Add configuration: ``` bundle: lock: enabled: true force: false ``` The force field can be set by passing the `--force` argument to `bricks bundle deploy`. Doing so means the deployment lock is acquired even if it is currently held. This should only be used in exceptional cases (e.g. a previous deployment has failed to release the lock). --- bundle/bundle.go | 4 ++ bundle/config/bundle.go | 3 ++ bundle/config/lock.go | 19 ++++++++ bundle/config/lock_test.go | 22 +++++++++ bundle/deploy/lock/acquire.go | 54 ++++++++++++++++++++++ bundle/deploy/lock/release.go | 42 +++++++++++++++++ bundle/deployer/deployer.go | 5 +- bundle/phases/deploy.go | 3 ++ cmd/bundle/deploy.go | 7 +++ internal/locker_test.go | 8 ++-- {bundle/deployer => libs/locker}/locker.go | 4 +- 11 files changed, 163 insertions(+), 8 deletions(-) create mode 100644 bundle/config/lock.go create mode 100644 bundle/config/lock_test.go create mode 100644 bundle/deploy/lock/acquire.go create mode 100644 bundle/deploy/lock/release.go rename {bundle/deployer => libs/locker}/locker.go (99%) diff --git a/bundle/bundle.go b/bundle/bundle.go index 02a02c06..5e6d5438 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -12,6 +12,7 @@ import ( "sync" "github.com/databricks/bricks/bundle/config" + "github.com/databricks/bricks/libs/locker" "github.com/databricks/databricks-sdk-go" "github.com/hashicorp/terraform-exec/tfexec" ) @@ -26,6 +27,9 @@ type Bundle struct { // Stores an initialized copy of this bundle's Terraform wrapper. Terraform *tfexec.Terraform + + // Stores the locker responsible for acquiring/releasing a deployment lock. + Locker *locker.Locker } func Load(path string) (*Bundle, error) { diff --git a/bundle/config/bundle.go b/bundle/config/bundle.go index 2871a2f6..982f9c55 100644 --- a/bundle/config/bundle.go +++ b/bundle/config/bundle.go @@ -21,4 +21,7 @@ type Bundle struct { // Terraform holds configuration related to Terraform. // For example, where to find the binary, which version to use, etc. Terraform *Terraform `json:"terraform,omitempty"` + + // Lock configures locking behavior on deployment. + Lock Lock `json:"lock"` } diff --git a/bundle/config/lock.go b/bundle/config/lock.go new file mode 100644 index 00000000..28d5a5ac --- /dev/null +++ b/bundle/config/lock.go @@ -0,0 +1,19 @@ +package config + +type Lock struct { + // Enabled toggles deployment lock. True by default. + // Use a pointer value so that only explicitly configured values are set + // and we don't merge configuration with zero-initialized values. + Enabled *bool `json:"enabled"` + + // Force acquisition of deployment lock even if it is currently held. + // This may be necessary if a prior deployment failed to release the lock. + Force bool `json:"force"` +} + +func (lock Lock) IsEnabled() bool { + if lock.Enabled != nil { + return *lock.Enabled + } + return true +} diff --git a/bundle/config/lock_test.go b/bundle/config/lock_test.go new file mode 100644 index 00000000..f28d046d --- /dev/null +++ b/bundle/config/lock_test.go @@ -0,0 +1,22 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLockDefaults(t *testing.T) { + lock := Lock{} + assert.True(t, lock.IsEnabled()) +} + +func TestLockIsEnabled(t *testing.T) { + lock := Lock{Enabled: new(bool)} + + *lock.Enabled = false + assert.False(t, lock.IsEnabled()) + + *lock.Enabled = true + assert.True(t, lock.IsEnabled()) +} diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go new file mode 100644 index 00000000..5069ea76 --- /dev/null +++ b/bundle/deploy/lock/acquire.go @@ -0,0 +1,54 @@ +package lock + +import ( + "context" + + "github.com/databricks/bricks/bundle" + "github.com/databricks/bricks/libs/locker" + "github.com/databricks/bricks/libs/log" +) + +type acquire struct{} + +func Acquire() bundle.Mutator { + return &acquire{} +} + +func (m *acquire) Name() string { + return "lock:acquire" +} + +func (m *acquire) init(b *bundle.Bundle) error { + user := b.Config.Workspace.CurrentUser.UserName + dir := b.Config.Workspace.StatePath.Workspace + l, err := locker.CreateLocker(user, dir, b.WorkspaceClient()) + if err != nil { + return err + } + + b.Locker = l + return nil +} + +func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { + // Return early if locking is disabled. + if !b.Config.Bundle.Lock.IsEnabled() { + log.Infof(ctx, "Skipping; locking is disabled") + return nil, nil + } + + err := m.init(b) + if err != nil { + return nil, err + } + + force := b.Config.Bundle.Lock.Force + log.Infof(ctx, "Acquiring deployment lock (force: %v)", force) + err = b.Locker.Lock(ctx, force) + if err != nil { + log.Errorf(ctx, "Failed to acquire deployment lock: %v", err) + return nil, err + } + + return nil, nil +} diff --git a/bundle/deploy/lock/release.go b/bundle/deploy/lock/release.go new file mode 100644 index 00000000..2af5974d --- /dev/null +++ b/bundle/deploy/lock/release.go @@ -0,0 +1,42 @@ +package lock + +import ( + "context" + + "github.com/databricks/bricks/bundle" + "github.com/databricks/bricks/libs/log" +) + +type release struct{} + +func Release() bundle.Mutator { + return &release{} +} + +func (m *release) Name() string { + return "lock:release" +} + +func (m *release) Apply(ctx context.Context, b *bundle.Bundle) ([]bundle.Mutator, error) { + // Return early if locking is disabled. + if !b.Config.Bundle.Lock.IsEnabled() { + log.Infof(ctx, "Skipping; locking is disabled") + return nil, nil + } + + // Return early if the locker is not set. + // It is likely an error occurred prior to initialization of the locker instance. + if b.Locker == nil { + log.Warnf(ctx, "Unable to release lock if locker is not configured") + return nil, nil + } + + log.Infof(ctx, "Releasing deployment lock") + err := b.Locker.Unlock(ctx) + if err != nil { + log.Errorf(ctx, "Failed to release deployment lock: %v", err) + return nil, err + } + + return nil, nil +} diff --git a/bundle/deployer/deployer.go b/bundle/deployer/deployer.go index d073790f..abcd15d0 100644 --- a/bundle/deployer/deployer.go +++ b/bundle/deployer/deployer.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "github.com/databricks/bricks/libs/locker" "github.com/databricks/bricks/libs/log" "github.com/databricks/databricks-sdk-go" "github.com/hashicorp/terraform-exec/tfexec" @@ -61,7 +62,7 @@ type Deployer struct { localRoot string remoteRoot string env string - locker *Locker + locker *locker.Locker wsc *databricks.WorkspaceClient } @@ -70,7 +71,7 @@ func Create(ctx context.Context, env, localRoot, remoteRoot string, wsc *databri if err != nil { return nil, err } - newLocker, err := CreateLocker(user.UserName, remoteRoot, wsc) + newLocker, err := locker.CreateLocker(user.UserName, remoteRoot, wsc) if err != nil { return nil, err } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 684a59b8..299cd3eb 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -4,6 +4,7 @@ import ( "github.com/databricks/bricks/bundle" "github.com/databricks/bricks/bundle/artifacts" "github.com/databricks/bricks/bundle/deploy/files" + "github.com/databricks/bricks/bundle/deploy/lock" "github.com/databricks/bricks/bundle/deploy/terraform" ) @@ -12,11 +13,13 @@ func Deploy() bundle.Mutator { return newPhase( "deploy", []bundle.Mutator{ + lock.Acquire(), files.Upload(), artifacts.UploadAll(), terraform.Interpolate(), terraform.Write(), terraform.Apply(), + lock.Release(), }, ) } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 5b786c0f..c96da816 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -14,6 +14,10 @@ var deployCmd = &cobra.Command{ PreRunE: root.MustConfigureBundle, RunE: func(cmd *cobra.Command, args []string) error { b := bundle.Get(cmd.Context()) + + // If `--force` is specified, force acquisition of the deployment lock. + b.Config.Bundle.Lock.Force = force + return bundle.Apply(cmd.Context(), b, []bundle.Mutator{ phases.Initialize(), phases.Build(), @@ -22,6 +26,9 @@ var deployCmd = &cobra.Command{ }, } +var force bool + func init() { AddCommand(deployCmd) + deployCmd.Flags().BoolVar(&force, "force", false, "Force acquisition of deployment lock.") } diff --git a/internal/locker_test.go b/internal/locker_test.go index 058f0d78..641e3fcc 100644 --- a/internal/locker_test.go +++ b/internal/locker_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/databricks/bricks/bundle/deployer" + lockpkg "github.com/databricks/bricks/libs/locker" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/repos" "github.com/stretchr/testify/assert" @@ -52,13 +52,13 @@ func TestAccLock(t *testing.T) { // Keep single locker unlocked. // We use this to check on the current lock through GetActiveLockState. - locker, err := deployer.CreateLocker("humpty.dumpty@databricks.com", remoteProjectRoot, wsc) + locker, err := lockpkg.CreateLocker("humpty.dumpty@databricks.com", remoteProjectRoot, wsc) require.NoError(t, err) lockerErrs := make([]error, numConcurrentLocks) - lockers := make([]*deployer.Locker, numConcurrentLocks) + lockers := make([]*lockpkg.Locker, numConcurrentLocks) for i := 0; i < numConcurrentLocks; i++ { - lockers[i], err = deployer.CreateLocker("humpty.dumpty@databricks.com", remoteProjectRoot, wsc) + lockers[i], err = lockpkg.CreateLocker("humpty.dumpty@databricks.com", remoteProjectRoot, wsc) require.NoError(t, err) } diff --git a/bundle/deployer/locker.go b/libs/locker/locker.go similarity index 99% rename from bundle/deployer/locker.go rename to libs/locker/locker.go index 136203e2..676b9513 100644 --- a/bundle/deployer/locker.go +++ b/libs/locker/locker.go @@ -1,4 +1,4 @@ -package deployer +package locker import ( "bytes" @@ -183,7 +183,7 @@ func (locker *Locker) Unlock(ctx context.Context) error { func (locker *Locker) RemotePath() string { // Note: remote paths are scoped to `targetDir`. Also see [CreateLocker]. - return ".bundle/deploy.lock" + return "deploy.lock" } func CreateLocker(user string, targetDir string, w *databricks.WorkspaceClient) (*Locker, error) {