Skip to content

Commit 7de69b1

Browse files
NihaNallappagariNihaNallappagari
committed
remove unused code
Address PR comments remove unnecessary logger update synhost skip test scenario fix linting error updated lint error fix linting error . update synhost skip test scenario fix linting error updated lint error fix linting error .
1 parent d0e56de commit 7de69b1

File tree

4 files changed

+47
-55
lines changed

4 files changed

+47
-55
lines changed

‎azure-ipam/ipconfig/ipconfig.go‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"net/netip"
88

99
"github.com/Azure/azure-container-networking/cns"
10-
"github.com/Azure/azure-container-networking/cns/logger"
1110
cniSkel "github.com/containernetworking/cni/pkg/skel"
1211
cniTypes "github.com/containernetworking/cni/pkg/types"
1312
"github.com/pkg/errors"
@@ -83,30 +82,18 @@ func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, *[]net.
8382
}
8483
podIPNets[i] = podIPNet
8584

85+
var gatewayStr string
8686
if podIPNet.Addr().Is4() {
87-
gatewayIP, err : = net.ParseIP(resp.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPAddress)
87+
gatewayStr = podInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress
8888
} else if podIPNet.Addr().Is6() {
89-
gatewayIP, err : = net.ParseIP(resp.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPv6Address)
89+
gatewayStr = podInfo.NetworkContainerPrimaryIPConfig.GatewayIPv6Address
9090
}
9191

92-
if gatewayIP != nil {
93-
gatewaysIPs[i] = gatewayIP
94-
} else {
95-
logger.Printf("gatewayIP is nil or gateway ip parse failed for pod ip %s", resp.PodIPInfo[i].PodIPConfig.IPAddress)
92+
gatewayIP := net.ParseIP(gatewayStr)
93+
if gatewayIP == nil {
94+
return nil, nil, errors.Errorf("failed to parse gateway IP %q for pod ip %s", gatewayStr, podInfo.PodIPConfig.IPAddress)
9695
}
97-
98-
var gatewayStr string
99-
if podIPNet.Addr().Is4() {
100-
gatewayStr = podInfo.NetworkContainerPrimaryIPConfig.GatewayIPAddress
101-
} else if podIPNet.Addr().Is6() {
102-
gatewayStr = podInfo.NetworkContainerPrimaryIPConfig.GatewayIPv6Address
103-
}
104-
105-
gatewayIP := net.ParseIP(gatewayStr)
106-
if gatewayIP == nil {
107-
return nil, nil, errors.Errorf("failed to parse gateway IP %q for pod ip %s", gatewayStr, podInfo.PodIPConfig.IPAddress)
108-
}
109-
gatewaysIPs[i] = gatewayIP
96+
gatewaysIPs[i] = gatewayIP
11097
}
11198

11299
return &podIPNets, &gatewaysIPs, nil

‎cns/restserver/internalapi.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo
195195
// For prefix-on-NIC SwiftV2 NIC scenario, the NC details is published to different path in NMAgent and call to NMA here creates error. So Skipping nma check, synchost version logic.
196196
ncStatus := service.state.ContainerStatus[idx]
197197
if mac := ncStatus.CreateNetworkContainerRequest.NetworkInterfaceInfo.MACAddress; mac != "" {
198-
logger.Printf("NC %s has MAC address for prefix on nic swiftv2 scenario, skipping syncHostNCVersion", ncStatus.ID)
198+
logger.Printf("NC %s has MAC address for prefix on nic swiftv2 prefix on NIC scenario, skipping syncHostNCVersion", ncStatus.ID)
199199
continue
200200
}
201201
// Will open a separate PR to convert all the NC version related variable to int. Change from string to int is a pain.
@@ -627,7 +627,6 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
627627
} else {
628628
logger.Errorf(returnMessage)
629629
}
630-
631630
if service.Options[common.OptProgramSNATIPTables] == true {
632631
returnCode, returnMessage = service.programSNATRules(req)
633632
if returnCode != 0 {

‎cns/restserver/internalapi_test.go‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const (
4242
batchSize = 10
4343
initPoolSize = 10
4444
ncID = "6a07155a-32d7-49af-872f-1e70ee366dc0"
45+
macAddress = "00:11:22:33:44:55"
4546
)
4647

4748
var dnsservers = []string{"8.8.8.8", "8.8.4.4"}
@@ -315,38 +316,26 @@ func TestPendingIPsGotUpdatedWhenSyncHostNCVersion(t *testing.T) {
315316
func TestSyncHostNCVersion_SkipsNCVersionForPrefixOnNICSwiftV2(t *testing.T) {
316317
orchestratorTypes := []string{cns.Kubernetes, cns.KubernetesCRD}
317318
for _, orchestratorType := range orchestratorTypes {
318-
orchestratorType := orchestratorType
319319
t.Run(orchestratorType, func(t *testing.T) {
320-
restartService()
321-
setEnv(t)
322-
setOrchestratorTypeInternal(orchestratorType)
323-
324-
ncID := "swiftv2-ncid"
325-
svc.state.ContainerStatus = map[string]containerstatus{
326-
ncID: {
327-
ID: ncID,
328-
HostVersion: "2",
329-
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
330-
NetworkContainerid: ncID,
331-
Version: "1",
332-
NetworkInterfaceInfo: cns.NetworkInterfaceInfo{
333-
MACAddress: "00:11:22:33:44:55",
334-
},
335-
},
336-
},
337-
}
338-
339-
// Should not error, and should not attempt version check
320+
req := createNCReqeustForSyncHostNCVersion(t, macAddress)
321+
containerStatus := svc.state.ContainerStatus[req.NetworkContainerid]
322+
//// HostVersion -1 and NC Version 0 and should remain unchanged, should not error
340323
svc.SyncHostNCVersion(context.Background(), orchestratorType)
324+
341325
// HostVersion and Version should remain unchanged
342-
containerStatus := svc.state.ContainerStatus[ncID]
343-
assert.Equal(t, "2", containerStatus.HostVersion)
344-
assert.Equal(t, "1", containerStatus.CreateNetworkContainerRequest.Version)
326+
assert.Equal(t, "-1", containerStatus.HostVersion)
327+
assert.Equal(t, "0", containerStatus.CreateNetworkContainerRequest.Version)
345328
})
346329
}
347330
}
348331

349-
func createNCReqeustForSyncHostNCVersion(t *testing.T) cns.CreateNetworkContainerRequest {
332+
func createNCReqeustForSyncHostNCVersion(t *testing.T, macAddresses ...string) cns.CreateNetworkContainerRequest {
333+
var macAddress string
334+
if len(macAddresses) > 0 {
335+
macAddress = macAddresses[0]
336+
} else {
337+
macAddress = ""
338+
}
350339
restartService()
351340
setEnv(t)
352341
setOrchestratorTypeInternal(cns.KubernetesCRD)
@@ -360,7 +349,7 @@ func createNCReqeustForSyncHostNCVersion(t *testing.T) cns.CreateNetworkContaine
360349
secIPConfig := newSecondaryIPConfig(ipAddress, ncVersion)
361350
ipID := uuid.New()
362351
secondaryIPConfigs[ipID.String()] = secIPConfig
363-
req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion))
352+
req := createNCReqInternal(t, secondaryIPConfigs, ncID, strconv.Itoa(ncVersion), macAddress)
364353
return req
365354
}
366355

@@ -939,7 +928,14 @@ func validateNetworkRequest(t *testing.T, req cns.CreateNetworkContainerRequest)
939928
}
940929
}
941930

