From fe05609c5e197b842f55434c9b99413822c13e74 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 30 Nov 2018 19:31:58 +0100 Subject: [PATCH] backend/remote: support the new force-unlock API Add support for the new `force-unlock` API and at the same time improve performance a bit by reducing the amount of API calls made when using the remote backend for state storage only. --- backend/remote/backend.go | 40 ++++++++---------- backend/remote/backend_mock.go | 18 ++++++++ backend/remote/backend_state.go | 75 +++++++++++---------------------- 3 files changed, 60 insertions(+), 73 deletions(-) diff --git a/backend/remote/backend.go b/backend/remote/backend.go index 86812137a..7f4c24326 100644 --- a/backend/remote/backend.go +++ b/backend/remote/backend.go @@ -63,8 +63,8 @@ type Remote struct { // workspace is used to map the default workspace to a remote workspace. workspace string - // prefix is used to filter down a set of workspaces that use a single. - // configuration + // prefix is used to filter down a set of workspaces that use a single + // configuration. prefix string // schema defines the configuration for the backend. @@ -400,7 +400,9 @@ func (b *Remote) DeleteWorkspace(name string) error { client := &remoteClient{ client: b.client, organization: b.organization, - workspace: name, + workspace: &tfe.Workspace{ + Name: name, + }, } return client.Delete() @@ -415,19 +417,6 @@ func (b *Remote) StateMgr(name string) (state.State, error) { return nil, backend.ErrWorkspacesNotSupported } - workspaces, err := b.workspaces() - if err != nil { - return nil, fmt.Errorf("Error retrieving workspaces: %v", err) - } - - exists := false - for _, workspace := range workspaces { - if name == workspace { - exists = true - break - } - } - // Configure the remote workspace name. switch { case name == backend.DefaultStateName: @@ -436,7 +425,12 @@ func (b *Remote) StateMgr(name string) (state.State, error) { name = b.prefix + name } - if !exists { + workspace, err := b.client.Workspaces.Read(context.Background(), b.organization, name) + if err != nil && err != tfe.ErrResourceNotFound { + return nil, fmt.Errorf("Failed to retrieve workspace %s: %v", name, err) + } + + if err == tfe.ErrResourceNotFound { options := tfe.WorkspaceCreateOptions{ Name: tfe.String(name), } @@ -447,7 +441,7 @@ func (b *Remote) StateMgr(name string) (state.State, error) { options.TerraformVersion = tfe.String(version.String()) } - _, err = b.client.Workspaces.Create(context.Background(), b.organization, options) + workspace, err = b.client.Workspaces.Create(context.Background(), b.organization, options) if err != nil { return nil, fmt.Errorf("Error creating workspace %s: %v", name, err) } @@ -456,7 +450,7 @@ func (b *Remote) StateMgr(name string) (state.State, error) { client := &remoteClient{ client: b.client, organization: b.organization, - workspace: name, + workspace: workspace, // This is optionally set during Terraform Enterprise runs. runID: os.Getenv("TFE_RUN_ID"), @@ -468,16 +462,16 @@ func (b *Remote) StateMgr(name string) (state.State, error) { // Operation implements backend.Enhanced. func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) { // Get the remote workspace name. - workspace := op.Workspace + name := op.Workspace switch { case op.Workspace == backend.DefaultStateName: - workspace = b.workspace + name = b.workspace case b.prefix != "" && !strings.HasPrefix(op.Workspace, b.prefix): - workspace = b.prefix + op.Workspace + name = b.prefix + op.Workspace } // Retrieve the workspace for this operation. - w, err := b.client.Workspaces.Read(ctx, b.organization, workspace) + w, err := b.client.Workspaces.Read(ctx, b.organization, name) if err != nil { return nil, generalError("Failed to retrieve workspace", err) } diff --git a/backend/remote/backend_mock.go b/backend/remote/backend_mock.go index c65de0258..8f9a41f03 100644 --- a/backend/remote/backend_mock.go +++ b/backend/remote/backend_mock.go @@ -967,6 +967,9 @@ func (m *mockWorkspaces) Lock(ctx context.Context, workspaceID string, options t if !ok { return nil, tfe.ErrResourceNotFound } + if w.Locked { + return nil, tfe.ErrWorkspaceLocked + } w.Locked = true return w, nil } @@ -976,6 +979,21 @@ func (m *mockWorkspaces) Unlock(ctx context.Context, workspaceID string) (*tfe.W if !ok { return nil, tfe.ErrResourceNotFound } + if !w.Locked { + return nil, tfe.ErrWorkspaceNotLocked + } + w.Locked = false + return w, nil +} + +func (m *mockWorkspaces) ForceUnlock(ctx context.Context, workspaceID string) (*tfe.Workspace, error) { + w, ok := m.workspaceIDs[workspaceID] + if !ok { + return nil, tfe.ErrResourceNotFound + } + if !w.Locked { + return nil, tfe.ErrWorkspaceNotLocked + } w.Locked = false return w, nil } diff --git a/backend/remote/backend_state.go b/backend/remote/backend_state.go index 99b795b4e..0f4375e7d 100644 --- a/backend/remote/backend_state.go +++ b/backend/remote/backend_state.go @@ -18,24 +18,14 @@ type remoteClient struct { lockInfo *state.LockInfo organization string runID string - workspace string + workspace *tfe.Workspace } // Get the remote state. func (r *remoteClient) Get() (*remote.Payload, error) { ctx := context.Background() - // Retrieve the workspace for which to create a new state. - w, err := r.client.Workspaces.Read(ctx, r.organization, r.workspace) - if err != nil { - if err == tfe.ErrResourceNotFound { - // If no state exists, then return nil. - return nil, nil - } - return nil, fmt.Errorf("Error retrieving workspace: %v", err) - } - - sv, err := r.client.StateVersions.Current(ctx, w.ID) + sv, err := r.client.StateVersions.Current(ctx, r.workspace.ID) if err != nil { if err == tfe.ErrResourceNotFound { // If no state exists, then return nil. @@ -67,12 +57,6 @@ func (r *remoteClient) Get() (*remote.Payload, error) { func (r *remoteClient) Put(state []byte) error { ctx := context.Background() - // Retrieve the workspace for which to create a new state. - w, err := r.client.Workspaces.Read(ctx, r.organization, r.workspace) - if err != nil { - return fmt.Errorf("Error retrieving workspace: %v", err) - } - // Read the raw state into a Terraform state. stateFile, err := statefile.Read(bytes.NewReader(state)) if err != nil { @@ -93,7 +77,7 @@ func (r *remoteClient) Put(state []byte) error { } // Create the new state. - _, err = r.client.StateVersions.Create(ctx, w.ID, options) + _, err = r.client.StateVersions.Create(ctx, r.workspace.ID, options) if err != nil { return fmt.Errorf("Error creating remote state: %v", err) } @@ -103,9 +87,9 @@ func (r *remoteClient) Put(state []byte) error { // Delete the remote state. func (r *remoteClient) Delete() error { - err := r.client.Workspaces.Delete(context.Background(), r.organization, r.workspace) + err := r.client.Workspaces.Delete(context.Background(), r.organization, r.workspace.Name) if err != nil && err != tfe.ErrResourceNotFound { - return fmt.Errorf("Error deleting workspace %s: %v", r.workspace, err) + return fmt.Errorf("Error deleting workspace %s: %v", r.workspace.Name, err) } return nil @@ -117,22 +101,8 @@ func (r *remoteClient) Lock(info *state.LockInfo) (string, error) { lockErr := &state.LockError{Info: r.lockInfo} - // Retrieve the workspace to lock. - w, err := r.client.Workspaces.Read(ctx, r.organization, r.workspace) - if err != nil { - lockErr.Err = err - return "", lockErr - } - - // Check if the workspace is already locked. - if w.Locked { - lockErr.Err = fmt.Errorf( - "remote state already\nlocked (lock ID: \"%s/%s\")", r.organization, r.workspace) - return "", lockErr - } - // Lock the workspace. - w, err = r.client.Workspaces.Lock(ctx, w.ID, tfe.WorkspaceLockOptions{ + _, err := r.client.Workspaces.Lock(ctx, r.workspace.ID, tfe.WorkspaceLockOptions{ Reason: tfe.String("Locked by Terraform"), }) if err != nil { @@ -151,27 +121,32 @@ func (r *remoteClient) Unlock(id string) error { lockErr := &state.LockError{Info: r.lockInfo} - // Verify the expected lock ID. - if r.lockInfo != nil && r.lockInfo.ID != id { - lockErr.Err = fmt.Errorf("lock ID does not match existing lock") - return lockErr + // With lock info this should be treated as a normal unlock. + if r.lockInfo != nil { + // Verify the expected lock ID. + if r.lockInfo.ID != id { + lockErr.Err = fmt.Errorf("lock ID does not match existing lock") + return lockErr + } + + // Unlock the workspace. + _, err := r.client.Workspaces.Unlock(ctx, r.workspace.ID) + if err != nil { + lockErr.Err = err + return lockErr + } + + return nil } // Verify the optional force-unlock lock ID. - if r.lockInfo == nil && r.organization+"/"+r.workspace != id { + if r.organization+"/"+r.workspace.Name != id { lockErr.Err = fmt.Errorf("lock ID does not match existing lock") return lockErr } - // Retrieve the workspace to lock. - w, err := r.client.Workspaces.Read(ctx, r.organization, r.workspace) - if err != nil { - lockErr.Err = err - return lockErr - } - - // Unlock the workspace. - w, err = r.client.Workspaces.Unlock(ctx, w.ID) + // Force unlock the workspace. + _, err := r.client.Workspaces.ForceUnlock(ctx, r.workspace.ID) if err != nil { lockErr.Err = err return lockErr