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

Add DetachIP helper #378

Merged
merged 1 commit into from
Jul 13, 2016
Merged

Add DetachIP helper #378

merged 1 commit into from
Jul 13, 2016

Conversation

nicolai86
Copy link
Contributor

@nicolai86 nicolai86 commented Jul 12, 2016

While working on my terraform provider I had the need to detach IPs from servers.

This doesn't seem to be exposed in the SDK so I added a helper function. However, I think it should be public within the SDK.

One question tho: do we need to stop servers in order to detach IPs ?

@@ -2313,6 +2313,35 @@ func (s *ScalewayAPI) AttachIP(ipID, serverID string) error {
return err
}

// DetachIP attachs an IP to a server
Copy link
Contributor

Choose a reason for hiding this comment

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

DetachIP detachs an IP from a server

@QuentinPerez
Copy link
Contributor

QuentinPerez commented Jul 12, 2016

Hi @nicolai86, one more time, thank you for your work ☺️

Can you take a look on my comments ?

For your question, no you don't need to stop the server to detach the IP.

Let me know if you need anything else.

@nicolai86
Copy link
Contributor Author

Late night copy pasta and this is the result .
Thanks for taking a look I'll clean this up today.

@nicolai86
Copy link
Contributor Author

@QuentinPerez thanks for the quick review yesterday. I've cleaned up the PR accordingly.

if err != nil {
return err
}
defer resp.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

You have to add this code between s.PutResponse and if err != nil

if resp != nil {
    defer resp.Body.Close()
}

according to http://devs.cloudimmunity.com/gotchas-and-common-mistakes-in-go-golang/#anameclose_http_resp_bodyaclosinghttpresponsebody

@QuentinPerez
Copy link
Contributor

I merge your PR, and I will fix some issues on our ScalewayIPScaleway to detach an IP more easily.

Thanks a lot !!! 😊 👍🏻

@QuentinPerez QuentinPerez merged commit 881a24e into scaleway:master Jul 13, 2016
@QuentinPerez QuentinPerez mentioned this pull request Jul 13, 2016
Merged
stack72 pushed a commit to hashicorp/terraform that referenced this pull request Jul 13, 2016
* Add scaleway provider

this PR allows the entire scaleway stack to be managed with terraform

example usage looks like this:

```
provider "scaleway" {
  api_key = "snap"
  organization = "snip"
}

resource "scaleway_ip" "base" {
  server = "${scaleway_server.base.id}"
}

resource "scaleway_server" "base" {
  name = "test"
  # ubuntu 14.04
  image = "aecaed73-51a5-4439-a127-6d8229847145"
  type = "C2S"
}

resource "scaleway_volume" "test" {
  name = "test"
  size_in_gb = 20
  type = "l_ssd"
}

resource "scaleway_volume_attachment" "test" {
  server = "${scaleway_server.base.id}"
  volume = "${scaleway_volume.test.id}"
}

resource "scaleway_security_group" "base" {
  name = "public"
  description = "public gateway"
}

resource "scaleway_security_group_rule" "http-ingress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "inbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}

resource "scaleway_security_group_rule" "http-egress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "outbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}
```

Note that volume attachments require the server to be stopped, which can lead to
downtimes of you attach new volumes to already used servers

* Update IP read to handle 404 gracefully

* Read back resource on update

* Ensure IP detachment works as expected

Sadly this is not part of the official scaleway api just yet

* Adjust detachIP helper

based on feedback from @QuentinPerez in
scaleway/scaleway-cli#378

* Cleanup documentation

* Rename api_key to access_key

following @stack72 suggestion and rename the provider api_key for more clarity

* Make tests less chatty by using custom logger
iceycake pushed a commit to ticketmaster/terraform that referenced this pull request Jul 22, 2016
* Add scaleway provider

this PR allows the entire scaleway stack to be managed with terraform

example usage looks like this:

```
provider "scaleway" {
  api_key = "snap"
  organization = "snip"
}

resource "scaleway_ip" "base" {
  server = "${scaleway_server.base.id}"
}

resource "scaleway_server" "base" {
  name = "test"
  # ubuntu 14.04
  image = "aecaed73-51a5-4439-a127-6d8229847145"
  type = "C2S"
}

resource "scaleway_volume" "test" {
  name = "test"
  size_in_gb = 20
  type = "l_ssd"
}

resource "scaleway_volume_attachment" "test" {
  server = "${scaleway_server.base.id}"
  volume = "${scaleway_volume.test.id}"
}

resource "scaleway_security_group" "base" {
  name = "public"
  description = "public gateway"
}

resource "scaleway_security_group_rule" "http-ingress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "inbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}

resource "scaleway_security_group_rule" "http-egress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "outbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}
```

