Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[occm] add tag on floating ip create #2577

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

[occm] add tag on floating ip create #2577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d28309f
[occm] add tag on floating ip create
lidongshengxdayu Apr 18, 2024
9180e6d
[occm] ensure tag exist on floating ip create or lookup
lidongshengxdayu Apr 26, 2024
a7a5daf
[occm] handling tag list error
lidongshengxdayu Apr 26, 2024
4319a88
[occm] untag FIP when force CCM to not delete the FIP
lidongshengxdayu Apr 26, 2024
0e29ff2
Merge remote-tracking branch 'origin/master'
lidongshengxdayu Apr 26, 2024
48ac312
Merge remote-tracking branch 'origin/master'
lidongshengxdayu Jul 3, 2024
51c738b
[occm] fix build issue with Upgrade Gophercloud to v2
lidongshengxdayu Jul 3, 2024
5252b7e
Merge branch 'kubernetes:master' into master
lidongshengxdayu Nov 7, 2024
7196a75
[occm] fix context argument pass to `updateFloatingIPTag`
lidongshengxdayu Nov 7, 2024
c16cc03
[occm] fix `updateFloatingIPTag` typo and error print
lidongshengxdayu Nov 7, 2024
f6189ee
[occm] add check `standard-attr-tag` when update floating ip tags
lidongshengxdayu Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions pkg/openstack/loadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/loadbalancers"
v2monitors "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/monitors"
v2pools "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/pools"
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/attributestags"
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/floatingips"
"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/subnets"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -618,6 +619,43 @@ func (lbaas *LbaasV2) updateFloatingIP(ctx context.Context, floatingip *floating
return floatingip, nil
}

func (lbaas *LbaasV2) updateFloatingIPTag(ctx context.Context, fip *floatingips.FloatingIP, Tag string, del bool) error {
if enabled, ok := lbaas.netExtensions["standard-attr-tag"]; !ok || !enabled {
return nil
}
if Tag == "" {
return fmt.Errorf("Error input tag argument ")
}
tags, err := attributestags.List(ctx, lbaas.network, "floatingips", fip.ID).Extract()
if err != nil {
klog.V(3).Infof("Cannot get floatIP tags for floating %s", fip.ID)
return err
}

found := slices.Contains(tags, Tag)

if (found && !del) || (!found && del) {
return nil
}

if found && del {
err = attributestags.Delete(ctx, lbaas.network, "floatingips", fip.ID, Tag).ExtractErr()
if err != nil {
klog.V(2).ErrorS(err, "Cannot update floating IP tag %s for floating %s", Tag, fip.ID)
}
return err
}

if !found && !del {
err = attributestags.Add(ctx, lbaas.network, "floatingips", fip.ID, Tag).ExtractErr()
if err != nil {
klog.V(2).ErrorS(err, "Cannot update floating IP tag %s for floating %s", Tag, fip.ID)
}
return err
}
return nil
}

// ensureFloatingIP manages a FIP for a Service and returns the address that should be advertised in the
// .Status.LoadBalancer. In particular it will:
// 1. Lookup if any FIP is already attached to the VIP port of the LB.
Expand Down Expand Up @@ -663,6 +701,7 @@ func (lbaas *LbaasV2) ensureFloatingIP(ctx context.Context, clusterName string,
if err != nil {
return "", err
}
_ = lbaas.updateFloatingIPTag(ctx, floatIP, svcConf.lbName, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why do you skip err?

Copy link
Author

@lidongshengxdayu lidongshengxdayu Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether returning an error will cause a wider range of logic errors, and I'm not very familiar with the code base.
And tag is optional. I think it should not affect the normal logic of floating ip. Even if an error occurs, it can be checked through the log.
At this position, even if an error occurs and return, the next loop cannot be updated either

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you think this and the add below need to return err, tell me and I will add it.

}
}
return lb.VipAddress, nil
Expand Down Expand Up @@ -762,6 +801,7 @@ func (lbaas *LbaasV2) ensureFloatingIP(ctx context.Context, clusterName string,
}

if floatIP != nil {
_ = lbaas.updateFloatingIPTag(ctx, floatIP, svcConf.lbName, false)
return floatIP.FloatingIP, nil
}

Expand Down Expand Up @@ -1760,6 +1800,13 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
if err != nil {
return nil, err
}
extensions, err := openstackutil.GetNetworkExtensions(ctx, lbaas.network)
if err == nil {
klog.V(2).ErrorS(err, "cannot get network extensions")
lbaas.netExtensions = extensions
} else {
lbaas.netExtensions = map[string]bool{}
}

klog.V(4).InfoS("Load balancer ensured", "lbID", loadbalancer.ID, "isLBOwner", isLBOwner, "createNewLB", createNewLB)

Expand Down
3 changes: 2 additions & 1 deletion pkg/openstack/openstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ type LoadBalancer struct {
opts LoadBalancerOpts
kclient kubernetes.Interface
eventRecorder record.EventRecorder
netExtensions map[string]bool
}

// LoadBalancerOpts have the options to talk to Neutron LBaaSV2 or Octavia
Expand Down Expand Up @@ -375,7 +376,7 @@ func (os *OpenStack) LoadBalancer() (cloudprovider.LoadBalancer, bool) {

klog.V(1).Info("Claiming to support LoadBalancer")

return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder}}, true
return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, map[string]bool{}}}, true
}

// Zones indicates that we support zones
Expand Down