Skip to content

Commit dde74a1

Browse files
kayruskayrus
authored
[core] improved readability of lb funcs, switch to slices.Contains (#2688)
1 parent a294df6 commit dde74a1

5 files changed

Lines changed: 48 additions & 57 deletions

File tree

‎pkg/identity/keystone/sync.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"os"
2424
"regexp"
25+
"slices"
2526
"strings"
2627
"sync"
2728

@@ -32,8 +33,6 @@ import (
3233
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3334
"k8s.io/client-go/kubernetes"
3435
"k8s.io/klog/v2"
35-
36-
cpoutil "k8s.io/cloud-provider-openstack/pkg/util"
3736
)
3837

3938
const (
@@ -314,7 +313,7 @@ func (s *Syncer) syncRoles(user *userInfo) *userInfo {
314313

315314
if roles, isPresent := user.Extra[Roles]; isPresent {
316315
for _, roleMap := range s.syncConfig.RoleMaps {
317-
if roleMap.KeystoneRole != "" && cpoutil.Contains(roles, roleMap.KeystoneRole) {
316+
if roleMap.KeystoneRole != "" && slices.Contains(roles, roleMap.KeystoneRole) {
318317
if len(roleMap.Groups) > 0 {
319318
user.Groups = append(user.Groups, roleMap.Groups...)
320319
}

‎pkg/openstack/instances.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"net"
2424
sysos "os"
2525
"regexp"
26+
"slices"
2627
"sort"
2728
"strings"
2829

@@ -41,7 +42,6 @@ import (
4142
cloudprovider "k8s.io/cloud-provider"
4243
"k8s.io/cloud-provider-openstack/pkg/client"
4344
"k8s.io/cloud-provider-openstack/pkg/metrics"
44-
"k8s.io/cloud-provider-openstack/pkg/util"
4545
"k8s.io/cloud-provider-openstack/pkg/util/errors"
4646
"k8s.io/cloud-provider-openstack/pkg/util/metadata"
4747
)
@@ -677,7 +677,7 @@ func nodeAddresses(srv *servers.Server, ports []PortWithTrunkDetails, client *go
677677
var addressType v1.NodeAddressType
678678
if props.IPType == "floating" {
679679
addressType = v1.NodeExternalIP
680-
} else if util.Contains(networkingOpts.PublicNetworkName, network) {
680+
} else if slices.Contains(networkingOpts.PublicNetworkName, network) {
681681
addressType = v1.NodeExternalIP
682682
// removing already added address to avoid listing it as both ExternalIP and InternalIP
683683
// may happen due to listing "private" network as "public" in CCM's config
@@ -687,7 +687,7 @@ func nodeAddresses(srv *servers.Server, ports []PortWithTrunkDetails, client *go
687687
},
688688
)
689689
} else {
690-
if len(networkingOpts.InternalNetworkName) == 0 || util.Contains(networkingOpts.InternalNetworkName, network) {
690+
if len(networkingOpts.InternalNetworkName) == 0 || slices.Contains(networkingOpts.InternalNetworkName, network) {
691691
addressType = v1.NodeInternalIP
692692
} else {
693693
klog.V(5).Infof("Node '%s' address '%s' ignored due to 'internal-network-name' option", srv.Name, props.Addr)

‎pkg/openstack/loadbalancer.go‎

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"net/http"
2424
"regexp"
25+
"slices"
2526
"strconv"
2627
"strings"
2728

@@ -386,23 +387,22 @@ func nodeAddressForLB(node *corev1.Node, preferredIPFamily corev1.IPFamily) (str
386387
}
387388

388389
allowedAddrTypes := []corev1.NodeAddressType{corev1.NodeInternalIP, corev1.NodeExternalIP}
389-
390-
for _, allowedAddrType := range allowedAddrTypes {
391-
for _, addr := range addrs {
392-
if addr.Type == allowedAddrType {
393-
switch preferredIPFamily {
394-
case corev1.IPv4Protocol:
395-
if netutils.IsIPv4String(addr.Address) {
396-
return addr.Address, nil
397-
}
398-
case corev1.IPv6Protocol:
399-
if netutils.IsIPv6String(addr.Address) {
400-
return addr.Address, nil
401-
}
402-
default:
403-
return addr.Address, nil
404-
}
390+
for _, addr := range addrs {
391+
if !slices.Contains(allowedAddrTypes, addr.Type) {
392+
// Skip the address type that is not allowed
393+
continue
394+
}
395+
switch preferredIPFamily {
396+
case corev1.IPv4Protocol:
397+
if netutils.IsIPv4String(addr.Address) {
398+
return addr.Address, nil
405399
}
400+
case corev1.IPv6Protocol:
401+
if netutils.IsIPv6String(addr.Address) {
402+
return addr.Address, nil
403+
}
404+
default:
405+
return addr.Address, nil
406406
}
407407
}
408408

@@ -558,7 +558,7 @@ func (lbaas *LbaasV2) deleteListeners(lbID string, listenerList []listeners.List
558558
func (lbaas *LbaasV2) deleteOctaviaListeners(lbID string, listenerList []listeners.Listener, isLBOwner bool, lbName string) error {
559559
for _, listener := range listenerList {
560560
// If the listener was created by this Service before or after supporting shared LB.
561-
if (isLBOwner && len(listener.Tags) == 0) || cpoutil.Contains(listener.Tags, lbName) {
561+
if (isLBOwner && len(listener.Tags) == 0) || slices.Contains(listener.Tags, lbName) {
562562
klog.InfoS("Deleting listener", "listenerID", listener.ID, "lbID", lbID)
563563

564564
pool, err := openstackutil.GetPoolByListener(lbaas.lb, lbID, listener.ID)
@@ -1069,7 +1069,7 @@ func (lbaas *LbaasV2) ensureOctaviaListener(lbID string, name string, curListene
10691069
updateOpts := listeners.UpdateOpts{}
10701070

10711071
if svcConf.supportLBTags {
1072-
if !cpoutil.Contains(listener.Tags, svcConf.lbName) {
1072+
if !slices.Contains(listener.Tags, svcConf.lbName) {
10731073
var newTags []string
10741074
copy(newTags, listener.Tags)
10751075
newTags = append(newTags, svcConf.lbName)
@@ -1612,7 +1612,7 @@ func (lbaas *LbaasV2) checkListenerPorts(service *corev1.Service, curListenerMap
16121612
if listener, isPresent := curListenerMapping[key]; isPresent {
16131613
// The listener is used by this Service if LB name is in the tags, or
16141614
// the listener was created by this Service.
1615-
if cpoutil.Contains(listener.Tags, lbName) || (len(listener.Tags) == 0 && isLBOwner) {
1615+
if slices.Contains(listener.Tags, lbName) || (len(listener.Tags) == 0 && isLBOwner) {
16161616
continue
16171617
} else {
16181618
return fmt.Errorf("the listener port %d already exists", svcPort.Port)
@@ -1721,7 +1721,7 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
17211721
sharedCount++
17221722
}
17231723
}
1724-
if !isLBOwner && !cpoutil.Contains(loadbalancer.Tags, lbName) && sharedCount+1 > lbaas.opts.MaxSharedLB {
1724+
if !isLBOwner && !slices.Contains(loadbalancer.Tags, lbName) && sharedCount+1 > lbaas.opts.MaxSharedLB {
17251725
return nil, fmt.Errorf("load balancer %s already shared with %d Services", loadbalancer.ID, sharedCount)
17261726
}
17271727

@@ -1825,7 +1825,7 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
18251825
// add LB name to load balancer tags.
18261826
if svcConf.supportLBTags {
18271827
lbTags := loadbalancer.Tags
1828-
if !cpoutil.Contains(lbTags, lbName) {
1828+
if !slices.Contains(lbTags, lbName) {
18291829
lbTags = append(lbTags, lbName)
18301830
klog.InfoS("Updating load balancer tags", "lbID", loadbalancer.ID, "tags", lbTags)
18311831
if err := openstackutil.UpdateLoadBalancerTags(lbaas.lb, loadbalancer.ID, lbTags); err != nil {
@@ -2029,7 +2029,7 @@ func (lbaas *LbaasV2) deleteLoadBalancer(loadbalancer *loadbalancers.LoadBalance
20292029
Protocol: proto,
20302030
Port: int(port.Port),
20312031
}]
2032-
if isPresent && cpoutil.Contains(listener.Tags, svcConf.lbName) {
2032+
if isPresent && slices.Contains(listener.Tags, svcConf.lbName) {
20332033
listenersToDelete = append(listenersToDelete, *listener)
20342034
}
20352035
}
@@ -2186,32 +2186,33 @@ func (lbaas *LbaasV2) ensureLoadBalancerDeleted(ctx context.Context, clusterName
21862186
// If the field is not specified, turn to parse and verify the AnnotationLoadBalancerSourceRangesKey annotation from a service,
21872187
// extracting the source ranges to allow, and if not present returns a default (allow-all) value.
21882188
func GetLoadBalancerSourceRanges(service *corev1.Service, preferredIPFamily corev1.IPFamily) (netsets.IPNet, error) {
2189-
var ipnets netsets.IPNet
2190-
var err error
21912189
// if SourceRange field is specified, ignore sourceRange annotation
21922190
if len(service.Spec.LoadBalancerSourceRanges) > 0 {
21932191
specs := service.Spec.LoadBalancerSourceRanges
2194-
ipnets, err = netsets.ParseIPNets(specs...)
2192+
ipnets, err := netsets.ParseIPNets(specs...)
21952193

21962194
if err != nil {
21972195
return nil, fmt.Errorf("service.Spec.LoadBalancerSourceRanges: %v is not valid. Expecting a list of IP ranges. For example, 10.0.0.0/24. Error msg: %v", specs, err)
21982196
}
2199-
} else {
2200-
val := service.Annotations[corev1.AnnotationLoadBalancerSourceRangesKey]
2201-
val = strings.TrimSpace(val)
2202-
if val == "" {
2203-
if preferredIPFamily == corev1.IPv6Protocol {
2204-
val = defaultLoadBalancerSourceRangesIPv6
2205-
} else {
2206-
val = defaultLoadBalancerSourceRangesIPv4
2207-
}
2208-
}
2209-
specs := strings.Split(val, ",")
2210-
ipnets, err = netsets.ParseIPNets(specs...)
2211-
if err != nil {
2212-
return nil, fmt.Errorf("%s: %s is not valid. Expecting a comma-separated list of source IP ranges. For example, 10.0.0.0/24,192.168.2.0/24", corev1.AnnotationLoadBalancerSourceRangesKey, val)
2197+
2198+
return ipnets, nil
2199+
}
2200+
2201+
val := service.Annotations[corev1.AnnotationLoadBalancerSourceRangesKey]
2202+
val = strings.TrimSpace(val)
2203+
if val == "" {
2204+
if preferredIPFamily == corev1.IPv6Protocol {
2205+
val = defaultLoadBalancerSourceRangesIPv6
2206+
} else {
2207+
val = defaultLoadBalancerSourceRangesIPv4
22132208
}
22142209
}
2210+
specs := strings.Split(val, ",")
2211+
ipnets, err := netsets.ParseIPNets(specs...)
2212+
if err != nil {
2213+
return nil, fmt.Errorf("%s: %s is not valid. Expecting a comma-separated list of source IP ranges. For example, 10.0.0.0/24,192.168.2.0/24", corev1.AnnotationLoadBalancerSourceRangesKey, val)
2214+
}
2215+
22152216
return ipnets, nil
22162217
}
22172218

‎pkg/openstack/openstack.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"io"
2323
"os"
24+
"slices"
2425
"strings"
2526
"time"
2627

@@ -258,11 +259,11 @@ func ReadConfig(config io.Reader) (Config, error) {
258259
cfg.Metadata.SearchOrder = fmt.Sprintf("%s,%s", metadata.ConfigDriveID, metadata.MetadataID)
259260
}
260261

261-
if !util.Contains(supportedLBProvider, cfg.LoadBalancer.LBProvider) {
262+
if !slices.Contains(supportedLBProvider, cfg.LoadBalancer.LBProvider) {
262263
klog.Warningf("Unsupported LoadBalancer Provider: %s", cfg.LoadBalancer.LBProvider)
263264
}
264265

265-
if !util.Contains(supportedContainerStore, cfg.LoadBalancer.ContainerStore) {
266+
if !slices.Contains(supportedContainerStore, cfg.LoadBalancer.ContainerStore) {
266267
klog.Warningf("Unsupported Container Store: %s", cfg.LoadBalancer.ContainerStore)
267268
}
268269

‎pkg/util/util.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,6 @@ func StringListEqual(list1, list2 []string) bool {
6969
return s1.Equal(s2)
7070
}
7171

72-
// Contains searches if a string list contains the given string or not.
73-
func Contains(list []string, strToSearch string) bool {
74-
for _, item := range list {
75-
if item == strToSearch {
76-
return true
77-
}
78-
}
79-
return false
80-
}
81-
8272
// StringToMap converts a string of comma-separated key-values into a map
8373
func StringToMap(str string) map[string]string {
8474
// break up a "key1=val,key2=val2,key3=,key4" string into a list

0 commit comments

Comments
 (0)