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

Support in-place instance ephemeral and floating IP changes #378

Open
askfongjojo opened this issue Jan 8, 2025 · 4 comments · May be fixed by #381
Open

Support in-place instance ephemeral and floating IP changes #378

askfongjojo opened this issue Jan 8, 2025 · 4 comments · May be fixed by #381
Assignees
Labels

Comments

@askfongjojo
Copy link
Contributor

Overview

Currently, a change in the instance external IP spec results in a forced replacement in terraform apply, e.g.

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # oxide_instance.mysql[0] must be replaced
-/+ resource "oxide_instance" "mysql" {
      ~ external_ips       = [
          - { # forces replacement
              - id   = "6c75764b-0350-4dfe-9f13-920ae0c3e75f" -> null
              - type = "floating" -> null
            },
            # (1 unchanged element hidden)
        ]
      ~ id                 = "8bc31b24-22c7-4d40-82aa-22a603564abe" -> (known after apply)
        name               = "mysql-0"
      ~ network_interfaces = [
          - {
              - description   = "Default NIC" -> null
              - id            = "93193c53-74f9-4de3-8311-0a788a1706c6" -> null
              - ip_address    = "172.30.0.11" -> null
              - mac_address   = "A8:40:25:F0:00:06" -> null
              - name          = "default" -> null
              - primary       = true -> null
              - subnet_id     = "11a7986f-475d-4bf1-bb96-bacb3dbeec3a" -> null
              - time_created  = "2025-01-08 19:50:49.67032 +0000 UTC" -> null
              - time_modified = "2025-01-08 19:50:49.67032 +0000 UTC" -> null
              - vpc_id        = "cd36ca5d-b536-4316-8b26-e330cafa3b9f" -> null
            },
          + {
              + description   = "Default NIC"
              + id            = (known after apply)
              + ip_address    = (known after apply)
              + mac_address   = (known after apply)
              + name          = "default"
              + primary       = (known after apply)
              + subnet_id     = "11a7986f-475d-4bf1-bb96-bacb3dbeec3a"
              + time_created  = (known after apply)
              + time_modified = (known after apply)
              + vpc_id        = "cd36ca5d-b536-4316-8b26-e330cafa3b9f"
            },
        ]
      ~ time_created       = "2025-01-08 19:50:49.429581 +0000 UTC" -> (known after apply)
      ~ time_modified      = "2025-01-08 19:50:49.429581 +0000 UTC" -> (known after apply)
        # (8 unchanged attributes hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

There is API support to dynamically attach/detach floating IP and ephemeral IP without the need to stop/start the instance. We can improve the user experience and instance uptime by having terraform apply make those external IP changes without recreating instnaces.

Implementation details

No response

Anything else you would like to add?

No response

@sudomateo sudomateo self-assigned this Jan 9, 2025
@karencfv
Copy link
Collaborator

karencfv commented Jan 9, 2025

@askfongjojo I think there is an issue with the detach ephemeral IP endpoint. The attach one takes the name or ID of the IP pool to take the IP from, but the detach endpoint doesn't. Does it just detach everything? Is this the expected behaviour? If not, then I think that would block this issue for now.

sudomateo added a commit that referenced this issue Jan 9, 2025
Closes #378.

This patch updates the `oxide_instance` resource to support in-place
modifications to its `external_ips` attribute. That is, the instance
will no longer be destroyed and recreated when `external_ips` is
modified. Instead, the instance will be stopped, external IPs will be
detachd and/or attached, and the instance will be started.

This patch also updates the `Read` method to refresh the external IPs
attached to the instance. Previously, since the external IPs required a
resource replacement there was no need to refresh them.

There are 2 types of external IPs, ephemeral and floating. There can
be at most 1 ephemeral external IP attached to an instance and any
number of floating external IPs. That means in order to modify ephemeral
external IPs the external IPs must be detached first and then attached.

This patch needs the following work before it can be merged.
- [ ] Add tests for the new `external_ips` modification logic.
- [ ] Add validation logic to `external_ips` to enforce at most 1
ephemeral external IP. This was previously "validated" during instance
creation but is not updated during instance update.

Here's the error that's returned when an instance is created with more
than 1 ephemeral external IP.

```
oxide_instance.foo: Creating...
╷
│ Error: Error creating instance
│
│   with oxide_instance.foo,
│   on main.tf line 24, in resource "oxide_instance" "foo":
│   24: resource "oxide_instance" "foo" {
│
│ API error: POST https://oxide.example.com/v1/instances?project=5476ccc9-464d-4dc4-bfc0-5154de1c986f
│ ----------- RESPONSE -----------
│ Status: 400 InvalidRequest
│ Message: An instance may not have more than 1 ephemeral IP address
│ RequestID: fc6144e5-fa76-4583-a024-2e49ce17140e
│ ------- RESPONSE HEADERS -------
│ Content-Type: [application/json]
│ X-Request-Id: [fc6144e5-fa76-4583-a024-2e49ce17140e]
│ Date: [Thu, 09 Jan 2025 03:28:48 GMT]
│ Content-Length: [166]
│
╵
```
@sudomateo sudomateo linked a pull request Jan 9, 2025 that will close this issue
2 tasks
@sudomateo
Copy link
Collaborator

@askfongjojo I think there is an issue with the detach ephemeral IP endpoint. The attach one takes the name or ID of the IP pool to take the IP from, but the detach endpoint doesn't. Does it just detach everything? Is this the expected behaviour? If not, then I think that would block this issue for now.

I didn't see this comment until after I already started working on this issue in #381. An instance can only have 1 ephemeral external IP attached at a time. That means the API will have the following behavior.

sudomateo added a commit that referenced this issue Jan 9, 2025
Closes #378.

This patch updates the `oxide_instance` resource to support in-place
modifications to its `external_ips` attribute. That is, the instance
will no longer be destroyed and recreated when `external_ips` is
modified. Instead, the instance will be stopped, external IPs will be
detachd and/or attached, and the instance will be started.

This patch also updates the `Read` method to refresh the external IPs
attached to the instance. Previously, since the external IPs required a
resource replacement there was no need to refresh them.

There are 2 types of external IPs, ephemeral and floating. There can
be at most 1 ephemeral external IP attached to an instance and any
number of floating external IPs. That means in order to modify ephemeral
external IPs the external IPs must be detached first and then attached.

This patch needs the following work before it can be merged.
- [ ] Add tests for the new `external_ips` modification logic.
- [ ] Add validation logic to `external_ips` to enforce at most 1
ephemeral external IP. This was previously "validated" during instance
creation but is not updated during instance update.

Here's the error that's returned when an instance is created with more
than 1 ephemeral external IP.

```
oxide_instance.foo: Creating...
╷
│ Error: Error creating instance
│
│   with oxide_instance.foo,
│   on main.tf line 24, in resource "oxide_instance" "foo":
│   24: resource "oxide_instance" "foo" {
│
│ API error: POST https://oxide.example.com/v1/instances?project=5476ccc9-464d-4dc4-bfc0-5154de1c986f
│ ----------- RESPONSE -----------
│ Status: 400 InvalidRequest
│ Message: An instance may not have more than 1 ephemeral IP address
│ RequestID: fc6144e5-fa76-4583-a024-2e49ce17140e
│ ------- RESPONSE HEADERS -------
│ Content-Type: [application/json]
│ X-Request-Id: [fc6144e5-fa76-4583-a024-2e49ce17140e]
│ Date: [Thu, 09 Jan 2025 03:28:48 GMT]
│ Content-Length: [166]
│
╵
```
@askfongjojo
Copy link
Contributor Author

Yes, unlike floating IP, each instance can have no more than one ephemeral IP. We can model after how it works on the web console which already makes use of the attach/detach APIs.

@karencfv
Copy link
Collaborator

karencfv commented Jan 9, 2025

Thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants