-
Notifications
You must be signed in to change notification settings - Fork 2k
AWS v7 Provider upgrade #6259
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.
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
base: dev
Are you sure you want to change the base?
AWS v7 Provider upgrade #6259
Conversation
7ecfba8 to
0cd4502
Compare
7cdee66 to
3f57e52
Compare
|
Draft message for release notes:
|
|
One thing to consider, adding support for |
3f57e52 to
79c8374
Compare
|
I hope we can push on this PR as a lot of features are blocked by it 👍 |
79c8374 to
bd42373
Compare
|
@vimtor Now that pulumi has been merged I've updated this PR so it's good to review now. |
9120021 to
9bc91fa
Compare
|
Would love for this to be merged, it's key for my project, and I've switched to using this branch as my base. |
vimtor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @jamesgibbons92
i left a couple of minor comments about the migration steps
everything else seems to be working. great job!
will be testing more examples today to see if they still work
cmd/sst/deploy.go
Outdated
| "", | ||
| "```bash frame=\"none\"", | ||
| "sst deploy --refresh", | ||
| "```", | ||
| "Pass --refresh flag to refresh your state resources before deploying any changes.", | ||
| "This is useful for making sure your state reflects your cloud resources accurately before applying updates.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking about what's the difference between sst deploy --refresh and sst refresh i discovered that the refresh command is broken in this pr:
default_7_12_0 pulumi:providers:aws
pulumi:providers:aws resource 'default_7_12_0' has a problem: Missing region information
Make sure you have set your AWS region, e.g. `pulumi config set aws:region us-west-2`.
regardless of whether we need this new flag, the user should be able to do:
sst refreshsst deploy
to upgrade their state right? @jamesgibbons92
if that's the case we might want to keep it simple and don't introduce new flags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh it's not necessary, I can remove it.
But you said the refresh command is broken? Or was it just when using as a deploy arg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when i tried doing:
- sst refresh
- sst deploy
to migrate the state i got an error
did it work for your?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refresh and deploy did work for me off this branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got this error when trying the refresh then deploy:
i'm also thinking that if someone runs sst refresh and they’re on a new version they might accidentally upgrade their state. seems unlikely though
maybe we should be more explicit - either with a major version that auto-refreshes the state on the first deploy or a specific sst upgrade command
i'm not sure tbh, what do you think @austeane @jamesgibbons92?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vimtor can you DM me your sst.config.ts so I can reproduce the region error - I've not seen it in my testing.
i'm also thinking that if someone runs sst refresh and they’re on a new version they might accidentally upgrade their state. seems unlikely though
I don't know how we could possibly handle this scenario. We would probably to a sst major version update anyway for the breaking changes, so it's up to the users to check the release notes and upgrade on a lower environment.
maybe we should be more explicit - either with a major version that auto-refreshes the state on the first deploy or a specific sst upgrade command
Yeh I think major version, i'm not sold on auto refresh, as users need to check their non-sst aws components and make any relevent updates. And I feel like having refresh and deploy as separate steps gives people time to check the refresh or recover if something goes wrong with state migration - they can fix this by rolling back to state with no changes to their infra. Wheras with an auto-refresh before a deploy it would just continue on with deploy which no chance to verify the changes between.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vimtor Do you have region set in your aws profile? I can reproduce this error if I remove my region config.
Looks like sst defaults to us-east-1 if you have no default region set. but this doesn't seem to play well with the --run-program pulumi arg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vimtor this should be resolved now if you can test again.
The issue was --run-program arg makes it such that the refresh command uses the latest provider you have in your program, rather than the one that is stored in state.
SST does not pass the provider config to pulumi cli via the --config arg when calling the refresh command (and it's not supported in the version of pulumi we're using). So to resolve i've set the AWS_REGION in the environment variables similarly to how sst sets AWS_PROFILE.
pkg/project/preflight.go
Outdated
| providerName: "aws", | ||
| fromVersion: "6.0.0", | ||
| toVersion: "7.0.0", | ||
| message: "Detected AWS provider upgrade to v7.\n\nUpgrading from v6 to v7 introduces some breaking changes and requires state migration before deploying. Refer to the SST release notes and the pulumi migration guide to make the relevent changes before deploying: https://www.pulumi.com/registry/packages/aws/how-to-guides/7-0-migration \n\nThis notice will clear after migrating your state", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| message: "Detected AWS provider upgrade to v7.\n\nUpgrading from v6 to v7 introduces some breaking changes and requires state migration before deploying. Refer to the SST release notes and the pulumi migration guide to make the relevent changes before deploying: https://www.pulumi.com/registry/packages/aws/how-to-guides/7-0-migration \n\nThis notice will clear after migrating your state", | |
| message: "Detected AWS provider upgrade to v7.\n\nUpgrading from v6 to v7 introduces some breaking changes and requires state migration before deploying. Refer to the SST release notes and the Pulumi migration guide to make the relevent changes before deploying: https://www.pulumi.com/registry/packages/aws/how-to-guides/7-0-migration \n\nThis notice will clear after migrating your state", |
from this message it didn't become clear that:
- the user needs to
refreshthe state - the migration is one-way only? (i tried going back and it didn't work)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I don't think you can rollback easily by downgrading the sst package, this would have to be handled on the provider level anyway.
Users would have to rollback the state manually, probably by restoring a previous version in the state s3 bucket (we should really have a cli command for this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
maybe we should be more explicit in the error about which commands have to be run and that rolling back is "impossible"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternatively we could run the commands for the user and show a nice confirmation UI like this:
#5109
but i believe just being more explicit should be enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update the copy to be more specific. I was expecting the more detailed steps to go in the release notes on github but it doesn't hurt to be specific here as well.
a79f7ba to
1457dce
Compare
1457dce to
473cf54
Compare
This will ensure it's available to the pulumi engine during refresh when using --run-program as refresh does not take the --config arg
AWS provider upgade has some breaking changes which requires some manual intervention before the user should deploy using the new version.
The steps are explained here https://www.pulumi.com/registry/packages/aws/how-to-guides/7-0-migration
Upgrade pulumi to latest version (this contains the new arg flag --run-program for refresh). (Thanks to @abnud11)This will be merged separately in another PR.sst difforsst deploy. Which asks the user to explicitly runsst refreshto migrate their state to the new version. Deploy/diff will be blocked until this is done and no resources in your state are using an outdated version.--run-programpulumi arg to refresh command, which is required for the migration.