From 35abf5c12c57f032b572454395ae1ab8ba6a8fb0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 10:37:14 +0000 Subject: [PATCH] backport of commit adf6eae080455b9e243585ced10ea475b4fa710a (#36192) Co-authored-by: Brandon Croft --- internal/backend/remote/backend_state.go | 31 +++++- internal/backend/remote/backend_state_test.go | 43 ++++++++ internal/backend/remote/retry.go | 101 ++++++++++++++++++ 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 internal/backend/remote/retry.go diff --git a/internal/backend/remote/backend_state.go b/internal/backend/remote/backend_state.go index b397c6f07cdd..0520f2862ed4 100644 --- a/internal/backend/remote/backend_state.go +++ b/internal/backend/remote/backend_state.go @@ -31,6 +31,22 @@ type remoteClient struct { forcePush bool } +// errorUnlockFailed is used within a retry loop to identify a non-retryable +// workspace unlock error +type errorUnlockFailed struct { + innerError error +} + +func (e errorUnlockFailed) FatalError() error { + return e.innerError +} + +func (e errorUnlockFailed) Error() string { + return e.innerError.Error() +} + +var _ Fatal = errorUnlockFailed{} + // Get the remote state. func (r *remoteClient) Get() (*remote.Payload, error) { ctx := context.Background() @@ -202,7 +218,20 @@ func (r *remoteClient) Unlock(id string) error { } // Unlock the workspace. - _, err := r.client.Workspaces.Unlock(ctx, r.workspace.ID) + // Unlock the workspace. + err := RetryBackoff(ctx, func() error { + _, err := r.client.Workspaces.Unlock(ctx, r.workspace.ID) + if err != nil { + if errors.Is(err, tfe.ErrWorkspaceLockedStateVersionStillPending) { + // This is a retryable error. + return err + } + // This will not be retried + return &errorUnlockFailed{innerError: err} + } + return nil + }) + if err != nil { lockErr.Err = err return lockErr diff --git a/internal/backend/remote/backend_state_test.go b/internal/backend/remote/backend_state_test.go index 8639894efcc1..578fe6b497cc 100644 --- a/internal/backend/remote/backend_state_test.go +++ b/internal/backend/remote/backend_state_test.go @@ -6,6 +6,7 @@ package remote import ( "bytes" "os" + "strings" "testing" "github.com/hashicorp/terraform/internal/backend" @@ -13,6 +14,7 @@ import ( "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/remote" "github.com/hashicorp/terraform/internal/states/statefile" + "github.com/hashicorp/terraform/internal/states/statemgr" ) func TestRemoteClient_impl(t *testing.T) { @@ -41,6 +43,47 @@ func TestRemoteClient_stateLock(t *testing.T) { remote.TestRemoteLocks(t, s1.(*remote.State).Client, s2.(*remote.State).Client) } +func TestRemoteClient_Unlock_invalidID(t *testing.T) { + b, bCleanup := testBackendDefault(t) + defer bCleanup() + + s1, err := b.StateMgr(backend.DefaultStateName) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + err = s1.Unlock("no") + if err == nil { + t.Fatal("expected error, got nil") + } + + if !strings.Contains(err.Error(), "does not match existing lock ID") { + t.Fatalf("expected erroor containing \"does not match existing lock ID\", got %v", err) + } +} + +func TestRemoteClient_Unlock(t *testing.T) { + b, bCleanup := testBackendDefault(t) + defer bCleanup() + + s1, err := b.StateMgr(backend.DefaultStateName) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + id, err := s1.Lock(&statemgr.LockInfo{ + ID: "test", + }) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + err = s1.Unlock(id) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } +} + func TestRemoteClient_Put_withRunID(t *testing.T) { // Set the TFE_RUN_ID environment variable before creating the client! if err := os.Setenv("TFE_RUN_ID", cloud.GenerateID("run-")); err != nil { diff --git a/internal/backend/remote/retry.go b/internal/backend/remote/retry.go new file mode 100644 index 000000000000..db70f1bd622f --- /dev/null +++ b/internal/backend/remote/retry.go @@ -0,0 +1,101 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package remote + +import ( + "context" + "log" + "sync/atomic" + "time" +) + +// Fatal implements a RetryBackoff func return value that, if encountered, +// signals that the func should not be retried. In that case, the error +// returned by the interface method will be returned by RetryBackoff +type Fatal interface { + FatalError() error +} + +// NonRetryableError is a simple implementation of Fatal that wraps an error +type NonRetryableError struct { + InnerError error +} + +// FatalError returns the inner error, but also implements Fatal, which +// signals to RetryBackoff that a non-retryable error occurred. +func (e NonRetryableError) FatalError() error { + return e.InnerError +} + +// Error returns the inner error string +func (e NonRetryableError) Error() string { + return e.InnerError.Error() +} + +var ( + initialBackoffDelay = time.Second + maxBackoffDelay = 3 * time.Second +) + +// RetryBackoff retries function f until nil or a FatalError is returned. +// RetryBackoff only returns an error if the context is in error or if a +// FatalError was encountered. +func RetryBackoff(ctx context.Context, f func() error) error { + // doneCh signals that the routine is done and sends the last error + var doneCh = make(chan struct{}) + var errVal atomic.Value + type errWrap struct { + E error + } + + go func() { + // the retry delay between each attempt + var delay time.Duration = 0 + defer close(doneCh) + + for { + select { + case <-ctx.Done(): + return + case <-time.After(delay): + } + + err := f() + switch e := err.(type) { + case nil: + return + case Fatal: + errVal.Store(errWrap{e.FatalError()}) + return + } + + delay *= 2 + if delay == 0 { + delay = initialBackoffDelay + } + + delay = min(delay, maxBackoffDelay) + + log.Printf("[WARN] retryable error: %q, delaying for %s", err, delay) + } + }() + + // Wait until done or deadline + select { + case <-doneCh: + case <-ctx.Done(): + } + + err, hadErr := errVal.Load().(errWrap) + var lastErr error + if hadErr { + lastErr = err.E + } + + if ctx.Err() != nil { + return ctx.Err() + } + + return lastErr +}