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.

Sign up for GitHub

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

Jump to bottom

Upgrade terraform provider nomad to v2.0.0 #113

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Upgrade terraform provider nomad to v2.0.0 #113

merged 3 commits into from
Sep 13, 2023

Conversation

mjeffryes
Copy link
Member

@mjeffryes mjeffryes commented Sep 12, 2023 •
edited
Loading

Fixes #105

This is my first major version update and first update from sdk v1 to sdk v2 so please review with care!

I followed the steps in:
https://github.com/pulumi/platform-providers-team/blob/main/playbooks/tf-provider-major-version-update.md

and also these additional steps to upgrade the shim from sdk-v1 to sdk-v2:

  • updated the imports of "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v1" to "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
  • used the NewProvider method from the tfshim/sdk-v2 to build the provider
  • updated the replace directives in go.mod with the replaces from latest upstream and from terraform-bridge

@github-actions
Copy link

github-actions bot commented Sep 12, 2023 •
edited
Loading

Does the PR have any schema changes?

Does the PR have any schema changes?

Found 1 breaking change:

Resources

  • 🟢 "nomad:index/aclBindingRule:AclBindingRule": required: "bindName" property is no longer Required

New resources:

  • index/csiVolume.CsiVolume
  • index/csiVolumeRegistration.CsiVolumeRegistration
  • index/nodePool.NodePool
  • index/variable.Variable

New functions:

  • index/getAllocations.getAllocations
  • index/getNodePool.getNodePool
  • index/getNodePools.getNodePools
  • index/getVariable.getVariable

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

@mjeffryes mjeffryes force-pushed the upgrade-terraform-provider-nomad-to-v2.0.0 branch 2 times, most recently from 82fdc23 to 8b1aa55 Compare September 12, 2023 22:30
@mjeffryes mjeffryes requested review from a team and lukehoban September 12, 2023 22:44
lukehoban
lukehoban approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

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

LGTM from a dependency perspective.

Note that there are dependencies on both of these:

  • github.com/hashicorp/nomad v1.6.0
  • github.com/hashicorp/vault v0.10.4

These have both moved to BSL in master, but I reconfirmed that the versions in use here are pre-BSL.

Note that we will need to remove these dependencies before we can update much further than this. We did that successfully for Pulumi-vault which I believe can apply here as well. I don’t recall what will be necessary on the nomad dependency.

iwahbe
iwahbe approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

Looks good to me. LGTM

provider/resources.go
contract.AssertNoErrorf(err, "failed to compute token mappings")
err = x.AutoAliasing(&prov, prov.GetMetadata())
prov.MustComputeTokens(tfbridgetokens.SingleModule("nomad_", mainMod, tfbridgetokens.MakeStandard(mainPkg)))
err := prov.ApplyAutoAliases()
Copy link
Member

Choose a reason for hiding this comment

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

There is a Must* variant of ApplyAutoAliases. If we are just going to a assert no error, we can just use that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the tip. I'm going to punt this to a fixup so I don't have to do the git tag shenanigans for the major version bump again.

@mjeffryes mjeffryes merged commit 2d5686e into master Sep 13, 2023
35 of 41 checks passed
@mjeffryes mjeffryes deleted the upgrade-terraform-provider-nomad-to-v2.0.0 branch September 13, 2023 16:55
@mjeffryes mjeffryes mentioned this pull request Sep 13, 2023
Merged
@guineveresaenger guineveresaenger mentioned this pull request Sep 13, 2023
Closed
mjeffryes added a commit to pulumi/ci-mgmt that referenced this pull request Sep 13, 2023
@mjeffryes
Update for nomad major version bump (#568)
db5eb5b 
Major version was bumped in
pulumi/pulumi-nomad#113. This brings the ci-mgmt
files in line with the new version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iwahbe iwahbe iwahbe approved these changes

@lukehoban lukehoban lukehoban approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade terraform-provider-nomad to v2.0.0
3 participants
@mjeffryes @lukehoban @iwahbe