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

provider/aws: Add Support For AWS VPN Gateways using aws-sdk-go #1137

Merged
merged 16 commits into from
Mar 19, 2015

Conversation

deverton
Copy link
Contributor

@deverton deverton commented Mar 5, 2015

Add's support for managing VPN gateways in AWS. The way it works (and most of the code) is based on the way that internet gateways are currently handled. It's written using the aws-sdk-go library though.

Dan Everton added 4 commits March 6, 2015 08:41
Uses the aws-sdk-go module and is based on the way the existing
aws_internet_gateway resource works.
@deverton
Copy link
Contributor Author

deverton commented Mar 6, 2015

One thing is I can't get the TestAccVpnGateway_tags acceptance test to pass and I'm not sure why it's not working. I get this error:

--- FAIL: TestAccVpnGateway_tags (118.38s)
    testing.go:121: Step 1 error: Error applying: 1 error(s) occurred:

        * 1 error(s) occurred:

        * 1 error(s) occurred:

        * The parameter Resources is not recognized

@catsby
Copy link
Contributor

catsby commented Mar 6, 2015

Hey @deverton thanks for the contribution!
I'll give this a better look-over today, initial glance looked good.

Regarding the acceptance test failure, I patched that issue in hashicorp/aws-sdk-go yesterday, can you run make updatedeps and try again? See hashicorp/aws-sdk-go#4 for the fix.

@deverton
Copy link
Contributor Author

deverton commented Mar 6, 2015

@catsby That patch has fixed the acceptance tests and everything now passes. Thanks!

@catsby
Copy link
Contributor

catsby commented Mar 9, 2015

Thanks! Taking a look now. I did notice that it needs go fmt ran on it, could you do that please?

VPCID: aws.String(d.Get("vpc_id").(string)),
})
if err != nil {
ec2err, ok := err.(*aws.APIError)
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm not mistaken, the return here should be aws.APIError. Does the * work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure as I'm only just starting to learn Go. I'll take a closer look.

@catsby
Copy link
Contributor

catsby commented Mar 9, 2015

Looks promising, thanks! Pending some nitpicks and questions though, can you take a look @deverton ?

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Mar 9, 2015
Dan Everton added 5 commits March 10, 2015 09:01
Current AWS documentation says there's only one type of VPN gateway for
now.
AWS returns IncorrectState not DependencyViolation when a VPN gateway is
still attached to a VPC.
@deverton
Copy link
Contributor Author

Okay, I think all of the feedback has been applied.

@catsby catsby removed the waiting-response An issue/pull request is waiting for a response from the community label Mar 10, 2015
@catsby
Copy link
Contributor

catsby commented Mar 11, 2015

@deverton excellent, thanks!

One final, super small nitpick that I missed: the VpnGatewayStateRefreshFunc is only used here, right? Let's make it private: vpnGatewayStateRefreshFunc (just lowercase it).

Let me know if you can get to that, otherwise I can merge and convert it.

Thanks again!

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Mar 11, 2015
@deverton
Copy link
Contributor Author

@catsby no problem, I've made that change.

@catsby catsby removed the waiting-response An issue/pull request is waiting for a response from the community label Mar 16, 2015
@deverton
Copy link
Contributor Author

@catsby I've updated the code to compile again after the goamz removal.

func resourceAwsVpnGatewayRead(d *schema.ResourceData, meta interface{}) error {
ec2conn := meta.(*AWSClient).ec2conn

vpnGatewayRaw, _, err := vpnGatewayStateRefreshFunc(ec2conn, d.Id())()
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you call a refresh func vpnGatewayStateRefreshFunc, but we only call it once and never try again, like in a wait.WaitForState in the resourceAwsVpnGatewayAttach function.

I see that's basically copied from Internet Gateway, so not a fault of yours 😄 . In your experience does this typically take time, and may fail on the first run?

@catsby
Copy link
Contributor

catsby commented Mar 19, 2015

I had another question, but I don't think it's a blocker.

catsby added a commit that referenced this pull request Mar 19, 2015
provider/aws: Add Support For AWS VPN Gateways using aws-sdk-go
@catsby catsby merged commit bb4dd8a into hashicorp:master Mar 19, 2015
@catsby
Copy link
Contributor

catsby commented Mar 19, 2015

Going ahead an pulling this in, thanks @deverton !

@radeksimko radeksimko mentioned this pull request Mar 25, 2015
7 tasks
@ghost
Copy link

ghost commented May 4, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 4, 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