Skip to content

Commit

Permalink
chore: Add case-insensitive string set utility
Browse files Browse the repository at this point in the history 
A new structure called IgnoreCaseSet has been introduced into the pkg/util/sets package. This constructs a set of strings that is case-insensitive, providing methods for applying standard set operations. It's beneficial for handling inputs where case sensitivity isn't important or might cause issues.
  • Loading branch information
@nilo19
nilo19 authored and k8s-infra-cherrypick-robot committed Feb 18, 2024
1 parent 239000e commit afcf72f
Show file tree
Hide file tree
Showing 20 changed files with 691 additions and 272 deletions.
77 changes: 37 additions & 40 deletions pkg/provider/azure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ import (
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/adal"
"github.com/Azure/go-autorest/autorest/azure"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/informers"
clientset "k8s.io/client-go/kubernetes"
Expand All @@ -48,7 +48,6 @@ import (
cloudnodeutil "k8s.io/cloud-provider/node/helpers"
nodeutil "k8s.io/component-helpers/node/util"
"k8s.io/klog/v2"
"sigs.k8s.io/yaml"

"sigs.k8s.io/cloud-provider-azure/pkg/azclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azclient/configloader"
Expand Down Expand Up @@ -76,10 +75,14 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssvmclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/zoneclient"

"sigs.k8s.io/yaml"

azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
ratelimitconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
"sigs.k8s.io/cloud-provider-azure/pkg/util/taints"
)

Expand Down Expand Up @@ -331,11 +334,11 @@ type MultipleStandardLoadBalancerConfigurationSpec struct {
// MultipleStandardLoadBalancerConfigurationStatus stores the properties regarding multiple standard load balancers.
type MultipleStandardLoadBalancerConfigurationStatus struct {
// ActiveServices stores the services that are supposed to use the load balancer.
ActiveServices sets.Set[string] `json:"activeServices" yaml:"activeServices"`
ActiveServices *utilsets.IgnoreCaseSet `json:"activeServices" yaml:"activeServices"`

// ActiveNodes stores the nodes that are supposed to be in the load balancer.
// It will be used in EnsureHostsInPool to make sure the given ones are in the backend pool.
ActiveNodes sets.Set[string] `json:"activeNodes" yaml:"activeNodes"`
ActiveNodes *utilsets.IgnoreCaseSet `json:"activeNodes" yaml:"activeNodes"`
}

// HasExtendedLocation returns true if extendedlocation prop are specified.
Expand Down Expand Up @@ -392,17 +395,17 @@ type Cloud struct {
// Lock for access to node caches, includes nodeZones, nodeResourceGroups, and unmanagedNodes.
nodeCachesLock sync.RWMutex
// nodeNames holds current nodes for tracking added nodes in VM caches.
nodeNames sets.Set[string]
nodeNames *utilsets.IgnoreCaseSet
// nodeZones is a mapping from Zone to a sets.Set[string] of Node's names in the Zone
// it is updated by the nodeInformer
nodeZones map[string]sets.Set[string]
nodeZones map[string]*utilsets.IgnoreCaseSet
// nodeResourceGroups holds nodes external resource groups
nodeResourceGroups map[string]string
// unmanagedNodes holds a list of nodes not managed by Azure cloud provider.
unmanagedNodes sets.Set[string]
unmanagedNodes *utilsets.IgnoreCaseSet
// excludeLoadBalancerNodes holds a list of nodes that should be excluded from LoadBalancer.
excludeLoadBalancerNodes sets.Set[string]
nodePrivateIPs map[string]sets.Set[string]
excludeLoadBalancerNodes *utilsets.IgnoreCaseSet
nodePrivateIPs map[string]*utilsets.IgnoreCaseSet
nodePrivateIPToNodeNameMap map[string]string
// nodeInformerSynced is for determining if the informer has synced.
nodeInformerSynced cache.InformerSynced
Expand Down Expand Up @@ -455,13 +458,13 @@ type Cloud struct {
// NewCloud returns a Cloud with initialized clients
func NewCloud(ctx context.Context, config *Config, callFromCCM bool) (cloudprovider.Interface, error) {
az := &Cloud{
nodeNames: sets.New[string](),
nodeZones: map[string]sets.Set[string]{},
nodeNames: utilsets.NewString(),
nodeZones: map[string]*utilsets.IgnoreCaseSet{},
nodeResourceGroups: map[string]string{},
unmanagedNodes: sets.New[string](),
unmanagedNodes: utilsets.NewString(),
routeCIDRs: map[string]string{},
excludeLoadBalancerNodes: sets.New[string](),
nodePrivateIPs: map[string]sets.Set[string]{},
excludeLoadBalancerNodes: utilsets.NewString(),
nodePrivateIPs: map[string]*utilsets.IgnoreCaseSet{},
nodePrivateIPToNodeNameMap: map[string]string{},
}

Expand Down Expand Up @@ -565,7 +568,7 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
// The default cloud config type is cloudConfigTypeMerge.
config.CloudConfigType = configloader.CloudConfigTypeMerge
} else {
supportedCloudConfigTypes := sets.New(
supportedCloudConfigTypes := utilsets.NewString(
string(configloader.CloudConfigTypeMerge),
string(configloader.CloudConfigTypeFile),
string(configloader.CloudConfigTypeSecret))
Expand All @@ -579,7 +582,7 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
strings.EqualFold(config.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypePODIP) {
config.LoadBalancerBackendPoolConfigurationType = consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration
} else {
supportedLoadBalancerBackendPoolConfigurationTypes := sets.New(
supportedLoadBalancerBackendPoolConfigurationTypes := utilsets.NewString(
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration),
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypeNodeIP),
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypePODIP))
Expand All @@ -591,7 +594,7 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
if config.ClusterServiceLoadBalancerHealthProbeMode == "" {
config.ClusterServiceLoadBalancerHealthProbeMode = consts.ClusterServiceLoadBalancerHealthProbeModeServiceNodePort
} else {
supportedClusterServiceLoadBalancerHealthProbeModes := sets.New(
supportedClusterServiceLoadBalancerHealthProbeModes := utilsets.NewString(
strings.ToLower(consts.ClusterServiceLoadBalancerHealthProbeModeServiceNodePort),
strings.ToLower(consts.ClusterServiceLoadBalancerHealthProbeModeShared),
)
Expand Down Expand Up @@ -782,8 +785,8 @@ func (az *Cloud) checkEnableMultipleStandardLoadBalancers() error {
return fmt.Errorf("multiple standard load balancers cannot be used with backend pool type %s", consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration)
}

names := sets.New[string]()
primaryVMSets := sets.New[string]()
names := utilsets.NewString()
primaryVMSets := utilsets.NewString()
for _, multiSLBConfig := range az.MultipleStandardLoadBalancerConfigurations {
if names.Has(multiSLBConfig.Name) {
return fmt.Errorf("duplicated multiple standard load balancer configuration name %s", multiSLBConfig.Name)
Expand Down Expand Up @@ -1257,15 +1260,12 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {

if newNode != nil {
// Add to nodeNames cache.
az.nodeNames.Insert(newNode.ObjectMeta.Name)
az.nodeNames = utilsets.SafeInsert(az.nodeNames, newNode.ObjectMeta.Name)

// Add to nodeZones cache.
newZone, ok := newNode.ObjectMeta.Labels[v1.LabelTopologyZone]
if ok && az.isAvailabilityZone(newZone) {
if az.nodeZones[newZone] == nil {
az.nodeZones[newZone] = sets.New[string]()
}
az.nodeZones[newZone].Insert(newNode.ObjectMeta.Name)
az.nodeZones[newZone] = utilsets.SafeInsert(az.nodeZones[newZone], newNode.ObjectMeta.Name)
}

// Add to nodeResourceGroups cache.
Expand Down Expand Up @@ -1301,15 +1301,12 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {

// Add to nodePrivateIPs cache
for _, address := range getNodePrivateIPAddresses(newNode) {
if az.nodePrivateIPs[strings.ToLower(newNode.Name)] == nil {
az.nodePrivateIPs[strings.ToLower(newNode.Name)] = sets.New[string]()
}
if az.nodePrivateIPToNodeNameMap == nil {
az.nodePrivateIPToNodeNameMap = make(map[string]string)
}

klog.V(6).Infof("adding IP address %s of the node %s", address, newNode.Name)
az.nodePrivateIPs[strings.ToLower(newNode.Name)].Insert(address)
az.nodePrivateIPs[strings.ToLower(newNode.Name)] = utilsets.SafeInsert(az.nodePrivateIPs[strings.ToLower(newNode.Name)], address)
az.nodePrivateIPToNodeNameMap[address] = newNode.Name
}
}
Expand Down Expand Up @@ -1345,7 +1342,7 @@ func (az *Cloud) updateNodeTaint(node *v1.Node) {
}

// GetActiveZones returns all the zones in which k8s nodes are currently running.
func (az *Cloud) GetActiveZones() (sets.Set[string], error) {
func (az *Cloud) GetActiveZones() (*utilsets.IgnoreCaseSet, error) {
if az.nodeInformerSynced == nil {
return nil, fmt.Errorf("azure cloud provider doesn't have informers set")
}
Expand All @@ -1356,9 +1353,9 @@ func (az *Cloud) GetActiveZones() (sets.Set[string], error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetActiveZones")
}

zones := sets.New[string]()
zones := utilsets.NewString()
for zone, nodes := range az.nodeZones {
if len(nodes) > 0 {
if nodes.Len() > 0 {
zones.Insert(zone)
}
}
Expand Down Expand Up @@ -1393,7 +1390,7 @@ func (az *Cloud) GetNodeResourceGroup(nodeName string) (string, error) {
}

// GetNodeNames returns a set of all node names in the k8s cluster.
func (az *Cloud) GetNodeNames() (sets.Set[string], error) {
func (az *Cloud) GetNodeNames() (*utilsets.IgnoreCaseSet, error) {
// Kubelet won't set az.nodeInformerSynced, return nil.
if az.nodeInformerSynced == nil {
return nil, nil
Expand All @@ -1405,14 +1402,14 @@ func (az *Cloud) GetNodeNames() (sets.Set[string], error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetNodeNames")
}

return sets.New(az.nodeNames.UnsortedList()...), nil
return utilsets.NewString(az.nodeNames.UnsortedList()...), nil
}

// GetResourceGroups returns a set of resource groups that all nodes are running on.
func (az *Cloud) GetResourceGroups() (sets.Set[string], error) {
func (az *Cloud) GetResourceGroups() (*utilsets.IgnoreCaseSet, error) {
// Kubelet won't set az.nodeInformerSynced, always return configured resourceGroup.
if az.nodeInformerSynced == nil {
return sets.New(az.ResourceGroup), nil
return utilsets.NewString(az.ResourceGroup), nil
}

az.nodeCachesLock.RLock()
Expand All @@ -1421,7 +1418,7 @@ func (az *Cloud) GetResourceGroups() (sets.Set[string], error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetResourceGroups")
}

resourceGroups := sets.New(az.ResourceGroup)
resourceGroups := utilsets.NewString(az.ResourceGroup)
for _, rg := range az.nodeResourceGroups {
resourceGroups.Insert(rg)
}
Expand All @@ -1430,7 +1427,7 @@ func (az *Cloud) GetResourceGroups() (sets.Set[string], error) {
}

// GetUnmanagedNodes returns a list of nodes not managed by Azure cloud provider (e.g. on-prem nodes).
func (az *Cloud) GetUnmanagedNodes() (sets.Set[string], error) {
func (az *Cloud) GetUnmanagedNodes() (*utilsets.IgnoreCaseSet, error) {
// Kubelet won't set az.nodeInformerSynced, always return nil.
if az.nodeInformerSynced == nil {
return nil, nil
Expand All @@ -1442,7 +1439,7 @@ func (az *Cloud) GetUnmanagedNodes() (sets.Set[string], error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetUnmanagedNodes")
}

return sets.New(az.unmanagedNodes.UnsortedList()...), nil
return utilsets.NewString(az.unmanagedNodes.UnsortedList()...), nil
}

// ShouldNodeExcludedFromLoadBalancer returns true if node is unmanaged, in external resource group or labeled with "node.kubernetes.io/exclude-from-external-load-balancers".
Expand All @@ -1466,7 +1463,7 @@ func (az *Cloud) ShouldNodeExcludedFromLoadBalancer(nodeName string) (bool, erro
return az.excludeLoadBalancerNodes.Has(nodeName), nil
}

func (az *Cloud) getActiveNodesByLoadBalancerName(lbName string) sets.Set[string] {
func (az *Cloud) getActiveNodesByLoadBalancerName(lbName string) *utilsets.IgnoreCaseSet {
az.multipleStandardLoadBalancersActiveNodesLock.Lock()
defer az.multipleStandardLoadBalancersActiveNodesLock.Unlock()

Expand All @@ -1476,7 +1473,7 @@ func (az *Cloud) getActiveNodesByLoadBalancerName(lbName string) sets.Set[string
}
}

return sets.New[string]()
return utilsets.NewString()
}

func isNodeReady(node *v1.Node) bool {
Expand Down
10 changes: 5 additions & 5 deletions pkg/provider/azure_fakes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"go.uber.org/mock/gomock"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/record"

"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/diskclient/mockdiskclient"
Expand All @@ -42,6 +41,7 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssclient/mockvmssclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssvmclient/mockvmssvmclient"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
)

// NewTestScaleSet creates a fake ScaleSet for unit test
Expand Down Expand Up @@ -96,12 +96,12 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
VMType: consts.VMTypeStandard,
LoadBalancerBackendPoolConfigurationType: consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration,
},
nodeZones: map[string]sets.Set[string]{},
nodeZones: map[string]*utilsets.IgnoreCaseSet{},
nodeInformerSynced: func() bool { return true },
nodeResourceGroups: map[string]string{},
unmanagedNodes: sets.New[string](),
excludeLoadBalancerNodes: sets.New[string](),
nodePrivateIPs: map[string]sets.Set[string]{},
unmanagedNodes: utilsets.NewString(),
excludeLoadBalancerNodes: utilsets.NewString(),
nodePrivateIPs: map[string]*utilsets.IgnoreCaseSet{},
routeCIDRs: map[string]string{},
eventRecorder: &record.FakeRecorder{},
lockMap: newLockMap(),
Expand Down
6 changes: 3 additions & 3 deletions pkg/provider/azure_instances_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"strings"
"testing"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute"
Expand All @@ -46,6 +45,7 @@ import (
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
)

// setTestVirtualMachines sets test virtual machine with powerstate.
Expand Down Expand Up @@ -883,7 +883,7 @@ func TestInstanceMetadata(t *testing.T) {

t.Run("instance not exists", func(t *testing.T) {
cloud := GetTestCloud(ctrl)
cloud.unmanagedNodes = sets.New("node0")
cloud.unmanagedNodes = utilsets.NewString("node0")

meta, err := cloud.InstanceMetadata(context.Background(), &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -992,7 +992,7 @@ func TestCloud_InstanceExists(t *testing.T) {
t.Run("should return true when instance is not managed by azure", func(t *testing.T) {
ctx := context.Background()
cloud := GetTestCloud(ctrl)
cloud.unmanagedNodes = sets.New("foo")
cloud.unmanagedNodes = utilsets.NewString("foo")
node := &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Expand Down
Loading

0 comments on commit afcf72f

Please sign in to comment.