[DNM/WIP] PSS: Add logic for initialising a new working directory with PSS. #37323
+2,656
−286
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SarahFrench
added
the
no-changelog-needed
SarahFrench
force-pushed
the
pss/init-of-new-pss-workingdir
branch
from
f3221a2 to
70a7741
Compare
SarahFrench
changed the base branch from
main
to
pss/multistage-provider-download-experiment
SarahFrench
force-pushed
the
pss/multistage-provider-download-experiment
branch
2 times, most recently
from
764650f to
4ad9f59
Compare
SarahFrench
force-pushed
the
pss/init-of-new-pss-workingdir
branch
from
0b54aee to
59e7511
Compare
SarahFrench
commented
Aug 12, 2025
| if err := defaultSMgr.WriteState(emptyState); err != nil { | ||
| diags = diags.Append(fmt.Errorf(errStateStoreWorkspaceCreate, c.Type, err)) | ||
| return diags | ||
| } | ||
| // TODO - implement Read/Write state RPC methods | ||
| if err := defaultSMgr.PersistState(nil); err != nil { | ||
| diags = diags.Append(fmt.Errorf(errStateStoreWorkspaceCreate, c.Type, err)) | ||
| return diags | ||
| } |
There was a problem hiding this comment.
Currently this PR is blocked here - we need to implement the Read/Write state RPC methods
SarahFrench
force-pushed
the
pss/multistage-provider-download-experiment
branch
from
626d03f to
31f2cc1
Compare
Base automatically changed from
pss/multistage-provider-download-experiment
to
main
August 18, 2025 10:20
SarahFrench
force-pushed
the
pss/init-of-new-pss-workingdir
branch
from
e5f4dbd to
a08dfd2
Compare
…nloads providers in two stages (#37350) * Add forked version of `run` logic that's only used if experiments are enabled * Reorder actions in experimental init - load in full config before configuring the backend. * Add getProvidersFromConfig method, initially as an exact copy of getProviders * Make getProvidersFromConfig not use state to get providers * Add `appendLockedDependencies` method to `Meta` to allow multi-phase saving to the dep locks file * Update experimental init to use new getProvidersFromConfig method * Add new getProvidersFromState method that only accepts state information as input for getting providers. Use in experimental init and append values to existing deps lock file * Update messages sent to view about provider download phases * Change init to save updates to the deps lock file only once * Make Terraform output report that a lock file _will_ be made after providers are determined from config * Remove use of `ProviderDownloadOutcome`s * Move repeated code into separate method * Change provider download approach: determine if locks changed at point of attempting to update the lockfile, keep record of incomplete providers inside init command struct * Refactor `mergeLockedDependencies` and update test * Add comments to provider download methods * Fix issue where incorrect message ouput to view when downloading providers * Update `mergeLockedDependencies` method to be more generic * Update `getProvidersFromState` method to receive in-progress config locks and merge those with any locks on file. This allows re-use of providers downloaded by `getProvidersFromConfig` in the same init command * Fix config for `TestInit_stateStoreBlockIsExperimental` * Improve testing of mergeLockedDependencies; state locks are always missing version constraints * Add tests for 2 phase provider download * Add test case to cover use of the `-upgrade` flag * Change the message shown when a provider is reused during the second provider download step. When downloading providers described only in the state then the provider may already be downloaded from a previous init (i.e. is recorded in the deps lock file) or downloaded during step 1 of provider download. The message here needs to cover both potential scenarios. * Update mergeLockedDependencies comment * fix: completely remove use of upgrade flag in getProvidersFromState * Fix: avoid nil pointer errors by returning an empty collection of locks when there is no state * Fix: use state store data only in diagnostic * Change how we make PSS experimental - avoid relying on a package level variable that causes tests to interact. * Remove full-stop in view message, update tests * Update span names to be unique * Re-add lost early returns * Remove unused view messages * Add comments to new view messages
…arsed config. This can only be done once modules have been parsed and the required providers data is available. There are multiple places where config is parsed, into either Config or Module structs, so this needs to be implemented in multiple places.
…r implied local) project
…ath for adding a state store for the first time
…igrating to (implied) local backend
…as passed some through
SarahFrench
force-pushed
the
pss/init-of-new-pss-workingdir
branch
from
d14a636 to
b84fd8e
Compare
Changelog WarningPlease remove either the 'no-changelog-needed' label or the changelog entry from this PR. |
|
Rebased to include the Read/Write state work in #37440 I did a really rough implementation of Get/Put methods in the remote state client to help these tests pass, but I think that work needs to be done properly and merged into main (probably in a separate PR to Radek's above) before being used in this PR. |
…is currently only possible for testing init commands interacting with the PSS command
I think? I assume that a response would be returned full of zero-values when EOF is encountered.
SarahFrench
commented
Sep 2, 2025
|
|
||
| // Verify that selected workspace exists in the state store. | ||
| if opts.Init && b != nil { | ||
| err := m.selectWorkspace(b) |
There was a problem hiding this comment.
Could we update func (m *Meta) selectWorkspace(b backend.Backend) error to respond to zero workspaces with a prompt to create the default workspace, instead of handling it here?
The function currently has a bunch of behaviours:
- If using
cloudbackend, allow users to create an initial workspace. - If using a different backend, the absence of any workspaces leads to an error.
- Allows selecting a workspace from a list of existing ones, if the currently selected workspace doesn't exist in the list of existing ones.
We could standardise on creating an initial workspace here regardless of backend type, but if the backend isn't cloud then TF does it without input and always calls it default?
Otherwise the provider never receives any state data to write!
…e after all state is received.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is stacked on top of #37350
Target Release
N/A
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry