Skip to content

Commit

Permalink
[core] improved readability of lb funcs, switch to slices.Contains (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
kayrus authored Oct 16, 2024
1 parent a294df6 commit dde74a1
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 57 deletions.
5 changes: 2 additions & 3 deletions pkg/identity/keystone/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"os"
"regexp"
"slices"
"strings"
"sync"

Expand All @@ -32,8 +33,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"

cpoutil "k8s.io/cloud-provider-openstack/pkg/util"
)

const (
Expand Down Expand Up @@ -314,7 +313,7 @@ func (s *Syncer) syncRoles(user *userInfo) *userInfo {

if roles, isPresent := user.Extra[Roles]; isPresent {
for _, roleMap := range s.syncConfig.RoleMaps {
if roleMap.KeystoneRole != "" && cpoutil.Contains(roles, roleMap.KeystoneRole) {
if roleMap.KeystoneRole != "" && slices.Contains(roles, roleMap.KeystoneRole) {
if len(roleMap.Groups) > 0 {
user.Groups = append(user.Groups, roleMap.Groups...)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/openstack/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net"
sysos "os"
"regexp"
"slices"
"sort"
"strings"

Expand All @@ -41,7 +42,6 @@ import (
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider-openstack/pkg/client"
"k8s.io/cloud-provider-openstack/pkg/metrics"
"k8s.io/cloud-provider-openstack/pkg/util"
"k8s.io/cloud-provider-openstack/pkg/util/errors"
"k8s.io/cloud-provider-openstack/pkg/util/metadata"
)
Expand Down Expand Up @@ -677,7 +677,7 @@ func nodeAddresses(srv *servers.Server, ports []PortWithTrunkDetails, client *go
var addressType v1.NodeAddressType
if props.IPType == "floating" {
addressType = v1.NodeExternalIP
} else if util.Contains(networkingOpts.PublicNetworkName, network) {
} else if slices.Contains(networkingOpts.PublicNetworkName, network) {
addressType = v1.NodeExternalIP
// removing already added address to avoid listing it as both ExternalIP and InternalIP
// may happen due to listing "private" network as "public" in CCM's config
Expand All @@ -687,7 +687,7 @@ func nodeAddresses(srv *servers.Server, ports []PortWithTrunkDetails, client *go
},
)
} else {
if len(networkingOpts.InternalNetworkName) == 0 || util.Contains(networkingOpts.InternalNetworkName, network) {
if len(networkingOpts.InternalNetworkName) == 0 || slices.Contains(networkingOpts.InternalNetworkName, network) {
addressType = v1.NodeInternalIP
} else {
klog.V(5).Infof("Node '%s' address '%s' ignored due to 'internal-network-name' option", srv.Name, props.Addr)
Expand Down
79 changes: 40 additions & 39 deletions pkg/openstack/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"net/http"
"regexp"
"slices"
"strconv"
"strings"

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

allowedAddrTypes := []corev1.NodeAddressType{corev1.NodeInternalIP, corev1.NodeExternalIP}

for _, allowedAddrType := range allowedAddrTypes {
for _, addr := range addrs {
if addr.Type == allowedAddrType {
switch preferredIPFamily {
case corev1.IPv4Protocol:
if netutils.IsIPv4String(addr.Address) {
return addr.Address, nil
}
case corev1.IPv6Protocol:
if netutils.IsIPv6String(addr.Address) {
return addr.Address, nil
}
default:
return addr.Address, nil
}
for _, addr := range addrs {
if !slices.Contains(allowedAddrTypes, addr.Type) {
// Skip the address type that is not allowed
continue
}
switch preferredIPFamily {
case corev1.IPv4Protocol:
if netutils.IsIPv4String(addr.Address) {
return addr.Address, nil
}
case corev1.IPv6Protocol:
if netutils.IsIPv6String(addr.Address) {
return addr.Address, nil
}
default:
return addr.Address, nil
}
}

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

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

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

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

if err != nil {
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)
}
} else {
val := service.Annotations[corev1.AnnotationLoadBalancerSourceRangesKey]
val = strings.TrimSpace(val)
if val == "" {
if preferredIPFamily == corev1.IPv6Protocol {
val = defaultLoadBalancerSourceRangesIPv6
} else {
val = defaultLoadBalancerSourceRangesIPv4
}
}
specs := strings.Split(val, ",")
ipnets, err = netsets.ParseIPNets(specs...)
if err != nil {
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)

return ipnets, nil
}

val := service.Annotations[corev1.AnnotationLoadBalancerSourceRangesKey]
val = strings.TrimSpace(val)
if val == "" {
if preferredIPFamily == corev1.IPv6Protocol {
val = defaultLoadBalancerSourceRangesIPv6
} else {
val = defaultLoadBalancerSourceRangesIPv4
}
}
specs := strings.Split(val, ",")
ipnets, err := netsets.ParseIPNets(specs...)
if err != nil {
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)
}

return ipnets, nil
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"io"
"os"
"slices"
"strings"
"time"

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

if !util.Contains(supportedLBProvider, cfg.LoadBalancer.LBProvider) {
if !slices.Contains(supportedLBProvider, cfg.LoadBalancer.LBProvider) {
klog.Warningf("Unsupported LoadBalancer Provider: %s", cfg.LoadBalancer.LBProvider)
}

if !util.Contains(supportedContainerStore, cfg.LoadBalancer.ContainerStore) {
if !slices.Contains(supportedContainerStore, cfg.LoadBalancer.ContainerStore) {
klog.Warningf("Unsupported Container Store: %s", cfg.LoadBalancer.ContainerStore)
}

Expand Down
10 changes: 0 additions & 10 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,6 @@ func StringListEqual(list1, list2 []string) bool {
return s1.Equal(s2)
}

// Contains searches if a string list contains the given string or not.
func Contains(list []string, strToSearch string) bool {
for _, item := range list {
if item == strToSearch {
return true
}
}
return false
}

// StringToMap converts a string of comma-separated key-values into a map
func StringToMap(str string) map[string]string {
// break up a "key1=val,key2=val2,key3=,key4" string into a list
Expand Down

0 comments on commit dde74a1

Please sign in to comment.