✨ Delete router/network/subnet

[hidekazuna]

What this PR does / why we need it:

This PR adds to delete router, network, and subnet feature. Currently how to delete cluster is not provided explicitly in my understanding, but we need to delete OpenStack resources easily even for testing. By this PR, we can delete OpenStack resources created with cluster-template-without-lb.yaml by the following.

kubectl delete machinedeployment capi-quickstart-md-0 -n capi-quickstart
kubectl delete kubeadmcontrolplane capi-quickstart-control-plane -n capi-quickstart
kubectl delete cluster capi-quickstart -n capi-quickstart

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

[hidekazuna]

/assign @sbueringer
/assign @jichenjc
/assign @prankul88

[sbueringer]

@hidekazuna

looks good to me. I'll test it later. Don't we have to delete the subnet?

[hidekazuna]

@sbueringer When we delete a network, The subnet is deleted as well.

[sbueringer]

@sbueringer When we delete a network, The subnet is deleted as well.

okay thx good to know

[jichenjc]

I don't remember clearly, do we have a chance that the network/router is not created by CPAO?
others LGTM

[sbueringer]

I think it's okay as the cluster network/subnet are afaik always created by us

[jichenjc]

ok, I didn't remember clearly, will try this later on and submit PR if need :)

[sbueringer]

@hidekazuna two small nits, apart from that everything worked :)

[hidekazuna]

@sbueringer Thanks, fixed.

[sbueringer]

I think a guard close would be more idiomatic in go, i.e.:

if !exist {
    s.logger.Info("No router", "router", network.Router.ID)
    return nil
}
if network.Subnet == nil || network.Subnet.ID == "" {
    s.logger.V(4).Info("No need to remove router interface since no subnet exists.")
    return nil
}
_, err := routers.RemoveInterface(s.client, network.Router.ID, routers.RemoveInterfaceOpts{
    SubnetID: network.Subnet.ID,
}).Extract()
if err != nil {
    return fmt.Errorf("unable to remove router interface: %v", err)
}
s.logger.V(4).Info("Removed RouterInterface of Router", "id", network.Router.ID)

err = routers.Delete(s.client, network.Router.ID).ExtractErr(

(same for DeleteNetwork)

[sbueringer]

I think you still have to check for empty routerID as in you're previous commit. The problem is if the ID is an empty string
and you list all routers in existsRouter without an ID you get all existing routers (same for network)

EDIT: Just looked it up. I would suggest using routers.Get (& networks.Get) instead. Should be even save with empty ID and just returns 0 or 1 router not all so should be more efficient. Although I would still check for the empty string before and then checking for the existence with get

[sbueringer]

when you're already on it :). You can inline this to

if err := networkingService.DeleteRouter(openStackCluster.Status.Network); err != nil {

[hidekazuna]

@sbueringer Fixed.

[sbueringer]

last one :)

[sbueringer]

/lgtm

[sbueringer]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hidekazuna, sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment