Skip to content

Commit 31be347

Browse files
committed
Use PortOpts instead of Network in createInstance()
This is a non-functional refactor, as PortOpts was the only field of Network previously in use apart from ID (the network ID), which is guaranteed to be set by the prior call to normalizePorts().
1 parent 41622df commit 31be347

File tree

3 files changed

+107
-120
lines changed

3 files changed

+107
-120
lines changed

pkg/cloud/services/compute/instance.go

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -171,42 +171,34 @@ func (s *Service) normalizePorts(ports []infrav1.PortOpts, openStackCluster *inf
171171
return normalizedPorts, nil
172172
}
173173

174-
// constructNetworks builds an array of networks from the network, subnet and ports items in the instance spec.
175-
// If no networks or ports are in the spec, returns a single network item for a network connection to the default cluster network.
176-
func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster, instanceSpec *InstanceSpec) ([]infrav1.Network, error) {
174+
// constructPorts builds an array of ports from the instance spec.
175+
// If no ports are in the spec, returns a single port for a network connection to the default cluster network.
176+
func (s *Service) constructPorts(openStackCluster *infrav1.OpenStackCluster, instanceSpec *InstanceSpec) ([]infrav1.PortOpts, error) {
177177
// Ensure user-specified ports have all required fields
178178
ports, err := s.normalizePorts(instanceSpec.Ports, openStackCluster, instanceSpec)
179179
if err != nil {
180180
return nil, err
181181
}
182-
nets := make([]infrav1.Network, 0, len(ports))
183-
for i := range ports {
184-
port := &ports[i]
185-
nets = append(nets, infrav1.Network{
186-
ID: port.Network.ID,
187-
Subnet: &infrav1.Subnet{},
188-
PortOpts: port,
189-
})
190-
}
191182

192183
// no networks or ports found in the spec, so create a port on the cluster network
193-
if len(nets) == 0 {
194-
nets = []infrav1.Network{{
195-
ID: openStackCluster.Status.Network.ID,
196-
Subnet: &infrav1.Subnet{
197-
ID: openStackCluster.Status.Network.Subnet.ID,
198-
},
199-
PortOpts: &infrav1.PortOpts{
200-
Trunk: &instanceSpec.Trunk,
184+
if len(ports) == 0 {
185+
ports = []infrav1.PortOpts{{
186+
Network: &infrav1.NetworkFilter{
187+
ID: openStackCluster.Status.Network.ID,
201188
},
189+
FixedIPs: []infrav1.FixedIP{{
190+
Subnet: &infrav1.SubnetFilter{
191+
ID: openStackCluster.Status.Network.Subnet.ID,
192+
},
193+
}},
194+
Trunk: &instanceSpec.Trunk,
202195
}}
203196
}
204197

205198
// trunk support is required if any port has trunk enabled
206199
portUsesTrunk := func() bool {
207-
for _, net := range nets {
208-
port := net.PortOpts
209-
if port != nil && port.Trunk != nil && *port.Trunk {
200+
for _, port := range ports {
201+
if port.Trunk != nil && *port.Trunk {
210202
return true
211203
}
212204
}
@@ -222,7 +214,7 @@ func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster,
222214
}
223215
}
224216

225-
return nets, nil
217+
return ports, nil
226218
}
227219

228220
func (s *Service) CreateInstance(eventObject runtime.Object, openStackCluster *infrav1.OpenStackCluster, instanceSpec *InstanceSpec, clusterName string, isBastion bool) (*InstanceStatus, error) {
@@ -266,7 +258,7 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste
266258
}
267259
}()
268260

269-
nets, err := s.constructNetworks(openStackCluster, instanceSpec)
261+
ports, err := s.constructPorts(openStackCluster, instanceSpec)
270262
if err != nil {
271263
return nil, err
272264
}
@@ -281,16 +273,14 @@ func (s *Service) createInstanceImpl(eventObject runtime.Object, openStackCluste
281273
return nil, fmt.Errorf("error getting security groups: %v", err)
282274
}
283275

284-
for i, network := range nets {
285-
if network.ID == "" {
286-
return nil, fmt.Errorf("no network was found or provided. Please check your machine configuration and try again")
287-
}
276+
for i := range ports {
277+
portOpts := &ports[i]
288278
iTags := []string{}
289279
if len(instanceSpec.Tags) > 0 {
290280
iTags = instanceSpec.Tags
291281
}
292-
portName := getPortName(instanceSpec.Name, network.PortOpts, i)
293-
port, err := networkingService.GetOrCreatePort(eventObject, clusterName, portName, network, securityGroups, iTags)
282+
portName := getPortName(instanceSpec.Name, portOpts, i)
283+
port, err := networkingService.GetOrCreatePort(eventObject, clusterName, portName, portOpts, securityGroups, iTags)
294284
if err != nil {
295285
return nil, err
296286
}

pkg/cloud/services/networking/port.go

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,12 @@ func (s *Service) GetPortFromInstanceIP(instanceID string, ip string) ([]ports.P
5353
return s.client.ListPort(portOpts)
5454
}
5555

56-
func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string, portName string, net infrav1.Network, instanceSecurityGroups []string, instanceTags []string) (*ports.Port, error) {
56+
func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string, portName string, portOpts *infrav1.PortOpts, instanceSecurityGroups []string, instanceTags []string) (*ports.Port, error) {
57+
networkID := portOpts.Network.ID
58+
5759
existingPorts, err := s.client.ListPort(ports.ListOpts{
5860
Name: portName,
59-
NetworkID: net.ID,
61+
NetworkID: networkID,
6062
})
6163
if err != nil {
6264
return nil, fmt.Errorf("searching for existing port for server: %v", err)
@@ -70,12 +72,6 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
7072
return nil, fmt.Errorf("multiple ports found with name \"%s\"", portName)
7173
}
7274

73-
// no port found, so create the port
74-
portOpts := net.PortOpts
75-
if portOpts == nil {
76-
portOpts = &infrav1.PortOpts{}
77-
}
78-
7975
description := portOpts.Description
8076
if description == "" {
8177
description = names.GetDescription(clusterName)
@@ -106,7 +102,7 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
106102
if len(portOpts.FixedIPs) > 0 {
107103
fips := make([]ports.IP, 0, len(portOpts.FixedIPs)+1)
108104
for _, fixedIP := range portOpts.FixedIPs {
109-
subnetID, err := s.getSubnetIDForFixedIP(fixedIP.Subnet, net.ID)
105+
subnetID, err := s.getSubnetIDForFixedIP(fixedIP.Subnet, networkID)
110106
if err != nil {
111107
return nil, err
112108
}
@@ -115,9 +111,6 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
115111
IPAddress: fixedIP.IPAddress,
116112
})
117113
}
118-
if net.Subnet.ID != "" {
119-
fips = append(fips, ports.IP{SubnetID: net.Subnet.ID})
120-
}
121114
fixedIPs = fips
122115
}
123116

@@ -140,7 +133,7 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
140133

141134
createOpts = ports.CreateOpts{
142135
Name: portName,
143-
NetworkID: net.ID,
136+
NetworkID: networkID,
144137
Description: description,
145138
AdminStateUp: portOpts.AdminStateUp,
146139
MACAddress: portOpts.MACAddress,

0 commit comments

Comments
 (0)