Skip to content

Commit 9473094

Browse files
author
Jing Zhang
committed
Take care comments for code clarity:
(1) Name variables and functions with subport (2) Cut the unit test to the core function
1 parent 7c0c862 commit 9473094

File tree

2 files changed

+36
-135
lines changed

2 files changed

+36
-135
lines changed

pkg/openstack/instances.go

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
583583
addrs := []v1.NodeAddress{}
584584

585585
// parse private IP addresses first in an ordered manner
586-
allPrivates := make(map[string][]Address)
586+
addressesByNetworkID := make(map[string][]Address)
587587
for _, port := range ports {
588588
for _, fixedIP := range port.FixedIPs {
589589
if port.Status == "ACTIVE" {
@@ -595,10 +595,8 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
595595
Address: fixedIP.IPAddress,
596596
},
597597
)
598-
if port.NetworkID != "" {
599-
addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress}
600-
allPrivates[port.NetworkID] = append(allPrivates[port.NetworkID], addr)
601-
}
598+
addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress}
599+
addressesByNetworkID[port.NetworkID] = append(addressesByNetworkID[port.NetworkID], addr)
602600
}
603601
}
604602
}
@@ -639,10 +637,11 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
639637
return nil, err
640638
}
641639

642-
// Add the private addresses that are not directly connected
643-
if len(addresses) < len(allPrivates) {
644-
numExtraPrivates := addExtraNodeAddresses(addresses, allPrivates)
645-
klog.V(5).Infof("Node '%s' is added %d extra private interfaces", srv.Name, numExtraPrivates)
640+
// Add the addresses assigned on subports via trunk
641+
// This adds the vlan networks to which subports are attached
642+
if len(addresses) < len(addressesByNetworkID) {
643+
nrSubportAddresses := addSubportNodeAddresses(addresses, addressesByNetworkID)
644+
klog.V(5).Infof("Node '%s' is added %d subport addresses", srv.Name, nrSubportAddresses)
646645
}
647646

648647
var networks []string
@@ -698,38 +697,40 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
698697
return addrs, nil
699698
}
700699

