Skip to content

Commit

Permalink
[ECS] Fix issue with multiple networks (#1057)
Browse files Browse the repository at this point in the history 
[ECS] Fix issue with multiple networks

Summary of the Pull Request
Fix issue with reading networks in resource/opentelekomcloud_compute_instance_v2
Fixes: #1039
PR Checklist

 Refers to: #1039
 Tests added/passed.
 Documentation updated.
 Schema updated.

Acceptance Steps Performed
=== RUN   TestAccComputeV2Instance_basic
--- PASS: TestAccComputeV2Instance_basic (140.33s)
=== RUN   TestAccComputeV2Instance_multiSecgroup
--- PASS: TestAccComputeV2Instance_multiSecgroup (149.64s)
=== RUN   TestAccComputeV2Instance_bootFromImage
--- PASS: TestAccComputeV2Instance_bootFromImage (79.78s)
=== RUN   TestAccComputeV2Instance_bootFromVolume
--- PASS: TestAccComputeV2Instance_bootFromVolume (78.77s)
=== RUN   TestAccComputeV2Instance_changeFixedIP
--- PASS: TestAccComputeV2Instance_changeFixedIP (80.90s)
=== RUN   TestAccComputeV2Instance_bootFromVolumeVolume
--- PASS: TestAccComputeV2Instance_bootFromVolumeVolume (100.41s)
=== RUN   TestAccComputeV2Instance_stopBeforeDestroy
--- PASS: TestAccComputeV2Instance_stopBeforeDestroy (126.18s)
=== RUN   TestAccComputeV2Instance_metadata
--- PASS: TestAccComputeV2Instance_metadata (137.88s)
=== RUN   TestAccComputeV2Instance_timeout
--- PASS: TestAccComputeV2Instance_timeout (87.94s)
=== RUN   TestAccComputeV2Instance_autoRecovery
--- PASS: TestAccComputeV2Instance_autoRecovery (127.58s)
=== RUN   TestAccComputeV2Instance_crazyNICs
--- PASS: TestAccComputeV2Instance_crazyNICs (231.48s)
=== RUN   TestAccComputeV2Instance_initialStateActive
--- PASS: TestAccComputeV2Instance_initialStateActive (215.49s)
=== RUN   TestAccComputeV2Instance_initialStateShutoff
--- PASS: TestAccComputeV2Instance_initialStateShutoff (287.69s)
PASS

Process finished with the exit code 0

Reviewed-by: None <None>
Reviewed-by: Anton Kachurin <katchuring@gmail.com>
Reviewed-by: Rodion Gyrbu <fpsoff@outlook.com>
  • Loading branch information
@lego963
lego963 authored May 14, 2021
1 parent 98525ba commit 5de0510
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 70 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/jen20/awspolicyequivalence v1.1.0
github.com/jinzhu/copier v0.2.3
github.com/mitchellh/go-homedir v1.1.0
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210504154301-ea895669e707
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210513155845-aa4509827820
github.com/unknwon/com v1.0.1
gopkg.in/yaml.v2 v2.4.0
)
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/oklog/run v1.0.0 h1:Ru7dDtJNOyC66gQ5dQmaCa0qIsAUFY3sFpK1Xk8igrw=
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210504154301-ea895669e707 h1:V1YQglGVFvt2l2anaTQO4sRlhnhajQZSbg6fRPPsrsI=
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210504154301-ea895669e707/go.mod h1:pzEP1kduNwv+hrI9R6/DFU/NiX7Kr9NiFjpQ7kJQTsM=
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210513155845-aa4509827820 h1:FV3zLfaCqsN07tJZu/AMKtv71XX5v1593VXZFKnqrLI=
github.com/opentelekomcloud/gophertelekomcloud v0.3.3-0.20210513155845-aa4509827820/go.mod h1:pzEP1kduNwv+hrI9R6/DFU/NiX7Kr9NiFjpQ7kJQTsM=
github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
Expand Down
1 change: 1 addition & 0 deletions opentelekomcloud/acceptance/ecs/resource_opentelekomcloud_compute_instance_v2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,7 @@ resource "opentelekomcloud_compute_instance_v2" "instance_1" {
]
name = "instance_1"
security_groups = ["default"]
network {
uuid = "%s"
}
Expand Down
87 changes: 42 additions & 45 deletions opentelekomcloud/services/ecs/compute_instance_v2_networking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//
// The end result, from the user's point of view, is a structured set of
// understandable network information within the instance resource.

package ecs

