Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add StateLocker interface to backend operations #17422

Merged
merged 3 commits into from
Feb 27, 2018
Merged

Add StateLocker interface to backend operations #17422

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9a7680
create clistate.Locker interface
jbardin Feb 23, 2018
bdd475e
use the new clistate.Locker in commands
jbardin Feb 23, 2018
2b97585
improve clistate.Locker docs
jbardin Feb 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions backend/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"time"

"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/config/module"
"github.com/hashicorp/terraform/state"
"github.com/hashicorp/terraform/terraform"
Expand Down Expand Up @@ -135,6 +136,10 @@ type Operation struct {
// state.Lockers for its duration, and Unlock when complete.
LockState bool

// StateLocker is used to lock the state while providing UI feedback to the
// user. This will be supplied by the Backend itself.
StateLocker clistate.Locker

// The duration to retry obtaining a State lock.
StateLockTimeout time.Duration

Expand Down
13 changes: 13 additions & 0 deletions backend/local/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"sync"

"github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/state"
"github.com/hashicorp/terraform/terraform"
Expand Down Expand Up @@ -260,12 +261,24 @@ func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend.
cancelCtx, cancel := context.WithCancel(context.Background())
runningOp.Cancel = cancel

if op.LockState {
op.StateLocker = clistate.NewLocker(stopCtx, op.StateLockTimeout, b.CLI, b.Colorize())
} else {
op.StateLocker = clistate.NewNoopLocker()
}

// Do it
go func() {
defer done()
defer stop()
defer cancel()

// the state was locked during context creation, unlock the state when
// the operation completes
defer func() {
runningOp.Err = op.StateLocker.Unlock(runningOp.Err)
}()

defer b.opLock.Unlock()
f(stopCtx, cancelCtx, op, runningOp)
}()
Expand Down
20 changes: 0 additions & 20 deletions backend/local/backend_apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/hashicorp/errwrap"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/command/format"
"github.com/hashicorp/terraform/config/module"
"github.com/hashicorp/terraform/state"
Expand Down Expand Up @@ -55,25 +54,6 @@ func (b *Local) opApply(
return
}

if op.LockState {
lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
defer cancel()

lockInfo := state.NewLockInfo()
lockInfo.Operation = op.Type.String()
lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
if err != nil {
runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
return
}

defer func() {
if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
runningOp.Err = multierror.Append(runningOp.Err, err)
}
}()
}

// Setup the state
runningOp.State = tfCtx.State()

Expand Down
12 changes: 12 additions & 0 deletions backend/local/backend_local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package local

import (
"context"
"errors"
"log"

"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/command/format"

"github.com/hashicorp/terraform/tfdiags"
Expand All @@ -20,6 +22,12 @@ func (b *Local) Context(op *backend.Operation) (*terraform.Context, state.State,
// to ask for input/validate.
op.Type = backend.OperationTypeInvalid

if op.LockState {
op.StateLocker = clistate.NewLocker(context.Background(), op.StateLockTimeout, b.CLI, b.Colorize())
} else {
op.StateLocker = clistate.NewNoopLocker()
}

return b.context(op)
}

Expand All @@ -30,6 +38,10 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State,
return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err)
}

if err := op.StateLocker.Lock(s, op.Type.String()); err != nil {
return nil, nil, errwrap.Wrapf("Error locking state: {{err}}", err)
}

if err := s.RefreshState(); err != nil {
return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err)
}
Expand Down
22 changes: 0 additions & 22 deletions backend/local/backend_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@ import (
"strings"

"github.com/hashicorp/errwrap"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/command/format"
"github.com/hashicorp/terraform/config/module"
"github.com/hashicorp/terraform/state"
"github.com/hashicorp/terraform/terraform"
)

Expand Down Expand Up @@ -62,25 +59,6 @@ func (b *Local) opPlan(
return
}

if op.LockState {
lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
defer cancel()

lockInfo := state.NewLockInfo()
lockInfo.Operation = op.Type.String()
lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
if err != nil {
runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
return
}

defer func() {
if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
runningOp.Err = multierror.Append(runningOp.Err, err)
}
}()
}

// Setup the state
runningOp.State = tfCtx.State()

Expand Down
22 changes: 0 additions & 22 deletions backend/local/backend_refresh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@ import (
"strings"

"github.com/hashicorp/errwrap"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/config/module"
"github.com/hashicorp/terraform/state"
"github.com/hashicorp/terraform/terraform"
)

Expand Down Expand Up @@ -53,25 +50,6 @@ func (b *Local) opRefresh(
return
}

