[CNI] zap logger for platform package (#2233)

* zap logger for platform package
Azure · Oct 13, 2023 · ae37d40 · ae37d40
1 parent a3ec127
commit ae37d40
Show file tree

Hide file tree

Showing 29 changed files with 204 additions and 92 deletions.
diff --git a/aitelemetry/telemetrywrapper_test.go b/aitelemetry/telemetrywrapper_test.go
@@ -29,7 +29,7 @@ func TestMain(m *testing.M) {
 		fmt.Printf("TestST LogDir configuration succeeded\n")
 	}
 
-	p := platform.NewExecClient()
+	p := platform.NewExecClient(nil)
 	if runtime.GOOS == "linux" {
 		//nolint:errcheck // initial test setup
 		p.ExecuteCommand("cp metadata_test.json /tmp/azuremetadata.json")

diff --git a/cni/network/network.go b/cni/network/network.go
@@ -117,7 +117,7 @@ func NewPlugin(name string,
 
 	nl := netlink.NewNetlink()
 	// Setup network manager.
-	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
+	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(logger), &netio.NetIO{})
 	if err != nil {
 		return nil, err
 	}

diff --git a/cni/network/plugin/main.go b/cni/network/plugin/main.go
@@ -157,6 +157,7 @@ func rootExecute() error {
 			InterfaceDetails: telemetry.InterfaceInfo{},
 			BridgeDetails:    telemetry.BridgeInfo{},
 			Version:          version,
+			Logger:           logger,
 		},
 	}
 
@@ -182,7 +183,8 @@ func rootExecute() error {
 		cniReport.GetReport(pluginName, version, ipamQueryURL)
 
 		var upTime time.Time
-		upTime, err = platform.GetLastRebootTime()
+		p := platform.NewExecClient(logger)
+		upTime, err = p.GetLastRebootTime()
 		if err == nil {
 			cniReport.VMUptime = upTime.Format("2006-01-02 15:04:05")
 		}

diff --git a/cni/telemetry/service/telemetrymain.go b/cni/telemetry/service/telemetrymain.go
@@ -143,7 +143,6 @@ func main() {
 	tbtemp.Cleanup(telemetry.FdName)
 
 	tb = telemetry.NewTelemetryBuffer(logger)
-
 	for {
 		logger.Info("Starting telemetry server")
 		err = tb.StartServer()

diff --git a/cnm/network/network.go b/cnm/network/network.go
@@ -53,7 +53,7 @@ func NewPlugin(config *common.PluginConfig) (NetPlugin, error) {
 
 	nl := netlink.NewNetlink()
 	// Setup network manager.
-	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
+	nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{})
 	if err != nil {
 		return nil, err
 	}

diff --git a/cnms/service/networkmonitor.go b/cnms/service/networkmonitor.go
@@ -157,7 +157,7 @@ func main() {
 		}
 
 		nl := netlink.NewNetlink()
-		nm, err := network.NewNetworkManager(nl, platform.NewExecClient(), &netio.NetIO{})
+		nm, err := network.NewNetworkManager(nl, platform.NewExecClient(nil), &netio.NetIO{})
 		if err != nil {
 			log.Printf("[monitor] Failed while creating network manager")
 			return

diff --git a/cns/dockerclient/dockerclient.go b/cns/dockerclient/dockerclient.go
@@ -148,7 +148,7 @@ func (c *Client) CreateNetwork(networkName string, nicInfo *wireserver.Interface
 
 // DeleteNetwork creates a network using docker network create.
 func (c *Client) DeleteNetwork(networkName string) error {
-	p := platform.NewExecClient()
+	p := platform.NewExecClient(nil)
 	logger.Printf("[Azure CNS] DeleteNetwork")
 
 	url := c.connectionURL + inspectNetworkPath + networkName

diff --git a/cns/restserver/util.go b/cns/restserver/util.go
@@ -539,8 +539,8 @@ func (service *HTTPRestService) restoreNetworkState() error {
 
 	if err == nil {
 		logger.Printf("[Azure CNS] Store timestamp is %v.", modTime)
-
-		rebootTime, err := platform.GetLastRebootTime()
+		p := platform.NewExecClient(nil)
+		rebootTime, err := p.GetLastRebootTime()
 		if err == nil && rebootTime.After(modTime) {
 			logger.Printf("[Azure CNS] reboot time %v mod time %v", rebootTime, modTime)
 			rebooted = true

diff --git a/ebtables/ebtables.go b/ebtables/ebtables.go
@@ -129,7 +129,7 @@ func GetEbtableRules(tableName, chainName string) ([]string, error) {
 		inChain bool
 		rules   []string
 	)
-	p := platform.NewExecClient()
+	p := platform.NewExecClient(nil)
 	command := fmt.Sprintf(
 		"ebtables -t %s -L %s --Lmac2",
 		tableName, chainName)
@@ -226,7 +226,7 @@ func EbTableRuleExists(tableName, chainName, matchSet string) (bool, error) {
 
 // runEbCmd runs an EB rule command.
 func runEbCmd(table, action, chain, rule string) error {
-	p := platform.NewExecClient()
+	p := platform.NewExecClient(nil)
 	command := fmt.Sprintf("ebtables -t %s %s %s %s", table, action, chain, rule)
 	_, err := p.ExecuteCommand(command)
 

diff --git a/ipam/manager.go b/ipam/manager.go
@@ -134,7 +134,8 @@ func (am *addressManager) restore(rehydrateIpamInfoOnReboot bool) error {
 		// Check if the VM is rebooted.
 		modTime, err := am.store.GetModificationTime()
 		if err == nil {
-			rebootTime, err := platform.GetLastRebootTime()
+			p := platform.NewExecClient(nil)
+			rebootTime, err := p.GetLastRebootTime()
 			log.Printf("[ipam] reboot time %v store mod time %v", rebootTime, modTime)
 
 			if err == nil && rebootTime.After(modTime) {

diff --git a/ipam/manager_test.go b/ipam/manager_test.go
@@ -229,7 +229,8 @@ var _ = Describe("Test Manager", func() {
 				am := &addressManager{
 					AddrSpaces: make(map[string]*addressSpace),
 				}
-				timeReboot, _ := platform.GetLastRebootTime()
+				p := platform.NewExecClient(nil)
+				timeReboot, _ := p.GetLastRebootTime()
 				am.store = &testutils.KeyValueStoreMock{
 					ModificationTime: timeReboot.Add(time.Hour),
 				}

diff --git a/iptables/iptables.go b/iptables/iptables.go
@@ -5,10 +5,13 @@ package iptables
 import (
 	"fmt"
 
-	"github.com/Azure/azure-container-networking/log"
+	"github.com/Azure/azure-container-networking/cni/log"
 	"github.com/Azure/azure-container-networking/platform"
+	"go.uber.org/zap"
 )
 
+var logger = log.CNILogger.With(zap.String("component", "cni-iptables"))
+
 // cni iptable chains
 const (
 	CNIInputChain  = "AZURECNIINPUT"
@@ -88,7 +91,7 @@ type IPTableEntry struct {
 func RunCmd(version, params string) error {
 	var cmd string
 
-	p := platform.NewExecClient()
+	p := platform.NewExecClient(logger)
 	iptCmd := iptables
 	if version == V6 {
 		iptCmd = ip6tables
@@ -132,7 +135,7 @@ func CreateChain(version, tableName, chainName string) error {
 		cmd := GetCreateChainCmd(version, tableName, chainName)
 		err = RunCmd(version, cmd.Params)
 	} else {
-		log.Printf("%s Chain exists in table %s", chainName, tableName)
+		logger.Info("Chain exists in table", zap.String("chainName", chainName), zap.String("tableName", tableName))
 	}
 
 	return err
@@ -157,7 +160,7 @@ func GetInsertIptableRuleCmd(version, tableName, chainName, match, target string
 // Insert iptable rule at beginning of iptable chain
 func InsertIptableRule(version, tableName, chainName, match, target string) error {
 	if RuleExists(version, tableName, chainName, match, target) {
-		log.Printf("Rule already exists")
+		logger.Info("Rule already exists")
 		return nil
 	}
 
@@ -175,7 +178,7 @@ func GetAppendIptableRuleCmd(version, tableName, chainName, match, target string
 // Append iptable rule at end of iptable chain
 func AppendIptableRule(version, tableName, chainName, match, target string) error {
 	if RuleExists(version, tableName, chainName, match, target) {
-		log.Printf("Rule already exists")
+		logger.Info("Rule already exists")
 		return nil
 	}
 

diff --git a/network/endpoint_windows.go b/network/endpoint_windows.go
@@ -64,7 +64,7 @@ func ConstructEndpointID(containerID string, netNsPath string, ifName string) (s
 }
 
 // newEndpointImpl creates a new endpoint in the network.
-func (nw *network) newEndpointImpl(cli apipaClient, _ netlink.NetlinkInterface, _ platform.ExecClient, _ netio.NetIOInterface, _ EndpointClient, epInfo *EndpointInfo) (*endpoint, error) {
+func (nw *network) newEndpointImpl(cli apipaClient, _ netlink.NetlinkInterface, plc platform.ExecClient, _ netio.NetIOInterface, _ EndpointClient, epInfo *EndpointInfo) (*endpoint, error) {
 	if useHnsV2, err := UseHnsV2(epInfo.NetNsPath); useHnsV2 {
 		if err != nil {
 			return nil, err
@@ -73,11 +73,11 @@ func (nw *network) newEndpointImpl(cli apipaClient, _ netlink.NetlinkInterface,
 		return nw.newEndpointImplHnsV2(cli, epInfo)
 	}
 
-	return nw.newEndpointImplHnsV1(epInfo)
+	return nw.newEndpointImplHnsV1(epInfo, plc)
 }
 
 // newEndpointImplHnsV1 creates a new endpoint in the network using HnsV1
-func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo) (*endpoint, error) {
+func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo, plc platform.ExecClient) (*endpoint, error) {
 	var vlanid int
 
 	if epInfo.Data != nil {
@@ -141,7 +141,7 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo) (*endpoint, error)
 	}
 
 	// add ipv6 neighbor entry for gateway IP to default mac in container
-	if err := nw.addIPv6NeighborEntryForGateway(epInfo); err != nil {
+	if err := nw.addIPv6NeighborEntryForGateway(epInfo, plc); err != nil {
 		return nil, err
 	}
 
@@ -169,7 +169,7 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo) (*endpoint, error)
 	return ep, nil
 }
 
-func (nw *network) addIPv6NeighborEntryForGateway(epInfo *EndpointInfo) error {
+func (nw *network) addIPv6NeighborEntryForGateway(epInfo *EndpointInfo, plc platform.ExecClient) error {
 	var (
 		err error
 		out string
@@ -183,7 +183,8 @@ func (nw *network) addIPv6NeighborEntryForGateway(epInfo *EndpointInfo) error {
 		// run powershell cmd to set neighbor entry for gw ip to 12-34-56-78-9a-bc
 		cmd := fmt.Sprintf("New-NetNeighbor -IPAddress %s -InterfaceAlias \"%s (%s)\" -LinkLayerAddress \"%s\"",
 			nw.Subnets[1].Gateway.String(), containerIfNamePrefix, epInfo.Id, defaultGwMac)
-		if out, err = platform.ExecutePowershellCommand(cmd); err != nil {
+
+		if out, err = plc.ExecutePowershellCommand(cmd); err != nil {
 			logger.Error("Adding ipv6 gw neigh entry failed", zap.Any("out", out), zap.Error(err))
 			return err
 		}

diff --git a/network/endpoint_windows_test.go b/network/endpoint_windows_test.go
@@ -159,7 +159,7 @@ func TestCreateEndpointImplHnsv1Timeout(t *testing.T) {
 		},
 		MacAddress: net.HardwareAddr("00:00:5e:00:53:01"),
 	}
-	_, err := nw.newEndpointImplHnsV1(epInfo)
+	_, err := nw.newEndpointImplHnsV1(epInfo, nil)
 
 	if err == nil {
 		t.Fatal("Failed to timeout HNS calls for creating endpoint")
@@ -186,7 +186,7 @@ func TestDeleteEndpointImplHnsv1Timeout(t *testing.T) {
 		},
 		MacAddress: net.HardwareAddr("00:00:5e:00:53:01"),
 	}
-	endpoint, err := nw.newEndpointImplHnsV1(epInfo)
+	endpoint, err := nw.newEndpointImplHnsV1(epInfo, nil)
 	if err != nil {
 		fmt.Printf("+%v", err)
 		t.Fatal(err)

diff --git a/network/manager.go b/network/manager.go
@@ -154,12 +154,12 @@ func (nm *networkManager) restore(isRehydrationRequired bool) error {
 	if isRehydrationRequired {
 		modTime, err := nm.store.GetModificationTime()
 		if err == nil {
-			rebootTime, err := platform.GetLastRebootTime()
+			rebootTime, err := nm.plClient.GetLastRebootTime()
 			logger.Info("reboot time, store mod time", zap.Any("rebootTime", rebootTime), zap.Any("modTime", modTime))
 			if err == nil && rebootTime.After(modTime) {
 				logger.Info("Detected Reboot")
 				rebooted = true
-				if clearNwConfig, err := platform.ClearNetworkConfiguration(); clearNwConfig {
+				if clearNwConfig, err := nm.plClient.ClearNetworkConfiguration(); clearNwConfig {
 					if err != nil {
 						logger.Error("Failed to clear network configuration", zap.Error(err))
 						return err

diff --git a/network/network_linux.go b/network/network_linux.go
@@ -243,12 +243,11 @@ func isGreaterOrEqaulUbuntuVersion(versionToMatch int) bool {
 	return false
 }
 
-func readDnsInfo(ifName string) (DNSInfo, error) {
+func (nm *networkManager) readDNSInfo(ifName string) (DNSInfo, error) {
 	var dnsInfo DNSInfo
 
-	p := platform.NewExecClient()
 	cmd := fmt.Sprintf("systemd-resolve --status %s", ifName)
-	out, err := p.ExecuteCommand(cmd)
+	out, err := nm.plClient.ExecuteCommand(cmd)
 	if err != nil {
 		return dnsInfo, err
 	}
@@ -289,8 +288,8 @@ func readDnsInfo(ifName string) (DNSInfo, error) {
 	return dnsInfo, nil
 }
 
-func saveDnsConfig(extIf *externalInterface) error {
-	dnsInfo, err := readDnsInfo(extIf.Name)
+func (nm *networkManager) saveDNSConfig(extIf *externalInterface) error {
+	dnsInfo, err := nm.readDNSInfo(extIf.Name)
 	if err != nil || len(dnsInfo.Servers) == 0 || dnsInfo.Suffix == "" {
 		logger.Info("Failed to read dns info from interface", zap.Any("dnsInfo", dnsInfo), zap.String("extIfName", extIf.Name),
 			zap.Error(err))
@@ -332,12 +331,11 @@ func (nm *networkManager) applyIPConfig(extIf *externalInterface, targetIf *net.
 	return nil
 }
 
-func applyDnsConfig(extIf *externalInterface, ifName string) error {
+func (nm *networkManager) applyDNSConfig(extIf *externalInterface, ifName string) error {
 	var (
 		setDnsList string
 		err        error
 	)
-	p := platform.NewExecClient()
 
 	if extIf != nil {
 		for _, server := range extIf.DNSInfo.Servers {
@@ -352,15 +350,15 @@ func applyDnsConfig(extIf *externalInterface, ifName string) error {
 
 		if setDnsList != "" {
 			cmd := fmt.Sprintf("systemd-resolve --interface=%s%s", ifName, setDnsList)
-			_, err = p.ExecuteCommand(cmd)
+			_, err = nm.plClient.ExecuteCommand(cmd)
 			if err != nil {
 				return err
 			}
 		}
 
 		if extIf.DNSInfo.Suffix != "" {
 			cmd := fmt.Sprintf("systemd-resolve --interface=%s --set-domain=%s", ifName, extIf.DNSInfo.Suffix)
-			_, err = p.ExecuteCommand(cmd)
+			_, err = nm.plClient.ExecuteCommand(cmd)
 		}
 
 	}
@@ -443,12 +441,11 @@ func (nm *networkManager) connectExternalInterface(extIf *externalInterface, nwI
 	isGreaterOrEqualUbuntu17 := isGreaterOrEqaulUbuntuVersion(ubuntuVersion17)
 	isSystemdResolvedActive := false
 	if isGreaterOrEqualUbuntu17 {
-		p := platform.NewExecClient()
 		// Don't copy dns servers if systemd-resolved isn't available
-		if _, cmderr := p.ExecuteCommand("systemctl status systemd-resolved"); cmderr == nil {
+		if _, cmderr := nm.plClient.ExecuteCommand("systemctl status systemd-resolved"); cmderr == nil {
 			isSystemdResolvedActive = true
 			logger.Info("Saving dns config from", zap.String("Name", hostIf.Name))
-			if err = saveDnsConfig(extIf); err != nil {
+			if err = nm.saveDNSConfig(extIf); err != nil {
 				logger.Error("Failed to save dns config", zap.Error(err))
 				return err
 			}
@@ -506,7 +503,7 @@ func (nm *networkManager) connectExternalInterface(extIf *externalInterface, nwI
 	if isGreaterOrEqualUbuntu17 && isSystemdResolvedActive {
 		logger.Info("Applying dns config on", zap.String("bridgeName", bridgeName))
 
-		if err = applyDnsConfig(extIf, bridgeName); err != nil {
+		if err = nm.applyDNSConfig(extIf, bridgeName); err != nil {
 			logger.Error("Failed to apply DNS configuration with", zap.Error(err))
 			return err
 		}

diff --git a/ovsctl/ovsctl.go b/ovsctl/ovsctl.go
@@ -58,7 +58,7 @@ type Ovsctl struct {
 }
 
 func NewOvsctl() Ovsctl {
-	return Ovsctl{execcli: platform.NewExecClient()}
+	return Ovsctl{execcli: platform.NewExecClient(logger)}
 }
 
 func (o Ovsctl) CreateOVSBridge(bridgeName string) error {

diff --git a/platform/mockexec.go b/platform/mockexec.go
@@ -1,6 +1,9 @@
 package platform
 
-import "errors"
+import (
+	"errors"
+	"time"
+)
 
 type MockExecClient struct {
 	returnError    bool
@@ -33,3 +36,19 @@ func (e *MockExecClient) ExecuteCommand(cmd string) (string, error) {
 func (e *MockExecClient) SetExecCommand(fn execCommandValidator) {
 	e.setExecCommand = fn
 }
+
+func (e *MockExecClient) ClearNetworkConfiguration() (bool, error) {
+	return true, nil
+}
+
+func (e *MockExecClient) ExecutePowershellCommand(_ string) (string, error) {
+	return "", nil
+}
+
+func (e *MockExecClient) GetLastRebootTime() (time.Time, error) {
+	return time.Time{}, nil
+}
+
+func (e *MockExecClient) KillProcessByName(_ string) error {
+	return nil
+}
diff --git a/platform/os.go b/platform/os.go
@@ -4,8 +4,9 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"log"
 	"os"
+
+	"github.com/Azure/azure-container-networking/log"
 )
 
 // ReadFileByLines reads file line by line and return array of lines.