import (
Expand Down Expand Up @@ -74,16 +75,15 @@ type InstanceNetwork struct {
func getAllInstanceNetworks(d *schema.ResourceData, meta interface{}) ([]InstanceNetwork, error) {
var instanceNetworks []InstanceNetwork

networks := d.Get("network").([]interface{})
for _, v := range networks {
networksRaw := d.Get("network").([]interface{})
for _, v := range networksRaw {
network := v.(map[string]interface{})
networkID := network["uuid"].(string)
networkName := network["name"].(string)
portID := network["port"].(string)

if networkID == "" && networkName == "" && portID == "" {
return nil, fmt.Errorf(
"At least one of network.uuid, network.name, or network.port must be set.")
return nil, fmt.Errorf("at least one of network.uuid, network.name, or network.port must be set")
}

// If a user specified both an ID and name, that makes things easy
Expand Down Expand Up @@ -162,7 +162,7 @@ func GetInstanceNetworkInfo(
if err == nil {
networkInfo, err := getInstanceNetworkInfoNeutron(networkClient, queryType, queryTerm)
if err != nil {
return nil, fmt.Errorf("Error trying to get network information from the Network API: %s", err)
return nil, fmt.Errorf("error trying to get network information from the Network API: %w", err)
}

return networkInfo, nil
Expand All @@ -173,12 +173,12 @@ func GetInstanceNetworkInfo(

computeClient, err := config.ComputeV2Client(config.GetRegion(d))
if err != nil {
return nil, fmt.Errorf("Error creating OpenTelekomCloud compute client: %s", err)
return nil, fmt.Errorf("error creating OpenTelekomCloud compute client: %w", err)
}

networkInfo, err := getInstanceNetworkInfoNovaNet(computeClient, queryType, queryTerm)
if err != nil {
return nil, fmt.Errorf("Error trying to get network information from the Nova API: %s", err)
return nil, fmt.Errorf("error trying to get network information from the Nova API: %w", err)
}

return networkInfo, nil
Expand All @@ -191,20 +191,17 @@ func getInstanceNetworkInfoNovaNet(

// If somehow a port ended up here, we should just error out.
if queryType == "port" {
return nil, fmt.Errorf(
"Unable to query a port (%s) using the Nova API", queryTerm)
return nil, fmt.Errorf("unable to query a port (%s) using the Nova API", queryTerm)
}

allPages, err := tenantnetworks.List(client).AllPages()
if err != nil {
return nil, fmt.Errorf(
"An error occurred while querying the Nova API for network information: %s", err)
return nil, fmt.Errorf("an error occurred while querying the Nova API for network information: %w", err)
}

networkList, err := tenantnetworks.ExtractNetworks(allPages)
if err != nil {
return nil, fmt.Errorf(
"An error occurred while querying the Nova API for network information: %s", err)
return nil, fmt.Errorf("an error occurred while querying the Nova API for network information: %w", err)
}

var networkFound bool
Expand Down Expand Up @@ -234,7 +231,7 @@ func getInstanceNetworkInfoNovaNet(
return v, nil
}

return nil, fmt.Errorf("Could not find any matching network for %s %s", queryType, queryTerm)
return nil, fmt.Errorf("could not find any matching network for %s %s", queryType, queryTerm)
}

// getInstanceNetworkInfoNeutron will query the neutron API for the network
Expand All @@ -250,22 +247,22 @@ func getInstanceNetworkInfoNeutron(
}
allPages, err := ports.List(client, listOpts).AllPages()
if err != nil {
return nil, fmt.Errorf("Unable to retrieve networks from the Network API: %s", err)
return nil, fmt.Errorf("unable to retrieve networks from the Network API: %w", err)
}

allPorts, err := ports.ExtractPorts(allPages)
if err != nil {
return nil, fmt.Errorf("Unable to retrieve networks from the Network API: %s", err)
return nil, fmt.Errorf("unable to retrieve networks from the Network API: %w", err)
}

var port ports.Port
switch len(allPorts) {
case 0:
return nil, fmt.Errorf("Could not find any matching port for %s %s", queryType, queryTerm)
return nil, fmt.Errorf("could not find any matching port for %s %s", queryType, queryTerm)
case 1:
port = allPorts[0]
default:
return nil, fmt.Errorf("More than one port found for %s %s", queryType, queryTerm)
return nil, fmt.Errorf("more than one port found for %s %s", queryType, queryTerm)
}

queryType = "id"
Expand All @@ -285,22 +282,22 @@ func getInstanceNetworkInfoNeutron(

allPages, err := networks.List(client, listOpts).AllPages()
if err != nil {
return nil, fmt.Errorf("Unable to retrieve networks from the Network API: %s", err)
return nil, fmt.Errorf("unable to retrieve networks from the Network API: %w", err)
}

allNetworks, err := networks.ExtractNetworks(allPages)
if err != nil {
return nil, fmt.Errorf("Unable to retrieve networks from the Network API: %s", err)
return nil, fmt.Errorf("unable to retrieve networks from the Network API: %w", err)
}

var network networks.Network
switch len(allNetworks) {
case 0:
return nil, fmt.Errorf("Could not find any matching network for %s %s", queryType, queryTerm)
return nil, fmt.Errorf("could not find any matching network for %s %s", queryType, queryTerm)
case 1:
network = allNetworks[0]
default:
return nil, fmt.Errorf("More than one network found for %s %s", queryType, queryTerm)
return nil, fmt.Errorf("more than one network found for %s %s", queryType, queryTerm)
}

v := map[string]interface{}{
Expand Down Expand Up @@ -371,17 +368,17 @@ func getInstanceAddresses(addresses map[string]interface{}) []InstanceAddresses
// expandInstanceNetworks takes network information found in []InstanceNetwork
// and builds a golangsdk []servers.Network for use in creating an Instance.
func expandInstanceNetworks(allInstanceNetworks []InstanceNetwork) []servers.Network {
var networks []servers.Network
var networkList []servers.Network
for _, v := range allInstanceNetworks {
n := servers.Network{
UUID: v.UUID,
Port: v.Port,
FixedIP: v.FixedIP,
}
networks = append(networks, n)
networkList = append(networkList, n)
}

return networks
return networkList
}

// FlattenInstanceNetworks collects instance network information from different
Expand All @@ -392,21 +389,26 @@ func FlattenInstanceNetworks(
config := meta.(*cfg.Config)
computeClient, err := config.ComputeV2Client(config.GetRegion(d))
if err != nil {
return nil, fmt.Errorf("Error creating OpenTelekomCloud compute client: %s", err)
return nil, fmt.Errorf("error creating OpenTelekomCloud compute client: %w", err)
}

server, err := servers.Get(computeClient, d.Id()).Extract()
if err != nil {
return nil, common.CheckDeleted(d, err, "server")
}

serverNICs, err := servers.GetNICs(computeClient, server.ID).Extract()
if err != nil {
return nil, err
}

allInstanceAddresses := getInstanceAddresses(server.Addresses)
allInstanceNetworks, err := getAllInstanceNetworks(d, meta)
if err != nil {
return nil, err
}

networks := []map[string]interface{}{}
var networkList []map[string]interface{}

// If there were no instance networks returned, this means that there
// was not a network specified in the Terraform configuration. When this
Expand All @@ -423,24 +425,21 @@ func FlattenInstanceNetworks(
"mac": instanceNIC.MAC,
}

// Use the same method as getAllInstanceNetworks to get the network uuid
networkInfo, err := GetInstanceNetworkInfo(d, meta, "name", instanceAddresses.NetworkName)
if err != nil {
log.Printf("[WARN] Error getting default network uuid: %s", err)
} else {
if v["uuid"] != nil {
v["uuid"] = networkInfo["uuid"].(string)
} else {
log.Printf("[WARN] Could not get default network uuid")
for _, nicObj := range serverNICs {
if nicObj.MACAddress == instanceNIC.MAC {
v["uuid"] = nicObj.NetID
}
}
if v["uuid"] == "" {
log.Printf("[WARN] Could not get default network uuid")
}

networks = append(networks, v)
networkList = append(networkList, v)
}
}

log.Printf("[DEBUG] flattenInstanceNetworks: %#v", networks)
return networks, nil
log.Printf("[DEBUG] flattenInstanceNetworks: %#v", networkList)
return networkList, nil
}

// Loop through all networks and addresses, merge relevant address details.
Expand All @@ -465,22 +464,20 @@ func FlattenInstanceNetworks(
"port": instanceNetwork.Port,
"access_network": instanceNetwork.AccessNetwork,
}
networks = append(networks, v)
networkList = append(networkList, v)
}
}
}

log.Printf("[DEBUG] flattenInstanceNetworks: %#v", networks)
return networks, nil
log.Printf("[DEBUG] flattenInstanceNetworks: %#v", networkList)
return networkList, nil
}

// GetInstanceAccessAddresses determines the best IP address to communicate
// with the instance. It does this by looping through all networks and looking
// for a valid IP address. Priority is given to a network that was flagged as
// an access_network.
func GetInstanceAccessAddresses(
d *schema.ResourceData, networks []map[string]interface{}) (string, string) {

func GetInstanceAccessAddresses(d *schema.ResourceData, networks []map[string]interface{}) (string, string) {
var hostv4, hostv6 string

// Loop through all networks
Expand Down
Loading

0 comments on commit 5de0510

Please sign in to comment.