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/digitalocean : Floating IPs #3748

Merged
merged 4 commits into from
Nov 24, 2015
Merged

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Nov 4, 2015

  • schema
  • Acceptance Tests
  • Documentation

This acceptance tests now work as follows:

make testacc TEST=./builtin/providers/digitalocean TESTARGS='-run=DigitalOceanFloatingIP' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/digitalocean -v -run=DigitalOceanFloatingIP -timeout 90m
=== RUN   TestAccDigitalOceanFloatingIP_Region
--- PASS: TestAccDigitalOceanFloatingIP_Region (24.08s)
=== RUN   TestAccDigitalOceanFloatingIP_Droplet
--- PASS: TestAccDigitalOceanFloatingIP_Droplet (93.20s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/digitalocean   117.288s

@stack72 stack72 changed the title [WIP] provider:digitalocean Floating IPs provider:digitalocean Floating IPs Nov 4, 2015
@stack72 stack72 changed the title provider:digitalocean Floating IPs provider/digitalocean : Floating IPs Nov 4, 2015
@carlosdp
Copy link
Contributor

Wait, why do we need to time.Sleep in Read again? Does waitForFloatingIPReady not cover waiting for the Floating IP to be unassigned/deleted?

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

@carlosdp I am not sure what action I can call waitForFloatingIPReady on. The code works as follows:

// Build up our creation options
    opts := &godo.FloatingIPCreateRequest{}

    if v, ok := d.GetOk("droplet_id"); ok {
        log.Printf("[INFO] Found a droplet_id to try and attach to the FloatingIP")
        opts.DropletID = v.(int)
    }

    log.Printf("[DEBUG] FloatingIP Create: %#v", opts)
    floatingIp, _, err := client.FloatingIPs.Create(opts)
    if err != nil {
        return fmt.Errorf("Error creating FloatingIP: %s", err)
    }
    d.SetId(floatingIp.IP)

The Create doesn't actually return a status (as per the API docs https://developers.digitalocean.com/documentation/v2/#create-a-new-floating-ip-assigned-to-a-droplet

To use the Action (client.FloatingIPActions.Assign), I need to pass an IP and a DropletID. I don't have an IP at this phase

the time.Sleep is only there as a last ditch attempt - I would love to be able to remove it

@carlosdp
Copy link
Contributor

Ahhhh, I see

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

@carlosdp it's kinda weird - right?

I have just been experimenting with this code instead:

func resourceDigitalOceanFloatingIpCreate(d *schema.ResourceData, meta interface{}) error {
    client := meta.(*godo.Client)

    if v, ok := d.GetOk("droplet_id"); ok {

        dropletOpts := &godo.FloatingIPCreateRequest{
            DropletID: v.(int),
        }

        log.Printf("[INFO] Creating FloatingIP")
        ipResp, _, err := client.FloatingIPs.Create(dropletOpts)
        if err != nil {
            return fmt.Errorf("Error Creating FloatingIP: %s", err)
        }

        log.Printf("[INFO] Assigning the Floating IP to the Droplet")
        action, _, err := client.FloatingIPActions.Assign(ipResp.IP, v.(int))
        if err != nil {
            return fmt.Errorf(
                "Error Assigning FloatingIP (%s) to the droplet: %s", d.Id(), err)
        }
        d.Set("ip_address", ipResp.IP)

        _, unassignedErr := waitForFloatingIPReady(d, "completed", []string{"new"}, "status", meta, action.ID)
        if unassignedErr != nil {
            return fmt.Errorf(
                "Error waiting for FloatingIP (%s) to be assigned: \n%s", d.Id(), unassignedErr)
        }
    } else if v, ok := d.GetOk("region"); ok {
        log.Printf("[INFO] Create a FloatingIP for a region")
        regionOpts := &godo.FloatingIPCreateRequest{
            Region: v.(string),
        }
        log.Printf("[DEBUG] FloatingIP Create: %#v", regionOpts)
        floatingIp, _, err := client.FloatingIPs.Create(regionOpts)
        if err != nil {
            return fmt.Errorf("Error creating FloatingIP: %s", err)
        }
        d.SetId(floatingIp.IP)
    } else {
        return fmt.Errorf("You need to specify a DropletId or a Region when creating a FloatingIP")
    }

    return resourceDigitalOceanFloatingIpRead(d, meta)
}

But I still get an error (just in a different place now)

* digitalocean_floating_ip.foobar: Error Assigning FloatingIP () to the droplet: POST https://api.digitalocean.com/v2/floating_ips/163.47.8.212/actions: 422 The floating IP already has a pending event.
2015/11/20 19:34:23 [ERROR] root: eval: *terraform.EvalSequence, err: 1 error(s) occurred:

So.... I think there either needs to be a change to the DO API or I am stuck

thoughts?

@carlosdp
Copy link
Contributor

@stack72 I think that might be because you created the Floating IP with a droplet_id and therefore it automatically "assigns" it to the droplet after it's created. So when you try to assign it again, it fails.

It's not in their docs, but I imagine you can "GET" the "actions" on a Floating IP instead after you create it? I would have to try it and see.

@carlosdp
Copy link
Contributor

@stack72 Yea, just confirmed that works, you can hit the /actions endpoint of a Floating IP and get a list of "actions" and use that in the waiting function.

(They should be documenting that, seems like a big oversight on their part, I'll let them know)

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

@carlosdp The go-SDK doesn't really support that.

// FloatingIPActionsService is an interface for interfacing with the
// floating IPs actions endpoints of the Digital Ocean API.
// See: https://developers.digitalocean.com/documentation/v2#floating-ips-action
type FloatingIPActionsService interface {
    Assign(ip string, dropletID int) (*Action, *Response, error)
    Unassign(ip string) (*Action, *Response, error)
    Get(ip string, actionID int) (*Action, *Response, error)
}

The Get request of the floating IPs actions requires an IP address and an actionId. Tried a 0 and still failed

TF_ACC=1 go test ./builtin/providers/digitalocean -v -run=FloatingIP_Droplet -timeout 90m
=== RUN   TestAccDigitalOceanFloatingIP_Droplet
--- FAIL: TestAccDigitalOceanFloatingIP_Droplet (52.36s)
    testing.go:137: Step 0 error: Error applying: 1 error(s) occurred:

        * digitalocean_floating_ip.foobar: Error Getting the Actions of the FloatingIP (): GET https://api.digitalocean.com/v2/floating_ips/163.47.8.212/actions/0: 404 The .
FAIL
exit status 1

I am going to open a request on their API to do this for me

@carlosdp
Copy link
Contributor

Are you going to open a PR? If not, I can do that for that library.

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

I was going to create an issue - I am not quite familiar enough with the API to open a PR right now

If you can help, that would be ace!

@carlosdp
Copy link
Contributor

Yea, I'll do that real quick, I really want this provider =P

@carlosdp
Copy link
Contributor

Opened digitalocean/godo#71, hopefully we can get a review on that soon. If you want to work against my branch though, just change the import directive to

import "github.com/carlosdp/godo"

until they merge it into master. That should allow you to test the changes and finalize this, pending DO.

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

@carlosdp I am getting the following error:

panic: interface conversion: interface {} is *godo.Client, not *godo.Client

@carlosdp
Copy link
Contributor

Ah, right, my bad. The import would have to be changed everywhere in the code path in order for it to work =P

Even though my Client is the "same", it's considered different by Go.

@stack72
Copy link
Contributor Author

stack72 commented Nov 20, 2015

So the code now looks as follows:

dropletOpts := &godo.FloatingIPCreateRequest{
            DropletID: v.(int),
        }

        log.Printf("[INFO] Creating FloatingIP")
        ipResp, _, err := client.FloatingIPs.Create(dropletOpts)
        if err != nil {
            return fmt.Errorf("Error Creating FloatingIP: %s", err)
        }

        log.Printf("[INFO] Assigning the Floating IP to the Droplet")
        actions, _, err := client.FloatingIPActions.List(ipResp.IP)
        if err != nil {
            return fmt.Errorf(
                "Error Getting the Actions of the FloatingIP (%s): %s", ipResp.IP, err)
        }

        _, unassignedErr := waitForFloatingIPReady(d, "completed", []string{"new"}, "status", meta, actions[0].ID)
        if unassignedErr != nil {
            return fmt.Errorf(
                "Error waiting for FloatingIP (%s) to be assigned: \n%s", ipResp.IP, unassignedErr)
        }

I am assuming that I shouldn't just use the actions[0].ID and range through the actions to match for the correct action - right?

@carlosdp
Copy link
Contributor

Yea, waitForFloatingIPReady or something should sort them by the datetime and grab the latest one and wait for that one to be "completed"

@carlosdp
Copy link
Contributor

The PR was merged on the godo lib, so you should be able to just go get -u it now!

@carlosdp
Copy link
Contributor

Alternatively, I guess if you know the droplet's region, you can just create the Floating IP on the region instead and then "Assign" it to the droplet, thereby giving you back the action ID, skipping all the listing logic. Your call.

@stack72
Copy link
Contributor Author

stack72 commented Nov 24, 2015

@carlosdp this has now been implemented and hopefully will be reviewed soon :)

make testacc TEST=./builtin/providers/digitalocean TESTARGS='-run=FloatingIP_' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/digitalocean -v -run=FloatingIP_ -timeout 90m
=== RUN   TestAccDigitalOceanFloatingIP_Region
--- PASS: TestAccDigitalOceanFloatingIP_Region (2.16s)
=== RUN   TestAccDigitalOceanFloatingIP_Droplet
--- PASS: TestAccDigitalOceanFloatingIP_Droplet (91.19s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/digitalocean   93.362s

The time.Sleep has now been removed

@jen20
Copy link
Contributor

jen20 commented Nov 24, 2015

Looks good to me. Thanks @stack72!

jen20 added a commit that referenced this pull request Nov 24, 2015
provider/digitalocean : New Resource for Floating IPs
@jen20 jen20 merged commit 1cde2e6 into hashicorp:master Nov 24, 2015
@carlosdp
Copy link
Contributor

👍 thanks @stack72 @jen20

@stack72 stack72 deleted the do-floatingips branch November 26, 2015 11:16
@ghost
Copy link

ghost commented Apr 29, 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 Apr 29, 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.

4 participants