942-
func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConfig, ncID, ncVersion string) *cns.CreateNetworkContainerRequest {
931+
func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConfig, ncID, ncVersion string, macAddress ...string) *cns.CreateNetworkContainerRequest {
932+
var mac string
933+
if len(macAddress) > 0 {
934+
mac = macAddress[0]
935+
} else {
936+
mac = ""
937+
}
938+
943939
var ipConfig cns.IPConfiguration
944940
ipConfig.DNSServers = dnsservers
945941
ipConfig.GatewayIPAddress = gatewayIP
@@ -953,6 +949,9 @@ func generateNetworkContainerRequest(secondaryIps map[string]cns.SecondaryIPConf
953949
NetworkContainerid: ncID,
954950
IPConfiguration: ipConfig,
955951
Version: ncVersion,
952+
NetworkInterfaceInfo: cns.NetworkInterfaceInfo{
953+
MACAddress: mac,
954+
},
956955
}
957956

958957
ncVersionInInt, _ := strconv.Atoi(ncVersion)
@@ -1079,8 +1078,14 @@ func validateIPAMStateAfterReconcile(t *testing.T, ncReqs []*cns.CreateNetworkCo
10791078
}
10801079
}
10811080

1082-
func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncID, ncVersion string) cns.CreateNetworkContainerRequest {
1083-
req := generateNetworkContainerRequest(secondaryIPConfigs, ncID, ncVersion)
1081+
func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.SecondaryIPConfig, ncID, ncVersion string, macAddresses ...string) cns.CreateNetworkContainerRequest {
1082+
var macAddress string
1083+
if len(macAddresses) > 0 {
1084+
macAddress = macAddresses[0]
1085+
} else {
1086+
macAddress = ""
1087+
}
1088+
req := generateNetworkContainerRequest(secondaryIPConfigs, ncID, ncVersion, macAddress)
10841089
returnCode := svc.CreateOrUpdateNetworkContainerInternal(req)
10851090
if returnCode != 0 {
10861091
t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode)

‎cns/restserver/ipam.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"net"
1010
"net/http"
11+
"net/netip"
1112
"strconv"
1213
"strings"
1314

@@ -1009,14 +1010,14 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([
10091010
break
10101011
}
10111012

1012-
ip := net.ParseIP(secIPConfig.IPAddress)
1013-
if ip == nil {
1013+
addr, err := netip.ParseAddr(secIPConfig.IPAddress)
1014+
if err != nil {
10141015
continue
10151016
}
10161017

1017-
if ip.To4() != nil {
1018+
if addr.Is4() {
10181019
ncIPFamilies[cns.IPv4] = struct{}{}
1019-
} else {
1020+
} else if addr.Is6() {
10201021
ncIPFamilies[cns.IPv6] = struct{}{}
10211022
}
10221023
}

0 commit comments

Comments
 (0)