jprdonnelly/opentf
1
0
Fork 0
mirror of https://github.com/opentffoundation/opentf.git synced 2025-12-25 01:00:16 -05:00
Code Issues Packages Projects Releases Wiki Activity

statemgr+remote: context.Context parameters

Browse Source 
This extends statemgr.Persistent, statemgr.Locker and remote.Client to
all expect context.Context parameters, and then updates all of the existing
implementations of those interfaces to support them.

All of the calls to statemgr.Persistent and statemgr.Locker methods outside
of tests are consistently context.TODO() for now, because the caller
landscape of these interfaces has some complications:

1. statemgr.Locker is also used by the clistate package for its state
   implementation that was derived from statemgr.Filesystem's predecessor,
   even though what clistate manages is not actually "state" in the sense
   of package statemgr. The callers of that are not yet ready to provide
   real contexts.

   In a future commit we'll either need to plumb context through to all of
   the clistate callers, or continue the effort to separate statemgr from
   clistate by introducing a clistate-specific "locker" API for it
   to use instead.

2. We call statemgr.Persistent and statemgr.Locker methods in situations
   where the active context might have already been cancelled, and so we'll
   need to make sure to ignore cancellation when calling those.

   This is mainly limited to PersistState and Unlock, since both need to
   be able to complete after a cancellation, but there are various
   codepaths that perform a Lock, Refresh, Persist, Unlock sequence and so
   it isn't yet clear where is the best place to enforce the invariant that
   Persist and Unlock must not be called with a cancelable context. We'll
   deal with that more in subsequent commits.

Within the various state manager and remote client implementations the
contexts _are_ wired together as best as possible with how these subsystems
are already laid out, and so once we deal with the problems above and make
callers provide suitable contexts they should be able to reach all of the
leaf API clients that might want to generate OpenTelemetry traces.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This commit is contained in:
Martin Atkins
2025-07-07 16:57:57 -07:00
parent ddf126b267
commit 67a5cd0911
103 changed files with 673 additions and 675 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
internal/backend/remote/backend.go
View File

@@ -557,7 +557,7 @@ func (b *Remote) DeleteWorkspace(_ context.Context, name string, _ bool) error {
encryption: b.encryption,
}
return client.Delete()
return client.Delete(context.TODO())
}
// StateMgr implements backend.Enhanced.

8
internal/backend/remote/backend_apply_test.go
View File

@@ -112,7 +112,7 @@ func TestRemote_applyBasic(t *testing.T) {
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
// An error suggests that the state was not unlocked after apply
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after apply: %s", err.Error())
}
}
@@ -140,7 +140,7 @@ func TestRemote_applyCanceled(t *testing.T) {
}
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after cancelling apply: %s", err.Error())
}
}
@@ -488,7 +488,7 @@ func TestRemote_applyWithExclude(t *testing.T) {
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
// An error suggests that the state was not unlocked after apply
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after failed apply: %s", err.Error())
}
}
@@ -654,7 +654,7 @@ func TestRemote_applyNoConfig(t *testing.T) {
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
// An error suggests that the state was not unlocked after apply
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after failed apply: %s", err.Error())
}
}

2
internal/backend/remote/backend_context.go
View File

@@ -64,7 +64,7 @@ func (b *Remote) LocalRun(ctx context.Context, op *backend.Operation) (*backend.
}()
log.Printf("[TRACE] backend/remote: reading remote state for workspace %q", remoteWorkspaceName)
if err := stateMgr.RefreshState(); err != nil {
if err := stateMgr.RefreshState(context.TODO()); err != nil {
diags = diags.Append(fmt.Errorf("error loading state: %w", err))
return nil, nil, diags
}

6
internal/backend/remote/backend_context_test.go
View File

@@ -228,7 +228,7 @@ func TestRemoteContextWithVars(t *testing.T) {
// When Context() returns an error, it should unlock the state,
// so re-locking it is expected to succeed.
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state: %s", err.Error())
}
} else {
@@ -237,7 +237,7 @@ func TestRemoteContextWithVars(t *testing.T) {
}
// When Context() succeeds, this should fail w/ "workspace already locked"
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err == nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err == nil {
t.Fatal("unexpected success locking state after Context")
}
}
@@ -445,7 +445,7 @@ func TestRemoteVariablesDoNotOverride(t *testing.T) {
}
// When Context() succeeds, this should fail w/ "workspace already locked"
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err == nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err == nil {
t.Fatal("unexpected success locking state after Context")
}

