Skip to content

Commit

Permalink
Merge pull request #2081 from Nordix/mquhuy/ensure-trunkport-deletion
Browse files Browse the repository at this point in the history
🐛 Fix sub-ports not deleted with trunks
  • Loading branch information
k8s-ci-robot authored May 23, 2024
2 parents 067d984 + dabce29 commit 7a19fb6
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 2 deletions.
1 change: 1 addition & 0 deletions controllers/openstackmachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ func Test_reconcileDelete(t *testing.T) {
trunkExtension.Alias = "trunk"
r.network.ListExtensions().Return([]extensions.Extension{trunkExtension}, nil)
r.network.ListTrunk(trunks.ListOpts{PortID: portUUID}).Return([]trunks.Trunk{{ID: trunkUUID}}, nil)
r.network.ListTrunkSubports(trunkUUID).Return([]trunks.Subport{}, nil)
r.network.DeleteTrunk(trunkUUID).Return(nil)
r.network.DeletePort(portUUID).Return(nil)
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/clients/mock/network.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions pkg/clients/networking.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ type NetworkClient interface {
CreateTrunk(opts trunks.CreateOptsBuilder) (*trunks.Trunk, error)
DeleteTrunk(id string) error

ListTrunkSubports(trunkID string) ([]trunks.Subport, error)
RemoveSubports(id string, opts trunks.RemoveSubportsOpts) error

ListRouter(opts routers.ListOpts) ([]routers.Router, error)
CreateRouter(opts routers.CreateOptsBuilder) (*routers.Router, error)
DeleteRouter(id string) error
Expand Down Expand Up @@ -238,6 +241,24 @@ func (c networkClient) DeleteTrunk(id string) error {
return mc.ObserveRequestIgnoreNotFound(trunks.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) ListTrunkSubports(trunkID string) ([]trunks.Subport, error) {
mc := metrics.NewMetricPrometheusContext("trunk", "listsubports")
subports, err := trunks.GetSubports(c.serviceClient, trunkID).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return subports, nil
}

func (c networkClient) RemoveSubports(id string, opts trunks.RemoveSubportsOpts) error {
mc := metrics.NewMetricPrometheusContext("trunk", "deletesubports")
_, err := trunks.RemoveSubports(c.serviceClient, id, opts).Extract()
if mc.ObserveRequest(err) != nil {
return err
}
return nil
}

func (c networkClient) ListTrunk(opts trunks.ListOptsBuilder) ([]trunks.Trunk, error) {
mc := metrics.NewMetricPrometheusContext("trunk", "list")
allPages, err := trunks.List(c.serviceClient, opts).AllPages()
Expand Down
57 changes: 55 additions & 2 deletions pkg/cloud/services/networking/trunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ import (
)

const (
timeoutTrunkDelete = 3 * time.Minute
retryIntervalTrunkDelete = 5 * time.Second
timeoutTrunkDelete = 3 * time.Minute
retryIntervalTrunkDelete = 5 * time.Second
retryIntervalSubportDelete = 30 * time.Second
)

func (s *Service) GetTrunkSupport() (bool, error) {
Expand Down Expand Up @@ -77,6 +78,42 @@ func (s *Service) getOrCreateTrunkForPort(eventObject runtime.Object, port *port
return trunk, nil
}

func (s *Service) RemoveTrunkSubports(trunkID string) error {
subports, err := s.client.ListTrunkSubports(trunkID)
if err != nil {
return err
}

if len(subports) == 0 {
return nil
}

portList := make([]trunks.RemoveSubport, len(subports))
for i, subport := range subports {
portList[i] = trunks.RemoveSubport{
PortID: subport.PortID,
}
}

removeSubportsOpts := trunks.RemoveSubportsOpts{
Subports: portList,
}

err = s.client.RemoveSubports(trunkID, removeSubportsOpts)
if err != nil {
return err
}

for _, subPort := range subports {
err := s.client.DeletePort(subPort.PortID)
if err != nil {
return err
}
}

return nil
}

func (s *Service) DeleteTrunk(eventObject runtime.Object, portID string) error {
listOpts := trunks.ListOpts{
PortID: portID,
Expand All @@ -88,6 +125,22 @@ func (s *Service) DeleteTrunk(eventObject runtime.Object, portID string) error {
if len(trunkInfo) != 1 {
return nil
}
// Delete sub-ports if trunk is associated with sub-ports
err = wait.PollUntilContextTimeout(context.TODO(), retryIntervalSubportDelete, timeoutTrunkDelete, true, func(_ context.Context) (bool, error) {
if err := s.RemoveTrunkSubports(trunkInfo[0].ID); err != nil {
if capoerrors.IsNotFound(err) || capoerrors.IsConflict(err) || capoerrors.IsRetryable(err) {
return false, nil
}
return false, err
}
return true, nil
})
if err != nil {
record.Warnf(eventObject, "FailedRemoveTrunkSubports", "Failed to delete sub ports trunk %s with id %s: %v", trunkInfo[0].Name, trunkInfo[0].ID, err)
return err
}

record.Eventf(eventObject, "SuccessfulRemoveTrunkSubports", "Removed trunk sub ports %s with id %s", trunkInfo[0].Name, trunkInfo[0].ID)

err = wait.PollUntilContextTimeout(context.TODO(), retryIntervalTrunkDelete, timeoutTrunkDelete, true, func(_ context.Context) (bool, error) {
if err := s.client.DeleteTrunk(trunkInfo[0].ID); err != nil {
Expand Down

0 comments on commit 7a19fb6

Please sign in to comment.