Skip to content

Commit 64bee2e

Browse files
alvaroalemankubermatic-bot
alvaroaleman
authored and
kubermatic-bot
committed
Release Openstack Floating IP on deletion if it was newly allocated (#439)
* Add finalizer to machine when associating a new floating ip * Release floating ip on Cleanup * Store floating ip id for cleanup in annotation * Improve error wording * Fix linting * Simplify cleanup * Simplify floating ip id reading * PR feedback * Error out if cloud provider env creation failed
1 parent a2ea740 commit 64bee2e

File tree

3 files changed

+138
-67
lines changed

3 files changed

+138
-67
lines changed

‎hack/ci-e2e-test.sh‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,12 @@ for try in {1..20}; do
5151
echo "Creating environment at cloud provider."
5252
terraform import hcloud_ssh_key.default 265119
5353
terraform apply -auto-approve
54-
if [[ $? == 0 ]]; then break; fi
54+
TF_RC=$?
55+
if [[ $TF_RC == 0 ]]; then break; fi
56+
if [[ $TF_RC != 0 ]] && [[ $try -eq 20 ]]; then
57+
echo "Creating cloud provider env failed!"
58+
exit 1
59+
fi
5560
echo "Sleeping for $try seconds"
5661
sleep ${try}s
5762
done

‎pkg/cloudprovider/provider/openstack/helper.go‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"sync"
77
"time"
88

9-
"github.com/golang/glog"
10-
119
"github.com/gophercloud/gophercloud"
1210
goopenstack "github.com/gophercloud/gophercloud/openstack"
1311
osavailabilityzones "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones"
@@ -328,66 +326,6 @@ func getFreeFloatingIPs(client *gophercloud.ProviderClient, region string, float
328326
return freeFIPs, nil
329327
}
330328

331-
func assignFloatingIPToInstance(client *gophercloud.ProviderClient, instanceID, floatingIPPoolName, region string, network *osnetworks.Network) error {
332-
port, err := getInstancePort(client, region, instanceID, network.ID)
333-
if err != nil {
334-
return fmt.Errorf("failed to get instance port for network %s in region %s: %v", network.ID, region, err)
335-
}
336-
337-
netClient, err := goopenstack.NewNetworkV2(client, gophercloud.EndpointOpts{Region: region})
338-
if err != nil {
339-
return fmt.Errorf("failed to create the networkv2 client for region %s: %v", region, err)
340-
}
341-
342-
floatingIPPool, err := getNetwork(client, region, floatingIPPoolName)
343-
if err != nil {
344-
return osErrorToTerminalError(err, fmt.Sprintf("failed to get floating ip pool %q", floatingIPPoolName))
345-
}
346-
347-
// We're only interested in the part which is vulnerable to concurrent access
348-
started := time.Now()
349-
glog.V(2).Infof("Assigning a floating IP to instance %s", instanceID)
350-
351-
floatingIPAssignLock.Lock()
352-
defer floatingIPAssignLock.Unlock()
353-
354-
freeFloatingIps, err := getFreeFloatingIPs(client, region, floatingIPPool)
355-
if err != nil {
356-
return osErrorToTerminalError(err, "failed to get free floating ips")
357-
}
358-
359-
var ip *osfloatingips.FloatingIP
360-
if len(freeFloatingIps) < 1 {
361-
if ip, err = createFloatingIP(client, region, port.ID, floatingIPPool); err != nil {
362-
return osErrorToTerminalError(err, "failed to allocate a floating ip")
363-
}
364-
} else {
365-
freeIP := freeFloatingIps[0]
366-
ip, err = osfloatingips.Update(netClient, freeIP.ID, osfloatingips.UpdateOpts{
367-
PortID: &port.ID,
368-
}).Extract()
369-
if err != nil {
370-
return fmt.Errorf("failed to update FloatingIP %s(%s): %v", freeIP.ID, freeIP.FloatingIP, err)
371-
}
372-
373-
// We're now going to wait 3 seconds and check if the IP is still ours. If not, we're going to fail
374-
// On our reference system it took ~3 seconds for a full FloatingIP allocation (Including creating a new one). It took ~600ms just for assigning one.
375-
time.Sleep(floatingReassignIPCheckPeriod)
376-
currentIP, err := osfloatingips.Get(netClient, ip.ID).Extract()
377-
if err != nil {
378-
return fmt.Errorf("failed to load FloatingIP %s after assignment has been done: %v", ip.FloatingIP, err)
379-
}
380-
// Verify if the port is still the one we set it to
381-
if currentIP.PortID != port.ID {
382-
return fmt.Errorf("floatingIP %s got reassigned", currentIP.FloatingIP)
383-
}
384-
}
385-
secondsTook := time.Since(started).Seconds()
386-
387-
glog.V(2).Infof("Successfully assigned the FloatingIP %s to instance %s. Took %f seconds(without the recheck wait period %f seconds). ", ip.FloatingIP, instanceID, secondsTook, floatingReassignIPCheckPeriod.Seconds())
388-
return nil
389-
}
390-
391329
func createFloatingIP(client *gophercloud.ProviderClient, region, portID string, floatingIPPool *osnetworks.Network) (*osfloatingips.FloatingIP, error) {
392330
netClient, err := goopenstack.NewNetworkV2(client, gophercloud.EndpointOpts{Region: region})
393331
if err != nil {

‎pkg/cloudprovider/provider/openstack/provider.go‎

Lines changed: 132 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
osextendedstatus "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/extendedstatus"
1515
"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/keypairs"
1616
osservers "github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
17+
osfloatingips "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
18+
osnetworks "github.com/gophercloud/gophercloud/openstack/networking/v2/networks"
1719
"github.com/gophercloud/gophercloud/pagination"
1820

1921
"github.com/kubermatic/machine-controller/pkg/cloudprovider/cloud"
@@ -24,12 +26,18 @@ import (
2426
"k8s.io/apimachinery/pkg/runtime"
2527
"k8s.io/apimachinery/pkg/types"
2628
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
29+
"k8s.io/apimachinery/pkg/util/sets"
2730
"k8s.io/apimachinery/pkg/util/wait"
2831

2932
"sigs.k8s.io/cluster-api/pkg/apis/cluster/common"
3033
"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
3134
)
3235

36+
const (
37+
floatingIPReleaseFinalizer = "kubermatic.io/release-openstack-floating-ip"
38+
floatingIPIDAnnotationKey = "kubermatic.io/release-openstack-floating-ip"
39+
)
40+
3341
type provider struct {
3442
configVarResolver *providerconfig.ConfigVarResolver
3543
}
@@ -349,7 +357,7 @@ func (p *provider) Validate(spec v1alpha1.MachineSpec) error {
349357
return nil
350358
}
351359

352-
func (p *provider) Create(machine *v1alpha1.Machine, _ *cloud.MachineCreateDeleteData, userdata string) (instance.Instance, error) {
360+
func (p *provider) Create(machine *v1alpha1.Machine, machineCreateDeleteData *cloud.MachineCreateDeleteData, userdata string) (instance.Instance, error) {
353361
c, _, _, err := p.getConfig(machine.Spec.ProviderConfig)
354362
if err != nil {
355363
return nil, cloudprovidererrors.TerminalError{
@@ -423,7 +431,7 @@ func (p *provider) Create(machine *v1alpha1.Machine, _ *cloud.MachineCreateDelet
423431

424432
// Find a free FloatingIP or allocate a new one
425433
if c.FloatingIPPool != "" {
426-
if err := assignFloatingIPToInstance(client, server.ID, c.FloatingIPPool, c.Region, network); err != nil {
434+
if err := assignFloatingIPToInstance(machineCreateDeleteData.Updater, machine, client, server.ID, c.FloatingIPPool, c.Region, network); err != nil {
427435
defer deleteInstanceDueToFatalLogged(computeClient, server.ID)
428436
return nil, fmt.Errorf("failed to assign a floating ip to instance %s: %v", server.ID, err)
429437
}
@@ -475,10 +483,20 @@ func deleteInstanceDueToFatalLogged(computeClient *gophercloud.ServiceClient, se
475483
glog.V(0).Infof("Instance %s got deleted", serverID)
476484
}
477485

478-
func (p *provider) Cleanup(machine *v1alpha1.Machine, _ *cloud.MachineCreateDeleteData) (bool, error) {
486+
func (p *provider) Cleanup(machine *v1alpha1.Machine, machineCreateDeleteData *cloud.MachineCreateDeleteData) (bool, error) {
487+
var hasFloatingIPReleaseFinalizer bool
488+
if finalizers := sets.NewString(machine.Finalizers...); finalizers.Has(floatingIPReleaseFinalizer) {
489+
hasFloatingIPReleaseFinalizer = true
490+
}
491+
479492
instance, err := p.Get(machine)
480493
if err != nil {
481494
if err == cloudprovidererrors.ErrInstanceNotFound {
495+
if hasFloatingIPReleaseFinalizer {
496+
if err := p.cleanupFloatingIP(machine, machineCreateDeleteData.Updater); err != nil {
497+
return false, fmt.Errorf("failed to clean up floating ip: %v", err)
498+
}
499+
}
482500
return true, nil
483501
}
484502
return false, err
@@ -502,10 +520,14 @@ func (p *provider) Cleanup(machine *v1alpha1.Machine, _ *cloud.MachineCreateDele
502520
return false, osErrorToTerminalError(err, "failed to get compute client")
503521
}
504522

505-
if err := osservers.Delete(computeClient, instance.ID()).ExtractErr(); err != nil {
523+
if err := osservers.Delete(computeClient, instance.ID()).ExtractErr(); err != nil && err.Error() != "Resource not found" {
506524
return false, osErrorToTerminalError(err, "failed to delete instance")
507525
}
508526

527+
if hasFloatingIPReleaseFinalizer {
528+
return false, p.cleanupFloatingIP(machine, machineCreateDeleteData.Updater)
529+
}
530+
509531
return false, nil
510532
}
511533

@@ -733,3 +755,109 @@ type forbiddenResponse struct {
733755
Code int `json:"code"`
734756
} `json:"forbidden"`
735757
}
758+
759+
func (p *provider) cleanupFloatingIP(machine *v1alpha1.Machine, updater cloud.MachineUpdater) error {
760+
floatingIPID, exists := machine.Annotations[floatingIPIDAnnotationKey]
761+
if !exists {
762+
return osErrorToTerminalError(fmt.Errorf("failed to release floating ip"),
763+
fmt.Sprintf("%s finalizer exists but %s annotation does not", floatingIPReleaseFinalizer, floatingIPIDAnnotationKey))
764+
}
765+
766+
c, _, _, err := p.getConfig(machine.Spec.ProviderConfig)
767+
if err != nil {
768+
return cloudprovidererrors.TerminalError{
769+
Reason: common.InvalidConfigurationMachineError,
770+
Message: fmt.Sprintf("Failed to parse MachineSpec, due to %v", err),
771+
}
772+
}
773+
774+
client, err := getClient(c)
775+
if err != nil {
776+
return osErrorToTerminalError(err, "failed to get a openstack client")
777+
}
778+
netClient, err := goopenstack.NewNetworkV2(client, gophercloud.EndpointOpts{Region: c.Region})
779+
if err != nil {
780+
return fmt.Errorf("failed to create the networkv2 client for region %s: %v", c.Region, err)
781+
}
782+
if err := osfloatingips.Delete(netClient, floatingIPID).ExtractErr(); err != nil && err.Error() != "Resource not found" {
783+
return fmt.Errorf("failed to delete floating ip %s: %v", floatingIPID, err)
784+
}
785+
if _, err := updater(machine, func(m *v1alpha1.Machine) {
786+
finalizers := sets.NewString(m.Finalizers...)
787+
finalizers.Delete(floatingIPReleaseFinalizer)
788+
m.Finalizers = finalizers.List()
789+
}); err != nil {
790+
return fmt.Errorf("failed to delete %s finalizer from Machine: %v", floatingIPReleaseFinalizer, err)
791+
}
792+
793+
return nil
794+
}
795+
796+
func assignFloatingIPToInstance(machineUpdater cloud.MachineUpdater, machine *v1alpha1.Machine, client *gophercloud.ProviderClient, instanceID, floatingIPPoolName, region string, network *osnetworks.Network) error {
797+
port, err := getInstancePort(client, region, instanceID, network.ID)
798+
if err != nil {
799+
return fmt.Errorf("failed to get instance port for network %s in region %s: %v", network.ID, region, err)
800+
}
801+
802+
netClient, err := goopenstack.NewNetworkV2(client, gophercloud.EndpointOpts{Region: region})
803+
if err != nil {
804+
return fmt.Errorf("failed to create the networkv2 client for region %s: %v", region, err)
805+
}
806+
807+
floatingIPPool, err := getNetwork(client, region, floatingIPPoolName)
808+
if err != nil {
809+
return osErrorToTerminalError(err, fmt.Sprintf("failed to get floating ip pool %q", floatingIPPoolName))
810+
}
811+
812+
// We're only interested in the part which is vulnerable to concurrent access
813+
started := time.Now()
814+
glog.V(2).Infof("Assigning a floating IP to instance %s", instanceID)
815+
816+
floatingIPAssignLock.Lock()
817+
defer floatingIPAssignLock.Unlock()
818+
819+
freeFloatingIps, err := getFreeFloatingIPs(client, region, floatingIPPool)
820+
if err != nil {
821+
return osErrorToTerminalError(err, "failed to get free floating ips")
822+
}
823+
824+
var ip *osfloatingips.FloatingIP
825+
if len(freeFloatingIps) < 1 {
826+
if ip, err = createFloatingIP(client, region, port.ID, floatingIPPool); err != nil {
827+
return osErrorToTerminalError(err, "failed to allocate a floating ip")
828+
}
829+
if _, err = machineUpdater(machine, func(m *v1alpha1.Machine) {
830+
m.Finalizers = append(m.Finalizers, floatingIPReleaseFinalizer)
831+
if m.Annotations == nil {
832+
m.Annotations = map[string]string{}
833+
}
834+
m.Annotations[floatingIPIDAnnotationKey] = ip.ID
835+
}); err != nil {
836+
return fmt.Errorf("failed to add floating ip release finalizer after allocating floating ip: %v", err)
837+
}
838+
} else {
839+
freeIP := freeFloatingIps[0]
840+
ip, err = osfloatingips.Update(netClient, freeIP.ID, osfloatingips.UpdateOpts{
841+
PortID: &port.ID,
842+
}).Extract()
843+
if err != nil {
844+
return fmt.Errorf("failed to update FloatingIP %s(%s): %v", freeIP.ID, freeIP.FloatingIP, err)
845+
}
846+
847+
// We're now going to wait 3 seconds and check if the IP is still ours. If not, we're going to fail
848+
// On our reference system it took ~3 seconds for a full FloatingIP allocation (Including creating a new one). It took ~600ms just for assigning one.
849+
time.Sleep(floatingReassignIPCheckPeriod)
850+
currentIP, err := osfloatingips.Get(netClient, ip.ID).Extract()
851+
if err != nil {
852+
return fmt.Errorf("failed to load FloatingIP %s after assignment has been done: %v", ip.FloatingIP, err)
853+
}
854+
// Verify if the port is still the one we set it to
855+
if currentIP.PortID != port.ID {
856+
return fmt.Errorf("floatingIP %s got reassigned", currentIP.FloatingIP)
857+
}
858+
}
859+
secondsTook := time.Since(started).Seconds()
860+
861+
glog.V(2).Infof("Successfully assigned the FloatingIP %s to instance %s. Took %f seconds(without the recheck wait period %f seconds). ", ip.FloatingIP, instanceID, secondsTook, floatingReassignIPCheckPeriod.Seconds())
862+
return nil
863+
}

0 commit comments

Comments
 (0)