4
internal/backend/remote/backend_plan_test.go
View File

@@ -97,7 +97,7 @@ func TestRemote_planBasic(t *testing.T) {
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
// An error suggests that the state was not unlocked after the operation finished
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after successful plan: %s", err.Error())
}
}
@@ -126,7 +126,7 @@ func TestRemote_planCanceled(t *testing.T) {
stateMgr, _ := b.StateMgr(t.Context(), backend.DefaultStateName)
// An error suggests that the state was not unlocked after the operation finished
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
if _, err := stateMgr.Lock(t.Context(), statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state after cancelled plan: %s", err.Error())
}
}

20
internal/backend/remote/backend_state.go
View File

@@ -36,9 +36,7 @@ type remoteClient struct {
}
// Get the remote state.
func (r *remoteClient) Get() (*remote.Payload, error) {
ctx := context.Background()
func (r *remoteClient) Get(ctx context.Context) (*remote.Payload, error) {
sv, err := r.client.StateVersions.ReadCurrent(ctx, r.workspace.ID)
if err != nil {
if err == tfe.ErrResourceNotFound {
@@ -93,9 +91,7 @@ func (r *remoteClient) uploadStateFallback(ctx context.Context, stateFile *state
}
// Put the remote state.
func (r *remoteClient) Put(state []byte) error {
ctx := context.Background()
func (r *remoteClient) Put(ctx context.Context, state []byte) error {
// Read the raw state into a OpenTofu state.
stateFile, err := statefile.Read(bytes.NewReader(state), r.encryption)
if err != nil {
@@ -145,8 +141,8 @@ 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.Name)
func (r *remoteClient) Delete(ctx context.Context) error {
err := r.client.Workspaces.Delete(ctx, r.organization, r.workspace.Name)
if err != nil && err != tfe.ErrResourceNotFound {
return fmt.Errorf("error deleting workspace %s: %w", r.workspace.Name, err)
}
@@ -161,9 +157,7 @@ func (r *remoteClient) EnableForcePush() {
}
// Lock the remote state.
func (r *remoteClient) Lock(info *statemgr.LockInfo) (string, error) {
ctx := context.Background()
func (r *remoteClient) Lock(ctx context.Context, info *statemgr.LockInfo) (string, error) {
lockErr := &statemgr.LockError{Info: r.lockInfo}
// Lock the workspace.
@@ -185,9 +179,7 @@ func (r *remoteClient) Lock(info *statemgr.LockInfo) (string, error) {
}
// Unlock the remote state.
func (r *remoteClient) Unlock(id string) error {
ctx := context.Background()
func (r *remoteClient) Unlock(ctx context.Context, id string) error {
// We first check if there was an error while uploading the latest
// state. If so, we will not unlock the workspace to prevent any
// changes from being applied until the correct state is uploaded.

2
internal/backend/remote/backend_state_test.go
View File

@@ -60,7 +60,7 @@ func TestRemoteClient_Put_withRunID(t *testing.T) {
// Store the new state to verify (this will be done
// by the mock that is used) that the run ID is set.
if err := client.Put(buf.Bytes()); err != nil {
if err := client.Put(t.Context(), buf.Bytes()); err != nil {
t.Fatalf("expected no error, got %v", err)
}
}

4
internal/backend/remote/backend_test.go
View File

@@ -382,13 +382,13 @@ func TestRemote_Unlock_ignoreVersion(t *testing.T) {
t.Fatalf("error: %v", err)
}
lockID, err := state.Lock(statemgr.NewLockInfo())
lockID, err := state.Lock(t.Context(), statemgr.NewLockInfo())
if err != nil {
t.Fatalf("error: %v", err)
}
// this should succeed since the version conflict is ignored
if err = state.Unlock(lockID); err != nil {
if err = state.Unlock(t.Context(), lockID); err != nil {
t.Fatalf("error: %v", err)
}
}