Note that volume attachments require the server to be stopped, which can lead to
downtimes of you attach new volumes to already used servers

* Update IP read to handle 404 gracefully

* Read back resource on update

* Ensure IP detachment works as expected

Sadly this is not part of the official scaleway api just yet

* Adjust detachIP helper

based on feedback from @QuentinPerez in
scaleway/scaleway-cli#378

* Cleanup documentation

* Rename api_key to access_key

following @stack72 suggestion and rename the provider api_key for more clarity

* Make tests less chatty by using custom logger
grubernaut pushed a commit to scaleway/terraform-provider-scaleway that referenced this pull request Jun 6, 2017
* Add scaleway provider

this PR allows the entire scaleway stack to be managed with terraform

example usage looks like this:

```
provider "scaleway" {
  api_key = "snap"
  organization = "snip"
}

resource "scaleway_ip" "base" {
  server = "${scaleway_server.base.id}"
}

resource "scaleway_server" "base" {
  name = "test"
  # ubuntu 14.04
  image = "aecaed73-51a5-4439-a127-6d8229847145"
  type = "C2S"
}

resource "scaleway_volume" "test" {
  name = "test"
  size_in_gb = 20
  type = "l_ssd"
}

resource "scaleway_volume_attachment" "test" {
  server = "${scaleway_server.base.id}"
  volume = "${scaleway_volume.test.id}"
}

resource "scaleway_security_group" "base" {
  name = "public"
  description = "public gateway"
}

resource "scaleway_security_group_rule" "http-ingress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "inbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}

resource "scaleway_security_group_rule" "http-egress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "outbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}
```

Note that volume attachments require the server to be stopped, which can lead to
downtimes of you attach new volumes to already used servers

* Update IP read to handle 404 gracefully

* Read back resource on update

* Ensure IP detachment works as expected

Sadly this is not part of the official scaleway api just yet

* Adjust detachIP helper

based on feedback from @QuentinPerez in
scaleway/scaleway-cli#378

* Cleanup documentation

* Rename api_key to access_key

following @stack72 suggestion and rename the provider api_key for more clarity

* Make tests less chatty by using custom logger
grubernaut pushed a commit to scaleway/terraform-provider-scaleway that referenced this pull request Jun 9, 2017
* Add scaleway provider

this PR allows the entire scaleway stack to be managed with terraform

example usage looks like this:

```
provider "scaleway" {
  api_key = "snap"
  organization = "snip"
}

resource "scaleway_ip" "base" {
  server = "${scaleway_server.base.id}"
}

resource "scaleway_server" "base" {
  name = "test"
  # ubuntu 14.04
  image = "aecaed73-51a5-4439-a127-6d8229847145"
  type = "C2S"
}

resource "scaleway_volume" "test" {
  name = "test"
  size_in_gb = 20
  type = "l_ssd"
}

resource "scaleway_volume_attachment" "test" {
  server = "${scaleway_server.base.id}"
  volume = "${scaleway_volume.test.id}"
}

resource "scaleway_security_group" "base" {
  name = "public"
  description = "public gateway"
}

resource "scaleway_security_group_rule" "http-ingress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "inbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}

resource "scaleway_security_group_rule" "http-egress" {
  security_group = "${scaleway_security_group.base.id}"

  action = "accept"
  direction = "outbound"
  ip_range = "0.0.0.0/0"
  protocol = "TCP"
  port = 80
}
```

Note that volume attachments require the server to be stopped, which can lead to
downtimes of you attach new volumes to already used servers

* Update IP read to handle 404 gracefully

* Read back resource on update

* Ensure IP detachment works as expected

Sadly this is not part of the official scaleway api just yet

* Adjust detachIP helper

based on feedback from @QuentinPerez in
scaleway/scaleway-cli#378

* Cleanup documentation

* Rename api_key to access_key

following @stack72 suggestion and rename the provider api_key for more clarity

* Make tests less chatty by using custom logger
clement-gilbert pushed a commit to clement-gilbert/scaleway-cli that referenced this pull request Mar 3, 2022
**C'est trop de labal.**
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.

2 participants