Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Upgrade rabbithole dependency to v2 (using semantic module versioning). #54

Merged
merged 4 commits into from
May 1, 2020

Conversation

niclic
Copy link
Contributor

@niclic niclic commented Apr 30, 2020

What this PR does:

  • Upgrades the rabbithole dependency to use v2 of that module.

Why do we need this PR?

  • Take advantage of semantic go module versioning.
  • Pull in some fixes and enhancements since v2.0.
  • I am working on adding a rabbitmq_federation_upstream resource to this provider. However, that requires changes to rabbithole. The PR is here. If that gets merged, then v2 is required to use the new functionality.

Here are the commands/steps I took.

# run tests
RABBITMQ_ENDPOINT='http://localhost:15672' \
RABBITMQ_USERNAME=guest \
RABBITMQ_PASSWORD=guest \
TF_LOG=DEBUG \
make testacc

# clean up go.mod and go.sum
go mod tidy

# download the latest version of rabbithole
go get github.com/michaelklishin/rabbit-hole/v2


# replace references to "github.com/michaelklishin/rabbit-hole" with "github.com/michaelklishin/rabbit-hole/v2"


# clean up some formatting issues
make fmt

# copy rabbithole/v2 to vendor directory
go mod vendor

# remove rabbithole v1
go mod tidy

# build the provider
make

# run vet
make vet

# run tests again, without caching
RABBITMQ_ENDPOINT='http://localhost:15672' \
RABBITMQ_USERNAME=guest \
RABBITMQ_PASSWORD=guest \
TF_LOG=DEBUG \
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -count=1 -timeout 120m

# all tests pass

I broke this into separate commits so you could see the changes for each command that was run. Happy to rebase this.

Copy link
Contributor

@cyrilgdn cyrilgdn left a comment

Choose a reason for hiding this comment

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

@niclic Thanks for opening this PR

By curiosity, federation was not working with the current version?
Because we were normally already on v2 but just without the semantic version because of this (fixed) issue michaelklishin/rabbit-hole#146 .

Anyway, it's cool to update and to use the semantic versioning 👍

@niclic
Copy link
Contributor Author

niclic commented May 1, 2020

@cyrilgdn

Yes, I noticed that too.

I got this error when trying to use the following remote path format in go.mod

# expected both sides to be v2
replace github.com/michaelklishin/rabbit-hole => github.com/niclic/rabbit-hole v2.1.1-0.20200426194252-fccfa4cf97f4 # latest commits to my branch

go build
go: errors parsing go.mod:
.../go.mod:8: replace github.com/niclic/rabbit-hole: version "v2.1.1-0.20200426194252-fccfa4cf97f4" invalid: should be v0 or v1, not v2

When I upgraded my local provider to use v2 everywhere (like in this PR), it worked. So I assumed the issue is related to the sematic versioning updates to rabbithole, because this version of go.mod produced expected federation missing errors:

# notice is says v1.5.0, and not v2.0.0
replace github.com/michaelklishin/rabbit-hole => github.com/niclic/rabbit-hole v1.5.0

go build
# github.com/terraform-providers/terraform-provider-rabbitmq/rabbitmq
rabbitmq/resource_federation_upstream.go:134:23: rmqc.GetFederationUpstream undefined (type *rabbithole.Client has no field or method GetFederationUpstream)

The fact that a local replace path in go.mod worked without any issues must be due to the way local paths work, so I had to use that.

# no module version
replace github.com/michaelklishin/rabbit-hole => github.com/niclic/rabbit-hole

go build
go: errors parsing go.mod:
/Users/rei/src/github.com/niclic/terraform-provider-rabbitmq/go.mod:8: replacement module without version must be directory path (rooted or starting with ./ or ../)

@cyrilgdn
Copy link
Contributor

cyrilgdn commented May 1, 2020

@niclic I see! Thanks!

@cyrilgdn cyrilgdn merged commit 2dd8ea8 into hashicorp:master May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants