kubernetes
/
kops
mirror of https://github.com/kubernetes/kops.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix OpenStack delete functions 

This PR introduces two fixes:
1) Add missing RetryWithBackoff to DeleteInstanceWithID
2) Fix broken retry logic in all other delete functions. In the current implementation, as the first Delete request will almost certainly return nil, the function will return true and the retry will not try again, resulting in assets not getting deleted from OpenStack

Also, the current writeBackoff is pretty aggressive and I introduced a bit less hasty deleteBackoff.

The change has been tested with OpenStack. I verified that all APIs we are hitting will eventually return the 404 (type) we are looking for.
This commit is contained in:
Otto Sulin 2021-02-05 15:41:37 +02:00
parent 5bab92c6e7
commit 24dcd840bb
12 changed files with 95 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
upup/pkg/fi/cloudup/openstack/cloud.go
View File

@ -90,6 +90,14 @@ var writeBackoff = wait.Backoff{
Steps: 5,
}
// deleteBackoff is the backoff strategy for openstack delete retries.
var deleteBackoff = wait.Backoff{
Duration: time.Second,
Factor: 5,
Jitter: 0.1,
Steps: 4,
}
type OpenstackCloud interface {
fi.Cloud

8
upup/pkg/fi/cloudup/openstack/floatingip.go
View File

@ -99,13 +99,15 @@ func (c *openstackCloud) DeleteFloatingIP(id string) (err error) {
}
func deleteFloatingIP(c OpenstackCloud, id string) (err error) {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err = l3floatingip.Delete(c.ComputeClient(), id).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("failed to delete floating ip %s: %v", id, err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if !done && err == nil {
err = wait.ErrWaitTimeout

18
upup/pkg/fi/cloudup/openstack/instance.go
View File

@ -151,7 +151,23 @@ func (c *openstackCloud) DeleteInstanceWithID(instanceID string) error {
}
func deleteInstanceWithID(c OpenstackCloud, instanceID string) error {
return servers.Delete(c.ComputeClient(), instanceID).ExtractErr()
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := servers.Delete(c.ComputeClient(), instanceID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting instance: %s", err)
}
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err
} else if done {
return nil
} else {
return wait.ErrWaitTimeout
}
}
// DetachInstance is not implemented yet. It needs to cause a cloud instance to no longer be counted against the group's size limits.

8
upup/pkg/fi/cloudup/openstack/keypair.go
View File

@ -79,13 +79,15 @@ func (c *openstackCloud) DeleteKeyPair(name string) error {
}
func deleteKeyPair(c OpenstackCloud, name string) error {
done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := keypairs.Delete(c.ComputeClient(), name).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting keypair: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

28
upup/pkg/fi/cloudup/openstack/loadbalancer.go
View File

@ -64,12 +64,15 @@ func deleteMonitor(c OpenstackCloud, monitorID string) error {
if c.LoadBalancerClient() == nil {
return fmt.Errorf("loadbalancer support not available in this deployment")
}
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := monitors.Delete(c.LoadBalancerClient(), monitorID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting pool: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err
@ -89,12 +92,15 @@ func deletePool(c OpenstackCloud, poolID string) error {
return fmt.Errorf("loadbalancer support not available in this deployment")
}
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := v2pools.Delete(c.LoadBalancerClient(), poolID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting pool: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err
@ -114,12 +120,15 @@ func deleteListener(c OpenstackCloud, listenerID string) error {
return fmt.Errorf("loadbalancer support not available in this deployment")
}
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := listeners.Delete(c.LoadBalancerClient(), listenerID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting listener: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err
@ -139,12 +148,15 @@ func deleteLB(c OpenstackCloud, lbID string, opts loadbalancers.DeleteOpts) erro
return fmt.Errorf("loadbalancer support not available in this deployment")
}
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := loadbalancers.Delete(c.LoadBalancerClient(), lbID, opts).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting loadbalancer: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/network.go
View File

@ -220,12 +220,15 @@ func (c *openstackCloud) DeleteNetwork(networkID string) error {
}
func deleteNetwork(c OpenstackCloud, networkID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := networks.Delete(c.NetworkingClient(), networkID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting network: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/port.go
View File

@ -130,12 +130,15 @@ func (c *openstackCloud) DeletePort(portID string) error {
}
func deletePort(c OpenstackCloud, portID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := ports.Delete(c.NetworkingClient(), portID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting port: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

14
upup/pkg/fi/cloudup/openstack/router.go
View File

@ -106,12 +106,15 @@ func (c *openstackCloud) DeleteRouterInterface(routerID string, opt routers.Remo
}
func deleteRouterInterface(c OpenstackCloud, routerID string, opt routers.RemoveInterfaceOptsBuilder) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
_, err := routers.RemoveInterface(c.NetworkingClient(), routerID, opt).Extract()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting router interface: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err
@ -127,12 +130,15 @@ func (c *openstackCloud) DeleteRouter(routerID string) error {
}
func deleteRouter(c OpenstackCloud, routerID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := routers.Delete(c.NetworkingClient(), routerID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting router: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/security_group.go
View File

@ -136,12 +136,15 @@ func (c *openstackCloud) DeleteSecurityGroup(sgID string) error {
}
func deleteSecurityGroup(c OpenstackCloud, sgID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := sg.Delete(c.NetworkingClient(), sgID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting security group: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/server_group.go
View File

@ -164,12 +164,15 @@ func (c *openstackCloud) DeleteServerGroup(groupID string) error {
}
func deleteServerGroup(c OpenstackCloud, groupID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := servergroups.Delete(c.ComputeClient(), groupID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting server group: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/subnet.go
View File

@ -106,12 +106,15 @@ func (c *openstackCloud) DeleteSubnet(subnetID string) error {
}
func deleteSubnet(c OpenstackCloud, subnetID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := subnets.Delete(c.NetworkingClient(), subnetID).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting subnet: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err

7
upup/pkg/fi/cloudup/openstack/volume.go
View File

@ -136,12 +136,15 @@ func (c *openstackCloud) DeleteVolume(volumeID string) error {
}
func deleteVolume(c OpenstackCloud, volumeID string) error {
done, err := vfs.RetryWithBackoff(writeBackoff, func() (bool, error) {
done, err := vfs.RetryWithBackoff(deleteBackoff, func() (bool, error) {
err := cinder.Delete(c.BlockStorageClient(), volumeID, cinder.DeleteOpts{}).ExtractErr()
if err != nil && !isNotFound(err) {
return false, fmt.Errorf("error deleting volume: %v", err)
}
return true, nil
if isNotFound(err) {
return true, nil
}
return false, nil
})
if err != nil {
return err