Skip to content
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

Upgrade pulumi/pulumi #945

Merged
merged 4 commits into from
Feb 6, 2024
Merged

Upgrade pulumi/pulumi #945

merged 4 commits into from
Feb 6, 2024

Conversation

blampe
Copy link
Contributor

@blampe blampe commented Jan 17, 2024

Bumps pu/pu across the entire repo, including a few examples that were still using very old versions.

@blampe blampe requested a review from a team January 17, 2024 18:19
Copy link

Does the PR have any schema changes?

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this probably works, it is not recommended to have the Pulumi version overtake the pulumi version used in the bridge.

I recommend keeping the pu/pu version currently in go.mod and updating the examples files to that.

Also this is somewhere between a missing feature in upgrade-provider and the setup of the Go tests in this repo:

  • upgrade-provider does not detect all go.mod files in the examples
  • this is because most providers expect a single go.mod file in `examples/, not one per example.

@blampe
Copy link
Contributor Author

blampe commented Jan 18, 2024

While this probably works, it is not recommended to have the Pulumi version overtake the pulumi version used in the bridge.

I recommend keeping the pu/pu version currently in go.mod and updating the examples files to that.

Good catch, updated.

Also this is somewhere between a missing feature in upgrade-provider and the setup of the Go tests in this repo:

  • upgrade-provider does not detect all go.mod files in the examples
  • this is because most providers expect a single go.mod file in `examples/, not one per example.

Yes, we want behavior analogous to pulumi/ci-mgmt#778 for bridged providers.

for MODFILE in $(find . -name go.mod); do pushd $(dirname $MODFILE); go get github.com/pulumi/pulumi-terraform-bridge/v3 ; go mod tidy; popd; done

I think this is essentially pulumi/upgrade-provider#142

@@ -5,7 +5,7 @@ go 1.21
require (
github.com/pulumi/pulumi-aws/sdk/v5 v5.30.0
github.com/pulumi/pulumi-docker/sdk/v4 v4.0.0-alpha.4
github.com/pulumi/pulumi/sdk/v3 v3.101.1
github.com/pulumi/pulumi/sdk/v3 v3.99.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you were looking at the changes between revisions, this is upgrading from 3.55 to 3.99.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM then thank you!

@t0yv0
Copy link
Member

t0yv0 commented Jan 18, 2024

Yeah I'm in the process of finishing up the bridge test on 101 pulumi/pulumi-terraform-bridge#1624 - as far as I can tell the p/p codegen version passes all tests, so if you want to make an exception and move forward that's fine with me, but normally we get the p/p dependency bumped automatically through bridge updates which should be coming shortly once I'm done validating the above and clearing any blockers.

@blampe
Copy link
Contributor Author

blampe commented Jan 18, 2024

normally we get the p/p dependency bumped automatically through bridge updates

@t0yv0 the problem is the updates aren't comprehensive, see my other comment here #945 (comment)

@t0yv0
Copy link
Member

t0yv0 commented Jan 18, 2024

Right. I thought you correctly identified the root cause on that we need to hack update-provider a little further.

@blampe blampe enabled auto-merge (squash) February 6, 2024 00:03
@blampe blampe merged commit 92b00a0 into master Feb 6, 2024
17 checks passed
@blampe blampe deleted the bump-pulumi branch February 6, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants