Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release-1.24] chore: remove orphaned nic from lb backend pool if the corresponding … #2845

Merged
merged 2 commits into from Dec 2, 2022
Merged

[release-1.24] chore: remove orphaned nic from lb backend pool if the corresponding … #2845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28302a1
chore: remove orphaned nic from lb backend pool if the corresponding …
nilo19 Nov 28, 2022
b7b1e30
chore: enrich logs when decoupling vmss from the lb
nilo19 Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/provider/azure_loadbalancer_backendpool.go
View file
Open in desktop
Expand Up @@ -185,8 +185,13 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
for _, ipConf := range *bp.BackendIPConfigurations {
ipConfID := to.String(ipConf.ID)
nodeName, _, err := bc.VMSet.GetNodeNameByIPConfigurationID(ipConfID)
if err != nil && !errors.Is(err, cloudprovider.InstanceNotFound) {
return false, false, err
if err != nil {
if errors.Is(err, cloudprovider.InstanceNotFound) {
klog.V(2).Infof("bc.ReconcileBackendPools for service (%s): vm not found for ipConfID %s", serviceName, ipConfID)
backendIPConfigurationsToBeDeleted = append(backendIPConfigurationsToBeDeleted, ipConf)
} else {
return false, false, err
}
}

// If a node is not supposed to be included in the LB, it
Expand Down
34 changes: 34 additions & 0 deletions pkg/provider/azure_loadbalancer_backendpool_test.go
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package provider

import (
"errors"
"fmt"
"strings"
"testing"
Expand All @@ -30,6 +31,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/fake"
cloudprovider "k8s.io/cloud-provider"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/loadbalancerclient/mockloadbalancerclient"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
Expand Down Expand Up @@ -295,6 +297,38 @@ func TestReconcileBackendPoolsNodeIPConfig(t *testing.T) {
assert.True(t, changed)
}

func TestReconcileBackendPoolsNodeIPConfigRemoveIPConfig(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

lb := buildDefaultTestLB(testClusterName, []string{
"/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool1-00000000-nic-1/ipConfigurations/ipconfig1",
"/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool2-00000000-nic-1/ipConfigurations/ipconfig1",
"/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool3-00000000-nic-1/ipConfigurations/ipconfig1",
})

mockVMSet := NewMockVMSet(ctrl)
mockVMSet.EXPECT().GetNodeNameByIPConfigurationID("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool1-00000000-nic-1/ipConfigurations/ipconfig1").Return("k8s-agentpool1-00000000", "", nil)
mockVMSet.EXPECT().GetNodeNameByIPConfigurationID("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool2-00000000-nic-1/ipConfigurations/ipconfig1").Return("k8s-agentpool2-00000000", "", nil)
mockVMSet.EXPECT().GetNodeNameByIPConfigurationID("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool3-00000000-nic-1/ipConfigurations/ipconfig1").Return("", "", cloudprovider.InstanceNotFound)
mockVMSet.EXPECT().EnsureBackendPoolDeleted(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil)
mockVMSet.EXPECT().GetPrimaryVMSetName().Return("k8s-agentpool1-00000000").Times(2)

az := GetTestCloud(ctrl)
az.VMSet = mockVMSet
az.nodeInformerSynced = func() bool { return true }
az.excludeLoadBalancerNodes = sets.NewString("k8s-agentpool1-00000000")

bc := newBackendPoolTypeNodeIPConfig(az)
svc := getTestService("test", v1.ProtocolTCP, nil, false, 80)
_, _, err := bc.ReconcileBackendPools(testClusterName, &svc, &lb)
assert.NoError(t, err)

mockVMSet.EXPECT().GetNodeNameByIPConfigurationID("/subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/k8s-agentpool1-00000000-nic-1/ipConfigurations/ipconfig1").Return("k8s-agentpool1-00000000", "", errors.New("error"))
_, _, err = bc.ReconcileBackendPools(testClusterName, &svc, &lb)
assert.Equal(t, "error", err.Error())
}

func TestReconcileBackendPoolsNodeIPConfigPreConfigured(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down
5 changes: 4 additions & 1 deletion pkg/provider/azure_vmss.go
View file
Open in desktop
Expand Up @@ -1519,8 +1519,9 @@ func getScaleSetAndResourceGroupNameByIPConfigurationID(ipConfigurationID string
}

func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName string) error {
vmssNamesMap := make(map[string]bool)
klog.V(2).Infof("ensureBackendPoolDeletedFromVMSS: vmSetName (%s), backendPoolID (%s)", vmSetName, backendPoolID)

vmssNamesMap := make(map[string]bool)
// the standard load balancer supports multiple vmss in its backend while the basic sku doesn't
if ss.useStandardLoadBalancer() {
cachedUniform, err := ss.vmssCache.Get(consts.VMSSKey, azcache.CacheReadTypeDefault)
Expand All @@ -1538,6 +1539,7 @@ func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName st
} else if v, ok := value.(*compute.VirtualMachineScaleSet); ok {
vmss = v
}
klog.V(2).Infof("ensureBackendPoolDeletedFromVmss: vmss (%s)", to.String(vmss.Name))

// When vmss is being deleted, CreateOrUpdate API would report "the vmss is being deleted" error.
// Since it is being deleted, we shouldn't send more CreateOrUpdate requests for it.
Expand Down Expand Up @@ -1571,6 +1573,7 @@ func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName st
loadBalancerBackendAddressPools = *primaryIPConfig.LoadBalancerBackendAddressPools
}
for _, loadBalancerBackendAddressPool := range loadBalancerBackendAddressPools {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: loadBalancerBackendAddressPool (%s) on vmss (%s)", to.String(loadBalancerBackendAddressPool.ID), to.String(vmss.Name))
if strings.EqualFold(to.String(loadBalancerBackendAddressPool.ID), backendPoolID) {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: found vmss %s with backend pool %s, removing it", to.String(vmss.Name), backendPoolID)
vmssNamesMap[to.String(vmss.Name)] = true
Expand Down