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

Fix acceptance tests for powerdns 4.1. #7

Merged
merged 2 commits into from
Jun 25, 2019
Merged

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Mar 16, 2018

  • Add trailing dots to record names to fix non-canonical warnings
  • Lowercase record names to fix spurious recreates

We could also change the code to automatically add trailing dots, lowercase records, and suppress diffs, but IMO it's better to pass values to powerdns with minimal modification--users should understand how powerdns works without having to learn a slightly different interface exposed by the provider.

Output of acceptance tests running against 4.1 using postgres backend:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m
?   	github.com/terraform-providers/terraform-provider-powerdns	[no test files]
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProviderImpl
--- PASS: TestProviderImpl (0.00s)
=== RUN   TestAccPDNSRecord_A
--- PASS: TestAccPDNSRecord_A (0.24s)
=== RUN   TestAccPDNSRecord_WithCount
--- PASS: TestAccPDNSRecord_WithCount (0.31s)
=== RUN   TestAccPDNSRecord_AAAA
--- PASS: TestAccPDNSRecord_AAAA (0.28s)
=== RUN   TestAccPDNSRecord_CNAME
--- PASS: TestAccPDNSRecord_CNAME (0.22s)
=== RUN   TestAccPDNSRecord_HINFO
--- PASS: TestAccPDNSRecord_HINFO (0.22s)
=== RUN   TestAccPDNSRecord_LOC
--- PASS: TestAccPDNSRecord_LOC (0.22s)
=== RUN   TestAccPDNSRecord_MX
--- PASS: TestAccPDNSRecord_MX (0.42s)
=== RUN   TestAccPDNSRecord_NAPTR
--- PASS: TestAccPDNSRecord_NAPTR (0.21s)
=== RUN   TestAccPDNSRecord_NS
--- PASS: TestAccPDNSRecord_NS (0.22s)
=== RUN   TestAccPDNSRecord_SPF
--- PASS: TestAccPDNSRecord_SPF (0.27s)
=== RUN   TestAccPDNSRecord_SSHFP
--- PASS: TestAccPDNSRecord_SSHFP (0.29s)
=== RUN   TestAccPDNSRecord_SRV
--- PASS: TestAccPDNSRecord_SRV (0.24s)
=== RUN   TestAccPDNSRecord_TXT
--- PASS: TestAccPDNSRecord_TXT (0.22s)
PASS
ok  	github.com/terraform-providers/terraform-provider-powerdns/powerdns	3.374s

Resolves #6, cc @anuriq @grubernaut

jmcarp added 2 commits March 16, 2018 01:42
* Add trailing dots to record names to fix non-canonical warnings
* Lowercase record names to fix spurious recreates
This was referenced Mar 16, 2018
@jmcarp
Copy link
Contributor Author

jmcarp commented Mar 22, 2018

cc @radeksimko

@Orionde
Copy link

Orionde commented Apr 17, 2018

Hello, any news for this pull request ? ping @jmcarp @grubernaut
I really need the set-ptr option :)

@jmcarp
Copy link
Contributor Author

jmcarp commented May 3, 2018

Hey @katbyte, are you/hashicorp maintaining this provider? There are a few outstanding pull requests that could use some review. Or, if you'd be willing to grant push access, my team could help maintain the provider. Or we can continue using our fork 😄

@dannyk81
Copy link
Contributor

Just wanted to say thanks for this (and other) PRs! these are things we've been waiting for for a while now.

Just wish someone would review, merge and create a new release 😄

Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

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

LGTM

@grubernaut
Copy link
Contributor

I'm no longer a maintainer of Terraform providers or Terraform core, but hopefully that helps get the ball rolling on this...

@dannyk81
Copy link
Contributor

Thanks @grubernaut!

Any idea who's maintaining this provider nowadays?

@grubernaut
Copy link
Contributor

@dannyk81 no idea, honestly. I'd wager that pinging @phinze, @tombuildsstuff, and @catsby should get someone to look at it though 😄

@anuriq
Copy link
Contributor

anuriq commented May 28, 2018

Wow, looks like separation of terraform providers caused a state where they feel like abandoned...

@grubernaut
Copy link
Contributor

grubernaut commented May 28, 2018

@anuriq well, that sure as shit wasn't the intention. Far from it, actually. We believed that splitting the providers would allow for community contriubtors to become maintainers of the community providers, and this is still possible, as far as I know. You're more than welcome to volunteer for this and I'm sure they would be glad to accept all the help that you're willing to give....

@dannyk81
Copy link
Contributor

@grubernaut looks like @jmcarp was willing to volunteer to help maintain this provider 😀

https://github.com/terraform-providers/terraform-provider-powerdns/pull/7#issuecomment-386158665

@grubernaut
Copy link
Contributor

😞 yea, I saw that as well. Unfortunately, as stated above, there's nothing I can do about it, as I left Hashicorp ~9 months ago. Best thing to do is ping them to give @jmcarp merge permissions in order to fully take over maintainership of the provider.

@jmcarp
Copy link
Contributor Author

jmcarp commented May 30, 2018

I haven't been able to find any current hashicorp folks--I've been tagging people here and on gitter. Is there a better way to track people down?

cc @phinze @tombuildsstuff @catsby

@catsby
Copy link

catsby commented May 30, 2018

Thanks @grubernaut , sorry for the silence here.

Hey @jmcarp sorry to you as well for the silence. This Provider could certainly use a maintainer, if you're offering!

About the PR:

I pulled this PR locally and ran the tests, but didn't get passing results, am I missing something? I'm mimicing how the tests are setup in our CI and that involes a docker image running PowerDNS:

The errors I'm getting all follow this pattern:

--- FAIL: TestAccPDNSRecord_TXT (0.01s)
        testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:

                * powerdns_record.test-txt: 1 error(s) occurred:

                * powerdns_record.test-txt: Failed to create PowerDNS Record: Error creating record set: text.sysa.xyz.:::TXT, reason: "RRset text.sysa.xyz. IN TXT: Name is out of zone"
FAIL

If there's another/better way to run the tests, please let me know!


Regarding maintaining the Provider:

We value our community contributors and their time and generally tend to avoid setting any particular obligations,
however before you do participate in the community as a maintainer we kindly ask you to read the etiquette
https://github.com/hashicorp/terraform/blob/master/docs/maintainer-etiquette.md
It briefly describes the process each collaborator follows when reviewing & merging PRs.

Additionally we expect our community to follow the guidelines listed on https://www.hashicorp.com/community-guidelines/ and ask maintainers to
lead as examples in that context. Please take time to review those guidelines,
unless you have done so already.

Lastly, make sure you have 2FA enabled for your GitHub account.

If you agree to these terms then we'd be happy to add yourself and optionally others as maintainers of this provider, and we would be very grateful for your help!

Let me know if you're interested.

@grubernaut
Copy link
Contributor

Thanks again @catsby ❤️

@dannyk81
Copy link
Contributor

ping @jmcarp 😄

@dannyk81
Copy link
Contributor

@catsby looks like @jmcarp is not responding, I wonder if someone else (someone from Hashicorp perhaps?) can take over here?

We use this provider quite a bit and is a shame to see it go stale like this 😢

@randomcamel
Copy link
Contributor

Is this still needed? AFAICT, except for 1 flaky test, the acceptance tests have been passing for a couple weeks at least.

@mbag mbag merged commit fc5e729 into pan-net:master Jun 25, 2019
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.

DNS Name is not canonical error message
8 participants