From aa7ec34414f84b7a6eefb33de50f9eaddd4880ad Mon Sep 17 00:00:00 2001 From: Nont Date: Thu, 18 Jan 2024 14:28:15 -0600 Subject: [PATCH] Address the code review comments --- pkg/monitor/azure/nsg/nsg.go | 10 +++++----- pkg/monitor/azure/nsg/nsg_test.go | 8 +++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/monitor/azure/nsg/nsg.go b/pkg/monitor/azure/nsg/nsg.go index 9b4be2a75dd..88a05dec66d 100644 --- a/pkg/monitor/azure/nsg/nsg.go +++ b/pkg/monitor/azure/nsg/nsg.go @@ -123,19 +123,19 @@ func (n *NSGMonitor) Monitor(ctx context.Context) []error { workerProfiles, _ := api.GetEnrichedWorkerProfiles(n.oc.Properties) workerSubnets := make([]subnetNSGConfig, 0, len(workerProfiles)) workerPrefixes := make([]netip.Prefix, 0, len(workerProfiles)) - subnetToMonitor := map[string]struct{}{} + // To minimize the possibility of NRP throttling, we only retrieve a subnet's info only once. + subnetsToMonitor := map[string]struct{}{} for _, wp := range workerProfiles { // Customer can configure a machineset with an invalid subnet. // In such case, the subnetID will be empty. - // We do not need to monitor any profiles with 0 machines either. - // To minimize the possibility of throttling, we only get it once. + // Also 0 machine means no worker nodes are there, so no point to monitor the profile. if len(wp.SubnetID) != 0 && wp.Count != 0 { - subnetToMonitor[wp.SubnetID] = struct{}{} + subnetsToMonitor[wp.SubnetID] = struct{}{} } } - for subnetID := range subnetToMonitor { + for subnetID := range subnetsToMonitor { s, err := n.toSubnetConfig(ctx, subnetID) if err != nil { // FP has no access to the subnet diff --git a/pkg/monitor/azure/nsg/nsg_test.go b/pkg/monitor/azure/nsg/nsg_test.go index db8c181ff77..e443eae6159 100644 --- a/pkg/monitor/azure/nsg/nsg_test.go +++ b/pkg/monitor/azure/nsg/nsg_test.go @@ -335,19 +335,17 @@ func TestMonitor(t *testing.T) { workerSubnet1.Properties.NetworkSecurityGroup = &nsg workerSubnet2.Properties.NetworkSecurityGroup = &nsg - _1 := mock.EXPECT(). + mock.EXPECT(). Get(ctx, resourcegroupName, vNetName, masterSubnetName, options). Return(masterSubnet, nil) - _2 := mock.EXPECT(). + mock.EXPECT(). Get(ctx, resourcegroupName, vNetName, workerSubnet1Name, options). Return(workerSubnet1, nil) - _3 := mock.EXPECT(). + mock.EXPECT(). Get(ctx, resourcegroupName, vNetName, workerSubnet2Name, options). Return(workerSubnet2, nil) - - gomock.InOrder(_1, _2, _3) }, }, {