if op.LockState {
lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
defer cancel()

lockInfo := state.NewLockInfo()
lockInfo.Operation = op.Type.String()
lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
if err != nil {
runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
return
}

defer func() {
if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
runningOp.Err = multierror.Append(runningOp.Err, err)
}
}()
}

// Set our state
runningOp.State = opState.State()
if runningOp.State.Empty() || !runningOp.State.HasResources() {
Expand Down
114 changes: 94 additions & 20 deletions command/clistate/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"context"
"fmt"
"strings"
"sync"
"time"

"github.com/hashicorp/errwrap"
multierror "github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform/helper/slowmessage"
"github.com/hashicorp/terraform/state"
"github.com/mitchellh/cli"
Expand Down Expand Up @@ -48,47 +50,119 @@ that no one else is holding a lock.
`
)

// Lock locks the given state and outputs to the user if locking
// is taking longer than the threshold. The lock is retried until the context
// is cancelled.
func Lock(ctx context.Context, s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) {
var lockID string
// Locker allows for more convenient locking, by creating the timeout context
// and LockInfo for the caller, while storing any related data required for
// Unlock.
type Locker interface {
// Lock the provided state, storing the reason string in the LockInfo.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was a little confusing to me at first because "the LockInfo" seems like an implementation detail of this function, rather than something visible on this interface. However, reading the code below I see that it gets created and passed into the LockWithContext function; I think it could help to clarify a little what side-effects this function has for the given state, since the interactions between subsystems here aren't obvious.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see the confusion. It's because it used to be visible in the interface -- you have to pass a LockInfo to the state Lock method. I'll clarify this a little more

Lock(s state.State, reason string) error
// Unlock the previously locked state.
// An optional error can be passed in, and will be combined with any error
// from the Unlock operation.
Unlock(error) error
}

type locker struct {
mu sync.Mutex
ctx context.Context
timeout time.Duration
state state.State
ui cli.Ui
color *colorstring.Colorize
lockID string
}

// Create a new Locker.
// The provided context will be used for lock cancellation, and combined with
// the timeout duration. Lock progress will be be reported to the user through
// the provided UI.
func NewLocker(
ctx context.Context,
timeout time.Duration,
ui cli.Ui,
color *colorstring.Colorize) Locker {

l := &locker{
ctx: ctx,
timeout: timeout,
ui: ui,
color: color,
}
return l
}

// Locker locks the given state and outputs to the user if locking is taking
// longer than the threshold. The lock is retried until the context is
// cancelled.
func (l *locker) Lock(s state.State, reason string) error {
l.mu.Lock()
defer l.mu.Unlock()

l.state = s

ctx, cancel := context.WithTimeout(l.ctx, l.timeout)
defer cancel()

lockInfo := state.NewLockInfo()
lockInfo.Operation = reason

err := slowmessage.Do(LockThreshold, func() error {
id, err := state.LockWithContext(ctx, s, info)
lockID = id
id, err := state.LockWithContext(ctx, s, lockInfo)
l.lockID = id
return err
}, func() {
if ui != nil {
ui.Output(color.Color(LockMessage))
if l.ui != nil {
l.ui.Output(l.color.Color(LockMessage))
}
})

if err != nil {
err = errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err)
return errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err)
}

return lockID, err
return nil
}

// Unlock unlocks the given state and outputs to the user if the
// unlock fails what can be done.
func Unlock(s state.State, id string, ui cli.Ui, color *colorstring.Colorize) error {
func (l *locker) Unlock(parentErr error) error {
l.mu.Lock()
defer l.mu.Unlock()

if l.lockID == "" {
return parentErr
}

err := slowmessage.Do(LockThreshold, func() error {
return s.Unlock(id)
return l.state.Unlock(l.lockID)
}, func() {
if ui != nil {
ui.Output(color.Color(UnlockMessage))
if l.ui != nil {
l.ui.Output(l.color.Color(UnlockMessage))
}
})

if err != nil {
ui.Output(color.Color(fmt.Sprintf(
l.ui.Output(l.color.Color(fmt.Sprintf(
"\n"+strings.TrimSpace(UnlockErrorMessage)+"\n", err)))

err = fmt.Errorf(
"Error releasing the state lock. Please see the longer error message above.")
if parentErr != nil {
parentErr = multierror.Append(parentErr, err)
}
}

return parentErr

}

type noopLocker struct{}

// NewNoopLocker returns a valid Locker that does nothing.
func NewNoopLocker() Locker {
return noopLocker{}
}

func (l noopLocker) Lock(state.State, string) error {
return nil
}

func (l noopLocker) Unlock(err error) error {
return err
}
Loading