701-
// addExtraNodeAddresses addes addresses attached via trunk
702-
func addExtraNodeAddresses(addresses, allPrivates map[string][]Address) int {
703-
extraPrivates := make(map[string][]Address)
704-
for network, interfaceAddresses := range allPrivates {
700+
// addSubportNodeAddresses adds addresses assigned on subports
701+
func addSubportNodeAddresses(addresses, addressesByNetworkID map[string][]Address) int {
702+
subportAddresses := make(map[string][]Address)
703+
for network, portAddresses := range addressesByNetworkID {
705704
found := false
706-
// For each address in the private network
707-
for _, interfaceAddress := range interfaceAddresses {
708-
// Check if the address is directly assigned
705+
for _, portAddress := range portAddresses {
706+
// Check if the address is directly attached to the node
709707
for _, nodeAddresses := range addresses {
710708
for _, nodeAddress := range nodeAddresses {
711-
if interfaceAddress.Addr == nodeAddress.Addr {
709+
if portAddress.Addr == nodeAddress.Addr {
712710
found = true
713711
break
714712
}
715713
}
716714
}
717715
}
718-
// All the addresses in the private network are not directly assigned
719-
// Save the private network
716+
// All the addresses in the vlan network are not directly attached to the node
717+
// Save the vlan network
720718
if !found {
721-
extraPrivates[network] = interfaceAddresses
719+
subportAddresses[network] = portAddresses
722720
}
723721
}
724-
for network, interfaceAddresses := range extraPrivates {
722+
// add subport addresses to the node addresses
723+
for network, portAddresses := range subportAddresses {
725724
srvAddresses, ok := addresses[network]
726725
if !ok {
727-
addresses[network] = interfaceAddresses
726+
addresses[network] = portAddresses
728727
} else {
729-
addresses[network] = append(srvAddresses, interfaceAddresses...)
728+
// this is to take care the corner case
729+
// where the same network is attached to the node both directly and via trunk
730+
addresses[network] = append(srvAddresses, portAddresses...)
730731
}
731732
}
732-
return len(extraPrivates)
733+
return len(subportAddresses)
733734
}
734735

735736
func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
@@ -758,24 +759,20 @@ func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]por
758759
trunk_details.TrunkDetailsExt
759760
}
760761

761-
var interfaces []ports.Port
762-
var allPorts []portWithTrunkDetails
762+
var allPorts []ports.Port
763+
var allPortDetails []portWithTrunkDetails
763764

764765
allPages, err := ports.List(client, listOpts).AllPages()
765766
if err != nil {
766-
return interfaces, err
767+
return allPorts, err
767768
}
768-
err = ports.ExtractPortsInto(allPages, &allPorts)
769+
err = ports.ExtractPortsInto(allPages, &allPortDetails)
769770
if err != nil {
770-
return interfaces, err
771+
return allPorts, err
771772
}
772773

773-
for _, portExt := range allPorts {
774-
interfaces = append(interfaces, portExt.Port)
775-
// Check if trunk is attached to the port
776-
if portExt.TrunkDetails.TrunkID == "" {
777-
continue
778-
}
774+
for _, portExt := range allPortDetails {
775+
allPorts = append(allPorts, portExt.Port)
779776
// Get subport addresses
780777
for _, subport := range portExt.TrunkDetails.SubPorts {
781778
p, err := ports.Get(client, subport.PortID).Extract()
@@ -790,10 +787,10 @@ func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]por
790787
}
791788
p.Status = "ACTIVE"
792789
p.NetworkID = n.Name
793-
interfaces = append(interfaces, *p)
790+
allPorts = append(allPorts, *p)
794791
}
795792
}
796793

797-
klog.V(5).Infof("Node %s has %d interfaces '%v'", serverID, len(interfaces), interfaces)
798-
return interfaces, nil
794+
klog.V(5).Infof("Node %s has %d ports '%v'", serverID, len(allPorts), allPorts)
795+
return allPorts, nil
799796
}

pkg/openstack/openstack_test.go

Lines changed: 1 addition & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -896,18 +896,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
896896
"addr": "3000:0:0:1::3a3",
897897
"OS-EXT-IPS:type": "fixed",
898898
},
899-
map[string]interface{}{
900-
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:1d:b7:a7",
901-
"version": float64(4),
902-
"addr": "13.13.13.186",
903-
"OS-EXT-IPS:type": "fixed",
904-
},
905-
map[string]interface{}{
906-
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:1d:b7:a7",
907-
"version": float64(6),
908-
"addr": "3000:0:0:1::38a",
909-
"OS-EXT-IPS:type": "fixed",
910-
},
911899
},
912900
"flat_net_phys6": []interface{}{
913901
map[string]interface{}{
@@ -922,24 +910,12 @@ func TestNodeAddressesWithSubports(t *testing.T) {
922910
"addr": "3000:0:0:1::1a3",
923911
"OS-EXT-IPS:type": "fixed",
924912
},
925-
map[string]interface{}{
926-
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:36:b7:8c",
927-
"version": float64(4),
928-
"addr": "14.14.14.98",
929-
"OS-EXT-IPS:type": "fixed",
930-
},
931-
map[string]interface{}{
932-
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:36:b7:8c",
933-
"version": float64(6),
934-
"addr": "3000:0:0:1::39d",
935-
"OS-EXT-IPS:type": "fixed",
936-
},
937913
},
938914
"mycluster-01-vlan702_network": []interface{}{
939915
map[string]interface{}{
940916
"OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:36:b7:8f",
941917
"version": float64(4),
942-
"addr": "169.252.60.15",
918+
"addr": "172.16.2.30",
943919
"OS-EXT-IPS:type": "fixed",
944920
},
945921
},
@@ -963,22 +939,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
963939
NetworkID: "7da9e62c-2ec1-4784-91f1-a6b560d7b665",
964940
MACAddress: "fa:16:3e:b3:9f:c2",
965941
},
966-
{
967-
Status: "ACTIVE",
968-
FixedIPs: []ports.IP{
969-
{
970-
SubnetID: "491fba80-095e-4dee-bf4b-69046ba0b7fa",
971-
IPAddress: "13.13.13.186",
972-
},
973-
{
974-
SubnetID: "fb6ed503-7912-45cf-9bb6-15543296e960",
975-
IPAddress: "3000:0:0:1::38a",
976-
},
977-
},
978-
ID: "97a49a02-3703-4632-be5e-a947a1e44138",
979-
NetworkID: "7da9e62c-2ec1-4784-91f1-a6b560d7b665",
980-
MACAddress: "fa:16:3e:1d:b7:a7",
981-
},
982942
{
983943
Status: "ACTIVE",
984944
FixedIPs: []ports.IP{
@@ -995,22 +955,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
995955
NetworkID: "59c2e3cc-26a2-44c8-b8dc-d10f3995bcd9",
996956
MACAddress: "fa:16:3e:15:20:50",
997957
},
998-
{
999-
Status: "ACTIVE",
1000-
FixedIPs: []ports.IP{
1001-
{
1002-
SubnetID: "b1c1f54e-f7cc-474f-be4c-f559ffb7fc94",
1003-
IPAddress: "14.14.14.98",
1004-
},
1005-
{
1006-
SubnetID: "3d3ab7f7-e235-4476-81a2-62982f9f9673",
1007-
IPAddress: "3000:0:0:1::39d",
1008-
},
1009-
},
1010-
ID: "a9145a03-1c58-442e-8ca5-cf85a65fdae4",
1011-
NetworkID: "59c2e3cc-26a2-44c8-b8dc-d10f3995bcd9",
1012-
MACAddress: "fa:16:3e:36:b7:8c",
1013-
},
1014958
{
1015959
Status: "ACTIVE",
1016960
FixedIPs: []ports.IP{
@@ -1023,18 +967,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
1023967
NetworkID: "ext-net1",
1024968
MACAddress: "fa:16:3e:ea:50:c9",
1025969
},
1026-
{
1027-
Status: "ACTIVE",
1028-
FixedIPs: []ports.IP{
1029-
{
1030-
SubnetID: "c4a00f06-ffc7-4ef8-a135-10b033dbde59",
1031-
IPAddress: "10.75.11.20",
1032-
},
1033-
},
1034-
ID: "37ee3b4c-b237-4648-86a5-3fbbf141c30b",
1035-
NetworkID: "ext-net2",
1036-
MACAddress: "fa:16:3e:ea:50:c9",
1037-
},
1038970
{
1039971
Status: "ACTIVE",
1040972
FixedIPs: []ports.IP{
@@ -1047,18 +979,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
1047979
NetworkID: "mycluster-01-vlan701_network",
1048980
MACAddress: "fa:16:3e:10:f0:27",
1049981
},
1050-
{
1051-
Status: "ACTIVE",
1052-
FixedIPs: []ports.IP{
1053-
{
1054-
SubnetID: "1236fcc3-f235-41bc-84f8-464329c1c4c5",
1055-
IPAddress: "172.16.1.241",
1056-
},
1057-
},
1058-
ID: "79f2a2de-ce1b-4424-83d8-a904e732f222",
1059-
NetworkID: "mycluster-01-vlan701_network2",
1060-
MACAddress: "fa:16:3e:10:f0:27",
1061-
},
1062982
{
1063983
Status: "ACTIVE",
1064984
FixedIPs: []ports.IP{
@@ -1075,22 +995,6 @@ func TestNodeAddressesWithSubports(t *testing.T) {
1075995
NetworkID: "mycluster-01-vlan702_network",
1076996
MACAddress: "fa:16:3e:c9:b0:7e",
1077997
},
1078-
{
1079-
Status: "ACTIVE",
1080-
FixedIPs: []ports.IP{
1081-
{
1082-
SubnetID: "fe9826f5-dfb5-4a4c-8a57-3a2963068bb6",
1083-
IPAddress: "172.16.2.36",
1084-
},
1085-
{
1086-
SubnetID: "779c28ee-63cf-4dd8-9c87-b82bfae8e1fb",
1087-
IPAddress: "3000:0:0:1::118",
1088-
},
1089-
},
1090-
ID: "87fac0f8-d46f-4744-940a-7d9526d3cdde",
1091-
NetworkID: "mycluster-01-vlan702_network2",
1092-
MACAddress: "fa:16:3e:c9:b0:7e",
1093-
},
1094998
}
1095999

10961000
networkingOpts := NetworkingOpts{

0 commit comments

Comments
 (0)