From 65c23ea0e7e5026686cf75f01e6a9ca71b62f8a6 Mon Sep 17 00:00:00 2001 From: Sebastien LAWNICZAK Date: Wed, 12 Aug 2015 18:01:35 +0200 Subject: [PATCH 1/3] Add floatingIP allocation for LB creation on openstack --- .../providers/openstack/openstack.go | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 47ca3a16536..40ca7c855f4 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -30,6 +30,7 @@ import ( "github.com/rackspace/gophercloud/openstack" "github.com/rackspace/gophercloud/openstack/compute/v2/flavors" "github.com/rackspace/gophercloud/openstack/compute/v2/servers" + "github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" "github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/members" "github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/monitors" "github.com/rackspace/gophercloud/openstack/networking/v2/extensions/lbaas/pools" @@ -70,6 +71,7 @@ func (d *MyDuration) UnmarshalText(text []byte) error { type LoadBalancerOpts struct { SubnetId string `gcfg:"subnet-id"` // required + FloatingNetworkId string `gcfg:"floating-network-id"` LBMethod string `gfcg:"lb-method"` CreateMonitor bool `gcfg:"create-monitor"` MonitorDelay MyDuration `gcfg:"monitor-delay"` @@ -527,6 +529,41 @@ func getVipByName(client *gophercloud.ServiceClient, name string) (*vips.Virtual return &vipList[0], nil } +func getFloatingIPByPortID(client *gophercloud.ServiceClient, portID string) (*floatingips.FloatingIP, error) { + opts := floatingips.ListOpts{ + PortID: portID, + } + pager := floatingips.List(client, opts) + + floatingIPList := make([]floatingips.FloatingIP, 0, 1) + + err := pager.EachPage(func(page pagination.Page) (bool, error) { + f, err := floatingips.ExtractFloatingIPs(page) + if err != nil { + return false, err + } + floatingIPList = append(floatingIPList, f...) + if len(floatingIPList) > 1 { + return false, ErrMultipleResults + } + return true, nil + }) + if err != nil { + if isNotFound(err) { + return nil, ErrNotFound + } + return nil, err + } + + if len(floatingIPList) == 0 { + return nil, ErrNotFound + } else if len(floatingIPList) > 1 { + return nil, ErrMultipleResults + } + + return &floatingIPList[0], nil +} + func (lb *LoadBalancer) GetTCPLoadBalancer(name, region string) (*api.LoadBalancerStatus, bool, error) { vip, err := getVipByName(lb.network, name) if err == ErrNotFound { @@ -639,9 +676,30 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne } status := &api.LoadBalancerStatus{} - status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}} + + if lb.opts.FloatingNetworkId == "" { + status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}} + + return status, nil + } + + vipAddr, err := getVipByName(lb.network, name) + if err != nil { + return nil, err + } + floatIPOpts := floatingips.CreateOpts{ + FloatingNetworkID: lb.opts.FloatingNetworkId, + PortID: vipAddr.PortID, + } + floatIP, err := floatingips.Create(lb.network, floatIPOpts).Extract() + if err != nil { + return nil, err + } + + status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}, {IP: floatIP.FloatingIP}} return status, nil + } func (lb *LoadBalancer) UpdateTCPLoadBalancer(name, region string, hosts []string) error { @@ -713,6 +771,17 @@ func (lb *LoadBalancer) EnsureTCPLoadBalancerDeleted(name, region string) error return err } + if lb.opts.FloatingNetworkId != "" && vip != nil { + floatingIP, err := getFloatingIPByPortID(lb.network, vip.PortID) + if err != nil && !isNotFound(err) { + return err + } + error := floatingips.Delete(lb.network, floatingIP.ID).ExtractErr() + if error != nil && !isNotFound(error) { + return error + } + } + // We have to delete the VIP before the pool can be deleted, // so no point continuing if this fails. if vip != nil { From be5f9094d0163aff40b9924b5070f671f046b1be Mon Sep 17 00:00:00 2001 From: Sebastien LAWNICZAK Date: Thu, 13 Aug 2015 15:51:06 +0200 Subject: [PATCH 2/3] Fix : do not try to delete floatingIP if it does not exists --- pkg/cloudprovider/providers/openstack/openstack.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 40ca7c855f4..5144a7dcd34 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -776,9 +776,11 @@ func (lb *LoadBalancer) EnsureTCPLoadBalancerDeleted(name, region string) error if err != nil && !isNotFound(err) { return err } - error := floatingips.Delete(lb.network, floatingIP.ID).ExtractErr() - if error != nil && !isNotFound(error) { - return error + if floatingIP != nil { + error := floatingips.Delete(lb.network, floatingIP.ID).ExtractErr() + if error != nil && !isNotFound(error) { + return error + } } } From 4c57053d2ce726cc14a9816c48547cbff131520a Mon Sep 17 00:00:00 2001 From: Sebastien LAWNICZAK Date: Tue, 15 Sep 2015 14:29:51 +0200 Subject: [PATCH 3/3] Code review correction Inverting code path on CreateTcploadBalancer to avoid branch divergence Removing useless variable vipAddr as vip have information needed Renaming 'error' variable on EnsureTCPLoadBalancerDeleted to be consistent --- .../providers/openstack/openstack.go | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 5144a7dcd34..4f72df42755 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -677,26 +677,20 @@ func (lb *LoadBalancer) CreateTCPLoadBalancer(name, region string, externalIP ne status := &api.LoadBalancerStatus{} - if lb.opts.FloatingNetworkId == "" { - status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}} + status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}} - return status, nil - } + if lb.opts.FloatingNetworkId != "" { + floatIPOpts := floatingips.CreateOpts{ + FloatingNetworkID: lb.opts.FloatingNetworkId, + PortID: vip.PortID, + } + floatIP, err := floatingips.Create(lb.network, floatIPOpts).Extract() + if err != nil { + return nil, err + } - vipAddr, err := getVipByName(lb.network, name) - if err != nil { - return nil, err + status.Ingress = append(status.Ingress, api.LoadBalancerIngress{IP: floatIP.FloatingIP}) } - floatIPOpts := floatingips.CreateOpts{ - FloatingNetworkID: lb.opts.FloatingNetworkId, - PortID: vipAddr.PortID, - } - floatIP, err := floatingips.Create(lb.network, floatIPOpts).Extract() - if err != nil { - return nil, err - } - - status.Ingress = []api.LoadBalancerIngress{{IP: vip.Address}, {IP: floatIP.FloatingIP}} return status, nil @@ -777,9 +771,9 @@ func (lb *LoadBalancer) EnsureTCPLoadBalancerDeleted(name, region string) error return err } if floatingIP != nil { - error := floatingips.Delete(lb.network, floatingIP.ID).ExtractErr() - if error != nil && !isNotFound(error) { - return error + err = floatingips.Delete(lb.network, floatingIP.ID).ExtractErr() + if err != nil && !isNotFound(err) { + return err } } }