Skip to content

Commit

Permalink
Fix kops update for OpenStack with LB
Browse files Browse the repository at this point in the history
In the last PR to support OVN provider for LB, listener will refer to
load balancer provider for ACL settings. While currently get listener
API returns empty Pools, which will cause nil pointer dereference when
referring Pool.Loadbalancer.Provider.

This commit fix this issue by getting pool information with
DefaultPoolID when Pools is empty. As I added GetPool function, the
origin GetPool function is renamed to GetPoolMember.
  • Loading branch information
ching-kuo committed May 25, 2022
1 parent c232710 commit 17d2f08
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 14 deletions.
3 changes: 2 additions & 1 deletion upup/pkg/fi/cloudup/openstack/cloud.go
Expand Up @@ -269,7 +269,8 @@ type OpenstackCloud interface {
AssociateToPool(server *servers.Server, poolID string, opts v2pools.CreateMemberOpts) (*v2pools.Member, error)
CreatePool(opts v2pools.CreateOpts) (*v2pools.Pool, error)
CreatePoolMonitor(opts monitors.CreateOpts) (*monitors.Monitor, error)
GetPool(poolID string, memberID string) (*v2pools.Member, error)
GetPool(poolID string) (*v2pools.Pool, error)
GetPoolMember(poolID string, memberID string) (*v2pools.Member, error)
ListPools(v2pools.ListOpts) ([]v2pools.Pool, error)

// ListMonitors will list HealthMonitors matching the provided options
Expand Down
31 changes: 28 additions & 3 deletions upup/pkg/fi/cloudup/openstack/loadbalancer.go
Expand Up @@ -302,11 +302,36 @@ func getLBStats(c OpenstackCloud, loadbalancerID string) (stats *loadbalancers.S
return stats, nil
}

func (c *openstackCloud) GetPool(poolID string, memberID string) (member *v2pools.Member, err error) {
return getPool(c, poolID, memberID)
func (c *openstackCloud) GetPool(poolID string) (pool *v2pools.Pool, err error) {
return getPool(c, poolID)
}

func getPool(c OpenstackCloud, poolID string, memberID string) (member *v2pools.Member, err error) {
func getPool(c OpenstackCloud, poolID string) (pool *v2pools.Pool, err error) {
if c.LoadBalancerClient() == nil {
return nil, fmt.Errorf("loadbalancer support not available in this deployment")
}

done, err := vfs.RetryWithBackoff(readBackoff, func() (bool, error) {
pool, err = v2pools.Get(c.LoadBalancerClient(), poolID).Extract()
if err != nil {
return false, err
}
return true, nil
})
if !done {
if err == nil {
err = wait.ErrWaitTimeout
}
return pool, err
}
return pool, nil
}

func (c *openstackCloud) GetPoolMember(poolID string, memberID string) (member *v2pools.Member, err error) {
return getPoolMember(c, poolID, memberID)
}

func getPoolMember(c OpenstackCloud, poolID string, memberID string) (member *v2pools.Member, err error) {
if c.LoadBalancerClient() == nil {
return nil, fmt.Errorf("loadbalancer support not available in this deployment")
}
Expand Down
8 changes: 6 additions & 2 deletions upup/pkg/fi/cloudup/openstack/mock_cloud.go
Expand Up @@ -369,8 +369,12 @@ func (c *MockCloud) GetLBFloatingSubnet() (subnet *subnets.Subnet, err error) {
return getLBFloatingSubnet(c, c.floatingSubnet)
}

func (c *MockCloud) GetPool(poolID string, memberID string) (member *v2pools.Member, err error) {
return getPool(c, poolID, memberID)
func (c *MockCloud) GetPool(poolID string) (pool *v2pools.Pool, err error) {
return getPool(c, poolID)
}

func (c *MockCloud) GetPoolMember(poolID string, memberID string) (member *v2pools.Member, err error) {
return getPoolMember(c, poolID, memberID)
}

func (c *MockCloud) GetPort(id string) (*ports.Port, error) {
Expand Down
1 change: 1 addition & 0 deletions upup/pkg/fi/cloudup/openstacktasks/lb.go
Expand Up @@ -139,6 +139,7 @@ func NewLBTaskFromCloud(cloud openstack.OpenstackCloud, lifecycle fi.Lifecycle,
find.ID = actual.ID
find.PortID = actual.PortID
find.VipSubnet = actual.VipSubnet
find.Provider = actual.Provider
}
return actual, nil
}
Expand Down
26 changes: 19 additions & 7 deletions upup/pkg/fi/cloudup/openstacktasks/lblistener.go
Expand Up @@ -65,15 +65,27 @@ func NewLBListenerTaskFromCloud(cloud openstack.OpenstackCloud, lifecycle fi.Lif
Lifecycle: lifecycle,
}

for _, pool := range listener.Pools {
poolTask, err := NewLBPoolTaskFromCloud(cloud, lifecycle, &pool, find.Pool)
if len(listener.Pools) > 0 {
for _, pool := range listener.Pools {
poolTask, err := NewLBPoolTaskFromCloud(cloud, lifecycle, &pool, find.Pool)
if err != nil {
return nil, fmt.Errorf("NewLBListenerTaskFromCloud: Failed to create new LBListener task for pool %s: %v", pool.Name, err)
} else {
listenerTask.Pool = poolTask
// TODO: Support Multiple?
break
}
}
} else {
pool, err := cloud.GetPool(listener.DefaultPoolID)
if err != nil {
return nil, fmt.Errorf("Fail to get pool with ID: %s: %v", listener.DefaultPoolID, err)
}
poolTask, err := NewLBPoolTaskFromCloud(cloud, lifecycle, pool, find.Pool)
if err != nil {
return nil, fmt.Errorf("NewLBListenerTaskFromCloud: Failed to create new LBListener task for pool %s: %v", pool.Name, err)
} else {
listenerTask.Pool = poolTask
// TODO: Support Multiple?
break
}
listenerTask.Pool = poolTask
}
if find != nil {
// Update all search terms
Expand Down Expand Up @@ -154,7 +166,7 @@ func (_ *LBListener) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, chan
e.ID = fi.String(listener.ID)
return nil
} else if len(changes.AllowedCIDRs) > 0 {
if useVIPACL && (fi.StringValue(e.Pool.Loadbalancer.Provider) != "ovn") {
if useVIPACL && (fi.StringValue(a.Pool.Loadbalancer.Provider) != "ovn") {
opts := listeners.UpdateOpts{
AllowedCIDRs: &changes.AllowedCIDRs,
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/openstacktasks/poolassociation.go
Expand Up @@ -85,7 +85,7 @@ func (p *PoolAssociation) Find(context *fi.Context) (*PoolAssociation, error) {
// check is member already created
var found *v2pools.Member
for _, member := range a.Members {
poolMember, err := cloud.GetPool(a.ID, member.ID)
poolMember, err := cloud.GetPoolMember(a.ID, member.ID)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 17d2f08

Please sign in to comment.