Skip to content

Commit

Permalink
fix(server): do not change primary private IP on apply
Browse files Browse the repository at this point in the history
  • Loading branch information
kangasta committed Nov 4, 2024
1 parent b090a1e commit bd9ff59
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 26 deletions.
41 changes: 24 additions & 17 deletions internal/service/server/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,19 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func findInterface(ifaces []upcloud.ServerInterface, index int, ip string) *upcloud.ServerInterface {
func findInterface(ifaces []upcloud.ServerInterface, index int) *upcloud.ServerInterface {
for _, iface := range ifaces {
if iface.Index == index {
return &iface
}
}
for _, iface := range ifaces {
if len(iface.IPAddresses) > 0 && iface.IPAddresses[0].Address == ip {
return &iface
}
}
return nil
}

func interfacesToMap(ifaces []interface{}) map[int]interface{} {
m := make(map[int]interface{})
for _, iface := range ifaces {
val := iface.(map[string]interface{})
m[val["index"].(int)] = iface
for i, iface := range ifaces {
m[i] = iface
}
return m
}
Expand All @@ -57,13 +51,13 @@ func canModifyInterface(plan map[string]interface{}, prev *upcloud.ServerInterfa
if v, ok := plan["type"].(string); !ok || prev.Type != v {
return false
}
if v, ok := plan["network"].(string); !ok || prev.Network != v {
return false
}
if v, ok := plan["ip_address_family"].(string); !ok || len(prev.IPAddresses) > 0 && prev.IPAddresses[0].Family != v {
return false
}
if prev.Type == upcloud.NetworkTypePrivate {
if v, ok := plan["network"].(string); !ok || prev.Network != v {
return false
}
if v, ok := plan["ip_address"].(string); !ok || len(prev.IPAddresses) > 0 && prev.IPAddresses[0].Address != v {
return false
}
Expand Down Expand Up @@ -103,18 +97,31 @@ func shouldModifyInterface(plan map[string]interface{}, addresses request.Create
return false
}

func setInterfaceValues(iface *upcloud.Interface) map[string]interface{} {
func setInterfaceValues(iface *upcloud.Interface, ipInState interface{}) map[string]interface{} {
ni := make(map[string]interface{})
additionalIPAddresses := []map[string]interface{}{}

// IP addresses are not returned in deterministic order. If any of the IP addresses of the interface match the IP address in state, use that.
if ip, ok := ipInState.(string); ok {
for _, ipAddress := range iface.IPAddresses {
if ipAddress.Address == ip {
ni["ip_address_family"] = ipAddress.Family
ni["ip_address"] = ipAddress.Address
if !ipAddress.Floating.Empty() {
ni["ip_address_floating"] = ipAddress.Floating.Bool()
}
}
}
}

for i, ipAddress := range iface.IPAddresses {
if i == 0 {
if i == 0 && ni["ip_address"] == nil {
ni["ip_address_family"] = ipAddress.Family
ni["ip_address"] = ipAddress.Address
if !ipAddress.Floating.Empty() {
ni["ip_address_floating"] = ipAddress.Floating.Bool()
}
} else if iface.Type == upcloud.NetworkTypePrivate {
} else if iface.Type == upcloud.NetworkTypePrivate && ipAddress.Address != ni["ip_address"].(string) {
additionalIPAddress := map[string]interface{}{
"ip_address": ipAddress.Address,
"ip_address_family": ipAddress.Family,
Expand Down Expand Up @@ -231,7 +238,7 @@ func updateServerNetworkInterfaces(ctx context.Context, svc *service.Service, d
}

if modified := modifiedInterfaces[index]; modified != nil {
newNetworkInterfaces[i] = setInterfaceValues(modified)
newNetworkInterfaces[i] = setInterfaceValues(modified, val["ip_address"])
continue
}

Expand Down Expand Up @@ -269,7 +276,7 @@ func updateServerNetworkInterfaces(ctx context.Context, svc *service.Service, d
if err != nil {
return err
}
newNetworkInterfaces[i] = setInterfaceValues(iface)
newNetworkInterfaces[i] = setInterfaceValues(iface, val["ip_address"])
}

if err := d.Set("network_interface", newNetworkInterfaces); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/service/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ func resourceServerRead(ctx context.Context, d *schema.ResourceData, meta interf
}
networkInterfaces := make([]map[string]interface{}, n)
for i := 0; i < n; i++ {
key := fmt.Sprintf("network_interface.%d", i)
key := interfaceKey(i)
val, ok := d.Get(key).(map[string]interface{})
if !ok {
return diag.Errorf("unable to read '%s' value", key)
Expand All @@ -607,13 +607,13 @@ func resourceServerRead(ctx context.Context, d *schema.ResourceData, meta interf
if index := val["index"].(int); index == 0 && i < len(server.Networking.Interfaces) {
iface = &server.Networking.Interfaces[i]
} else {
iface = findInterface(server.Networking.Interfaces, val["index"].(int), "")
iface = findInterface(server.Networking.Interfaces, val["index"].(int))
}
if iface == nil {
continue
}

ni := setInterfaceValues((*upcloud.Interface)(iface))
ni := setInterfaceValues((*upcloud.Interface)(iface), val["ip_address"])
networkInterfaces[i] = ni

if iface.Type == upcloud.NetworkTypePublic &&
Expand Down
4 changes: 2 additions & 2 deletions upcloud/resource_upcloud_server_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func TestAccUpcloudServerInterfaceMatching(t *testing.T) {
Config: testDataS3,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr(this, "network_interface.#", "3"),
resource.TestCheckResourceAttr(this, "network_interface.0.index", "4"),
checkStringDoesNotChange(this, "network_interface.0.ip_address", &thisIP4),
resource.TestCheckResourceAttr(this, "network_interface.0.index", "5"),
checkStringDoesNotChange(this, "network_interface.0.ip_address", &thisIP5),
resource.TestCheckResourceAttr(this, "network_interface.1.index", "10"),
checkStringDoesNotChange(this, "network_interface.1.ip_address", &thisIP1),
resource.TestCheckResourceAttr(this, "network_interface.2.index", "3"),
Expand Down
9 changes: 5 additions & 4 deletions upcloud/testdata/upcloud_server/server_ifaces_s3.tf
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ resource "upcloud_network" "this" {
}
}

resource "upcloud_floating_ip_address" "this" {
mac_address = upcloud_server.this.network_interface[0].mac_address
}
# Nested field can not be set as unkown, so we need to remove the floating IP address to avoid data consistency error: https://github.com/hashicorp/terraform-plugin-sdk/issues/459
# resource "upcloud_floating_ip_address" "this" {
# mac_address = upcloud_server.this.network_interface[1].mac_address
# }

resource "upcloud_server" "this" {
hostname = "${var.prefix}vm"
Expand All @@ -34,7 +35,7 @@ resource "upcloud_server" "this" {
keys = ["ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIB8Q"]
}

// Reorder interfaces
// Reorder interfaces, remove interface with index 4

network_interface {
index = 5
Expand Down

0 comments on commit bd9ff59

Please sign in to comment.