Skip to content

Commit

Permalink
Merge pull request #3290 from lzhecheng/refactor-solve-warning
Browse files Browse the repository at this point in the history
Refactor: Solve go warnings
  • Loading branch information
k8s-ci-robot authored Feb 9, 2023
2 parents 40c848b + aa35568 commit 945c780
Show file tree
Hide file tree
Showing 22 changed files with 136 additions and 143 deletions.
70 changes: 32 additions & 38 deletions pkg/provider/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"net/http"
"os"
"strings"
Expand Down Expand Up @@ -79,11 +78,6 @@ import (
nodemanager "sigs.k8s.io/cloud-provider-azure/pkg/nodemanager"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"

// ensure the newly added package from azure-sdk-for-go is in vendor/
_ "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/containerserviceclient"
// ensure the newly added package from azure-sdk-for-go is in vendor/
_ "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/deploymentclient"

"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -331,17 +325,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.String
// nodeZones is a mapping from Zone to a sets.String of Node's names in the Zone
nodeNames sets.Set[string]
// 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.String
nodeZones map[string]sets.Set[string]
// nodeResourceGroups holds nodes external resource groups
nodeResourceGroups map[string]string
// unmanagedNodes holds a list of nodes not managed by Azure cloud provider.
unmanagedNodes sets.String
unmanagedNodes sets.Set[string]
// excludeLoadBalancerNodes holds a list of nodes that should be excluded from LoadBalancer.
excludeLoadBalancerNodes sets.String
nodePrivateIPs map[string]sets.String
excludeLoadBalancerNodes sets.Set[string]
nodePrivateIPs map[string]sets.Set[string]
// nodeInformerSynced is for determining if the informer has synced.
nodeInformerSynced cache.InformerSynced

Expand Down Expand Up @@ -442,13 +436,13 @@ func (az *Cloud) configSecretMetadata(secretName, secretNamespace, cloudConfigKe

func NewCloudFromSecret(ctx context.Context, clientBuilder cloudprovider.ControllerClientBuilder, secretName, secretNamespace, cloudConfigKey string) (cloudprovider.Interface, error) {
az := &Cloud{
nodeNames: sets.NewString(),
nodeZones: map[string]sets.String{},
nodeNames: sets.New[string](),
nodeZones: map[string]sets.Set[string]{},
nodeResourceGroups: map[string]string{},
unmanagedNodes: sets.NewString(),
unmanagedNodes: sets.New[string](),
routeCIDRs: map[string]string{},
excludeLoadBalancerNodes: sets.NewString(),
nodePrivateIPs: map[string]sets.String{},
excludeLoadBalancerNodes: sets.New[string](),
nodePrivateIPs: map[string]sets.Set[string]{},
}

az.configSecretMetadata(secretName, secretNamespace, cloudConfigKey)
Expand All @@ -474,13 +468,13 @@ func NewCloudWithoutFeatureGates(ctx context.Context, configReader io.Reader, ca
}

az := &Cloud{
nodeNames: sets.NewString(),
nodeZones: map[string]sets.String{},
nodeNames: sets.New[string](),
nodeZones: map[string]sets.Set[string]{},
nodeResourceGroups: map[string]string{},
unmanagedNodes: sets.NewString(),
unmanagedNodes: sets.New[string](),
routeCIDRs: map[string]string{},
excludeLoadBalancerNodes: sets.NewString(),
nodePrivateIPs: map[string]sets.String{},
excludeLoadBalancerNodes: sets.New[string](),
nodePrivateIPs: map[string]sets.Set[string]{},
}

err = az.InitializeCloudFromConfig(ctx, config, false, callFromCCM)
Expand Down Expand Up @@ -527,12 +521,12 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
// The default cloud config type is cloudConfigTypeMerge.
config.CloudConfigType = cloudConfigTypeMerge
} else {
supportedCloudConfigTypes := sets.NewString(
supportedCloudConfigTypes := sets.New(
string(cloudConfigTypeMerge),
string(cloudConfigTypeFile),
string(cloudConfigTypeSecret))
if !supportedCloudConfigTypes.Has(string(config.CloudConfigType)) {
return fmt.Errorf("cloudConfigType %v is not supported, supported values are %v", config.CloudConfigType, supportedCloudConfigTypes.List())
return fmt.Errorf("cloudConfigType %v is not supported, supported values are %v", config.CloudConfigType, supportedCloudConfigTypes.UnsortedList())
}
}

Expand All @@ -541,12 +535,12 @@ func (az *Cloud) InitializeCloudFromConfig(ctx context.Context, config *Config,
strings.EqualFold(config.LoadBalancerBackendPoolConfigurationType, consts.LoadBalancerBackendPoolConfigurationTypePODIP) {
config.LoadBalancerBackendPoolConfigurationType = consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration
} else {
supportedLoadBalancerBackendPoolConfigurationTypes := sets.NewString(
supportedLoadBalancerBackendPoolConfigurationTypes := sets.New(
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration),
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypeNodeIP),
strings.ToLower(consts.LoadBalancerBackendPoolConfigurationTypePODIP))
if !supportedLoadBalancerBackendPoolConfigurationTypes.Has(strings.ToLower(config.LoadBalancerBackendPoolConfigurationType)) {
return fmt.Errorf("loadBalancerBackendPoolConfigurationType %s is not supported, supported values are %v", config.LoadBalancerBackendPoolConfigurationType, supportedLoadBalancerBackendPoolConfigurationTypes.List())
return fmt.Errorf("loadBalancerBackendPoolConfigurationType %s is not supported, supported values are %v", config.LoadBalancerBackendPoolConfigurationType, supportedLoadBalancerBackendPoolConfigurationTypes.UnsortedList())
}
}

Expand Down Expand Up @@ -924,7 +918,7 @@ func ParseConfig(configReader io.Reader) (*Config, error) {
return nil, nil
}

configContents, err := ioutil.ReadAll(configReader)
configContents, err := io.ReadAll(configReader)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1118,7 +1112,7 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
newZone, ok := newNode.ObjectMeta.Labels[consts.LabelFailureDomainBetaZone]
if ok && az.isAvailabilityZone(newZone) {
if az.nodeZones[newZone] == nil {
az.nodeZones[newZone] = sets.NewString()
az.nodeZones[newZone] = sets.New[string]()
}
az.nodeZones[newZone].Insert(newNode.ObjectMeta.Name)
}
Expand Down Expand Up @@ -1163,7 +1157,7 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
// Add to nodePrivateIPs cache
for _, address := range getNodePrivateIPAddresses(newNode) {
if az.nodePrivateIPs[newNode.Name] == nil {
az.nodePrivateIPs[newNode.Name] = sets.NewString()
az.nodePrivateIPs[newNode.Name] = sets.New[string]()
}

klog.V(4).Infof("adding IP address %s of the node %s", address, newNode.Name)
Expand All @@ -1173,7 +1167,7 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
}

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

zones := sets.NewString()
zones := sets.New[string]()
for zone, nodes := range az.nodeZones {
if len(nodes) > 0 {
zones.Insert(zone)
Expand Down Expand Up @@ -1221,7 +1215,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.String, error) {
func (az *Cloud) GetNodeNames() (sets.Set[string], error) {
// Kubelet won't set az.nodeInformerSynced, return nil.
if az.nodeInformerSynced == nil {
return nil, nil
Expand All @@ -1233,14 +1227,14 @@ func (az *Cloud) GetNodeNames() (sets.String, error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetNodeNames")
}

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

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

az.nodeCachesLock.RLock()
Expand All @@ -1249,7 +1243,7 @@ func (az *Cloud) GetResourceGroups() (sets.String, error) {
return nil, fmt.Errorf("node informer is not synced when trying to GetResourceGroups")
}

resourceGroups := sets.NewString(az.ResourceGroup)
resourceGroups := sets.New(az.ResourceGroup)
for _, rg := range az.nodeResourceGroups {
resourceGroups.Insert(rg)
}
Expand All @@ -1258,7 +1252,7 @@ func (az *Cloud) GetResourceGroups() (sets.String, error) {
}

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

return sets.NewString(az.unmanagedNodes.List()...), nil
return sets.New(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 Down
2 changes: 1 addition & 1 deletion pkg/provider/azure_backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func (az *Cloud) ListManagedLBs(service *v1.Service, nodes []*v1.Node, clusterNa
return nil, fmt.Errorf("ListManagedLBs: failed to get agent pool vmSet names: %w", err)
}

agentPoolVMSetNamesSet := sets.NewString()
agentPoolVMSetNamesSet := sets.New[string]()
if agentPoolVMSetNames != nil && len(*agentPoolVMSetNames) > 0 {
for _, vmSetName := range *agentPoolVMSetNames {
klog.V(6).Infof("ListManagedLBs: found agent pool vmSet name %s", vmSetName)
Expand Down
26 changes: 13 additions & 13 deletions pkg/provider/azure_controller_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,22 +811,22 @@ func TestFilteredDetachingDisks(t *testing.T) {

disks := []compute.DataDisk{
{
Name: pointer.StringPtr("DiskName1"),
ToBeDetached: pointer.BoolPtr(false),
Name: pointer.String("DiskName1"),
ToBeDetached: pointer.Bool(false),
ManagedDisk: &compute.ManagedDiskParameters{
ID: pointer.StringPtr("ManagedID"),
ID: pointer.String("ManagedID"),
},
},
{
Name: pointer.StringPtr("DiskName2"),
ToBeDetached: pointer.BoolPtr(true),
Name: pointer.String("DiskName2"),
ToBeDetached: pointer.Bool(true),
},
{
Name: pointer.StringPtr("DiskName3"),
Name: pointer.String("DiskName3"),
ToBeDetached: nil,
},
{
Name: pointer.StringPtr("DiskName4"),
Name: pointer.String("DiskName4"),
ToBeDetached: nil,
},
}
Expand Down Expand Up @@ -1048,21 +1048,21 @@ func TestFilterNonExistingDisks(t *testing.T) {
},
},
{
Name: pointer.StringPtr("DiskName2"),
Name: pointer.String("DiskName2"),
ManagedDisk: &compute.ManagedDiskParameters{
ID: pointer.StringPtr(diskURIPrefix + "DiskName2"),
ID: pointer.String(diskURIPrefix + "DiskName2"),
},
},
{
Name: pointer.StringPtr("DiskName3"),
Name: pointer.String("DiskName3"),
ManagedDisk: &compute.ManagedDiskParameters{
ID: pointer.StringPtr(diskURIPrefix + "DiskName3"),
ID: pointer.String(diskURIPrefix + "DiskName3"),
},
},
{
Name: pointer.StringPtr("DiskName4"),
Name: pointer.String("DiskName4"),
ManagedDisk: &compute.ManagedDiskParameters{
ID: pointer.StringPtr(diskURIPrefix + "DiskName4"),
ID: pointer.String(diskURIPrefix + "DiskName4"),
},
},
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/provider/azure_fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
VMType: consts.VMTypeStandard,
LoadBalancerBackendPoolConfigurationType: consts.LoadBalancerBackendPoolConfigurationTypeNodeIPConfiguration,
},
nodeZones: map[string]sets.String{},
nodeZones: map[string]sets.Set[string]{},
nodeInformerSynced: func() bool { return true },
nodeResourceGroups: map[string]string{},
unmanagedNodes: sets.NewString(),
excludeLoadBalancerNodes: sets.NewString(),
nodePrivateIPs: map[string]sets.String{},
unmanagedNodes: sets.New[string](),
excludeLoadBalancerNodes: sets.New[string](),
nodePrivateIPs: map[string]sets.Set[string]{},
routeCIDRs: map[string]string{},
eventRecorder: &record.FakeRecorder{},
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/provider/azure_instance_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package provider
import (
"encoding/json"
"fmt"
"io/ioutil"
"io"
"net/http"

"k8s.io/klog/v2"
Expand Down Expand Up @@ -187,7 +187,7 @@ func (ims *InstanceMetadataService) getInstanceMetadata(key string) (*InstanceMe
return nil, fmt.Errorf("failure of getting instance metadata with response %q", resp.Status)
}

data, err := ioutil.ReadAll(resp.Body)
data, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -225,7 +225,7 @@ func (ims *InstanceMetadataService) getLoadBalancerMetadata() (*LoadBalancerMeta
return nil, fmt.Errorf("failure of getting loadbalancer metadata with response %q", resp.Status)
}

data, err := ioutil.ReadAll(resp.Body)
data, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/azure_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ func (az *Cloud) reconcileSharedLoadBalancer(service *v1.Service, clusterName st
return existingLBs, nil
}

lbNamesToBeDeleted := sets.NewString()
lbNamesToBeDeleted := sets.New[string]()
// delete unwanted LBs
for _, lb := range existingLBs {
klog.V(4).Infof("reconcileSharedLoadBalancer: checking LB %s", pointer.StringDeref(lb.Name, ""))
Expand Down Expand Up @@ -3203,7 +3203,7 @@ func (az *Cloud) safeDeletePublicIP(service *v1.Service, pipResourceGroup string

// Check whether there are still load balancer rules referring to it.
if len(referencedLBRules) > 0 {
referencedLBRuleIDs := sets.NewString()
referencedLBRuleIDs := sets.New[string]()
for _, refer := range referencedLBRules {
referencedLBRuleIDs.Insert(pointer.StringDeref(refer.ID, ""))
}
Expand Down
Loading

0 comments on commit 945c780

Please sign in to comment.