Skip to content

Commit

Permalink
Fix port fetch should check for missing trunk
Browse files Browse the repository at this point in the history 
Signed-off-by: Huy Mai <[email protected]>
  • Loading branch information
@mquhuy
mquhuy committed Nov 16, 2024
1 parent 80473c6 commit a0063ed
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 21 deletions.
14 changes: 13 additions & 1 deletion pkg/cloud/services/compute/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ func TestService_ReconcileInstance(t *testing.T) {
Description: portOpts["description"].(string),
}, nil
})
networkRecorder.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
}

// Expected calls if we delete the network port
Expand Down Expand Up @@ -460,6 +459,7 @@ func TestService_ReconcileInstance(t *testing.T) {
expectDefaultImageAndFlavor(r.compute, r.image)

expectCreateServer(r.compute, getDefaultServerMap(), false)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectServerPollSuccess(r.compute)
},
wantErr: false,
Expand All @@ -472,6 +472,7 @@ func TestService_ReconcileInstance(t *testing.T) {
expectDefaultImageAndFlavor(r.compute, r.image)

expectCreateServer(r.compute, getDefaultServerMap(), true)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)

// Make sure we delete ports
expectCleanupDefaultPort(r.network)
Expand All @@ -491,6 +492,7 @@ func TestService_ReconcileInstance(t *testing.T) {
expect: func(r *recorders) {
expectDefaultImageAndFlavor(r.compute, r.image)
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)

// Looking up the second port fails
r.network.ListPort(ports.ListOpts{
Expand All @@ -511,6 +513,7 @@ func TestService_ReconcileInstance(t *testing.T) {
expectDefaultImageAndFlavor(r.compute, r.image)

expectCreateServer(r.compute, getDefaultServerMap(), false)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectServerPoll(r.compute, []string{"BUILDING", "ACTIVE"})
},
wantErr: false,
Expand All @@ -523,6 +526,7 @@ func TestService_ReconcileInstance(t *testing.T) {
expectDefaultImageAndFlavor(r.compute, r.image)

expectCreateServer(r.compute, getDefaultServerMap(), false)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectServerPoll(r.compute, []string{"BUILDING", "ERROR"})

// Don't delete ports because the server is created: DeleteInstance will do it
Expand Down Expand Up @@ -567,6 +571,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
}
expectCreateServer(r.compute, createMap, false)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectServerPollSuccess(r.compute)

// Don't delete ports because the server is created: DeleteInstance will do it
Expand Down Expand Up @@ -614,6 +619,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
}
expectCreateServer(r.compute, createMap, false)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectServerPollSuccess(r.compute)

// Don't delete ports because the server is created: DeleteInstance will do it
Expand All @@ -631,6 +637,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectDefaultImageAndFlavor(r.compute, r.image)

r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}).
Expand Down Expand Up @@ -679,6 +686,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectDefaultImageAndFlavor(r.compute, r.image)

r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-root", openStackMachineName)}).
Expand Down Expand Up @@ -767,6 +775,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectDefaultImageAndFlavor(r.compute, r.image)

r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}).
Expand Down Expand Up @@ -836,6 +845,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectDefaultImageAndFlavor(r.compute, r.image)

r.volume.ListVolumes(volumes.ListOpts{Name: fmt.Sprintf("%s-etcd", openStackMachineName)}).
Expand Down Expand Up @@ -893,6 +903,7 @@ func TestService_ReconcileInstance(t *testing.T) {
},
expect: func(r *recorders) {
expectUseExistingDefaultPort(r.network)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)
expectDefaultImageAndFlavor(r.compute, r.image)

// Make sure we delete ports
Expand All @@ -918,6 +929,7 @@ func TestService_ReconcileInstance(t *testing.T) {
r.network.ListExtensions().Return(extensions, nil)

expectCreatePort(r.network, portName, networkUUID)
r.network.ReplaceAllAttributesTags("ports", portUUID, attributestags.ReplaceAllOpts{Tags: []string{"test-tag"}}).Return(nil, nil)

// Check for existing trunk
r.network.ListTrunk(newGomegaMockMatcher(
Expand Down
53 changes: 33 additions & 20 deletions pkg/cloud/services/networking/port.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,32 @@ func (s *Service) GetPortFromInstanceIP(instanceID string, ip string) ([]ports.P
return s.client.ListPort(portOpts)
}

// ensurePortTagsAndTrunk tags the specified port and checks if a trunk is needed.
// If required, it creates and tags the trunk.
func (s *Service) ensurePortTagsAndTrunk(eventObject runtime.Object, clusterName string, port *ports.Port, portOpts *infrav1.PortOpts, instanceTags []string) error {
var tags []string
tags = append(tags, instanceTags...)
tags = append(tags, portOpts.Tags...)
if len(tags) > 0 {
if err := s.replaceAllAttributesTags(eventObject, portResource, port.ID, tags); err != nil {
record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace port tags %s: %v", port.Name, err)
return err
}
}
if portOpts.Trunk != nil && *portOpts.Trunk {
trunk, err := s.getOrCreateTrunk(eventObject, clusterName, port.Name, port.ID)
if err != nil {
record.Warnf(eventObject, "FailedCreateTrunk", "Failed to create trunk for port %s: %v", port.Name, err)
return err
}
if err = s.replaceAllAttributesTags(eventObject, trunkResource, trunk.ID, tags); err != nil {
record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", port.Name, err)
return err
}
}
return nil
}

func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string, portName string, portOpts *infrav1.PortOpts, instanceSecurityGroups []string, instanceTags []string) (*ports.Port, error) {
networkID := portOpts.Network.ID

Expand All @@ -66,7 +92,11 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
}

if len(existingPorts) == 1 {
return &existingPorts[0], nil
port := &existingPorts[0]
if err := s.ensurePortTagsAndTrunk(eventObject, clusterName, port, portOpts, instanceTags); err != nil {
return nil, err
}
return port, nil
}

if len(existingPorts) > 1 {
Expand Down Expand Up @@ -166,27 +196,10 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
return nil, err
}

var tags []string
tags = append(tags, instanceTags...)
tags = append(tags, portOpts.Tags...)
if len(tags) > 0 {
if err = s.replaceAllAttributesTags(eventObject, portResource, port.ID, tags); err != nil {
record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace port tags %s: %v", portName, err)
return nil, err
}
if err := s.ensurePortTagsAndTrunk(eventObject, clusterName, port, portOpts, instanceTags); err != nil {
return nil, err
}
record.Eventf(eventObject, "SuccessfulCreatePort", "Created port %s with id %s", port.Name, port.ID)
if portOpts.Trunk != nil && *portOpts.Trunk {
trunk, err := s.getOrCreateTrunk(eventObject, clusterName, port.Name, port.ID)
if err != nil {
record.Warnf(eventObject, "FailedCreateTrunk", "Failed to create trunk for port %s: %v", portName, err)
return nil, err
}
if err = s.replaceAllAttributesTags(eventObject, trunkResource, trunk.ID, tags); err != nil {
record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", portName, err)
return nil, err
}
}

return port, nil
}
Expand Down

0 comments on commit a0063ed

Please sign in to comment.