diff --git a/aitelemetry/telemetrywrapper_test.go b/aitelemetry/telemetrywrapper_test.go index 5fda52de70..08e63c8dc7 100644 --- 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 index 0fee94da91..43526618fb 100644 --- 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 index dd81255e00..923e8cbd49 100644 --- 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 index 6324a28374..bf21571d0b 100644 --- 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 index ff93bd3fa4..0f10c5b94b 100644 --- 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 index c4e070b7b1..ff99114b20 100644 --- 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 index 406e58245b..2e4686d16b 100644 --- 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 index f8b4822a07..1f6fe6b23c 100644 --- 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 index ba8e215622..44340701d7 100644 --- 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 index 6a634594a8..710ce21305 100644 --- 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 index 46266e7a25..8e3c4591f7 100644 --- 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 index bc8feb9aca..2e80e30227 100644 --- 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 index 2b2c3b57b0..5e66f02e51 100644 --- 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 index 32bc79c8c9..4e65c6a2ba 100644 --- 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 index 8616a0bf69..ab849c307a 100644 --- 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 index d056b0b10d..3ff27a503e 100644 --- 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,7 +350,7 @@ 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 } @@ -360,7 +358,7 @@ func applyDnsConfig(extIf *externalInterface, ifName string) error { 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 index 9360467f26..bdca8de8cc 100644 --- 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 index 22b03f6935..d4f8558d5a 100644 --- 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 index 8f8300ecbd..c0b17a0fc4 100644 --- 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. diff --git a/platform/osInterface.go b/platform/osInterface.go index 116e0ee5cd..fa3a91ed8d 100644 --- a/platform/osInterface.go +++ b/platform/osInterface.go @@ -2,6 +2,8 @@ package platform import ( "time" + + "go.uber.org/zap" ) const ( @@ -10,16 +12,22 @@ const ( type execClient struct { Timeout time.Duration + logger *zap.Logger } //nolint:revive // ExecClient make sense type ExecClient interface { ExecuteCommand(command string) (string, error) + GetLastRebootTime() (time.Time, error) + ClearNetworkConfiguration() (bool, error) + ExecutePowershellCommand(command string) (string, error) + KillProcessByName(processName string) error } -func NewExecClient() ExecClient { +func NewExecClient(logger *zap.Logger) ExecClient { return &execClient{ Timeout: defaultExecTimeout * time.Second, + logger: logger, } } diff --git a/platform/os_linux.go b/platform/os_linux.go index b99979a226..444b32071e 100644 --- a/platform/os_linux.go +++ b/platform/os_linux.go @@ -13,6 +13,7 @@ import ( "time" "github.com/Azure/azure-container-networking/log" + "go.uber.org/zap" ) const ( @@ -53,18 +54,22 @@ func GetOSInfo() string { } func GetProcessSupport() error { - p := NewExecClient() + p := NewExecClient(nil) cmd := fmt.Sprintf("ps -p %v -o comm=", os.Getpid()) _, err := p.ExecuteCommand(cmd) return err } // GetLastRebootTime returns the last time the system rebooted. -func GetLastRebootTime() (time.Time, error) { +func (p *execClient) GetLastRebootTime() (time.Time, error) { // Query last reboot time. out, err := exec.Command("uptime", "-s").Output() if err != nil { - log.Printf("Failed to query uptime, err:%v", err) + if p.logger != nil { + p.logger.Error("Failed to query uptime", zap.Error(err)) + } else { + log.Printf("Failed to query uptime, err:%v", err) + } return time.Time{}.UTC(), err } @@ -72,7 +77,11 @@ func GetLastRebootTime() (time.Time, error) { layout := "2006-01-02 15:04:05" rebootTime, err := time.ParseInLocation(layout, string(out[:len(out)-1]), time.Local) if err != nil { - log.Printf("Failed to parse uptime, err:%v", err) + if p.logger != nil { + p.logger.Error("Failed to parse uptime", zap.Error(err)) + } else { + log.Printf("Failed to parse uptime, err:%v", err) + } return time.Time{}.UTC(), err } @@ -80,7 +89,11 @@ func GetLastRebootTime() (time.Time, error) { } func (p *execClient) ExecuteCommand(command string) (string, error) { - log.Printf("[Azure-Utils] %s", command) + if p.logger != nil { + p.logger.Info("[Azure-Utils]", zap.String("command", command)) + } else { + log.Printf("[Azure-Utils] %s", command) + } var stderr bytes.Buffer var out bytes.Buffer @@ -102,7 +115,7 @@ func (p *execClient) ExecuteCommand(command string) (string, error) { } func SetOutboundSNAT(subnet string) error { - p := NewExecClient() + p := NewExecClient(nil) cmd := fmt.Sprintf("iptables -t nat -A POSTROUTING -m iprange ! --dst-range 168.63.129.16 -m addrtype ! --dst-type local ! -d %v -j MASQUERADE", subnet) _, err := p.ExecuteCommand(cmd) @@ -115,12 +128,15 @@ func SetOutboundSNAT(subnet string) error { // ClearNetworkConfiguration clears the azure-vnet.json contents. // This will be called only when reboot is detected - This is windows specific -func ClearNetworkConfiguration() (bool, error) { +func (p *execClient) ClearNetworkConfiguration() (bool, error) { return false, nil } -func KillProcessByName(processName string) error { - p := NewExecClient() +func (p *execClient) ExecutePowershellCommand(_ string) (string, error) { + return "", nil +} + +func (p *execClient) KillProcessByName(processName string) error { cmd := fmt.Sprintf("pkill -f %v", processName) _, err := p.ExecuteCommand(cmd) return err @@ -151,7 +167,7 @@ func GetOSDetails() (map[string]string, error) { } func GetProcessNameByID(pidstr string) (string, error) { - p := NewExecClient() + p := NewExecClient(nil) pidstr = strings.Trim(pidstr, "\n") cmd := fmt.Sprintf("ps -p %s -o comm=", pidstr) out, err := p.ExecuteCommand(cmd) @@ -167,7 +183,7 @@ func GetProcessNameByID(pidstr string) (string, error) { } func PrintDependencyPackageDetails() { - p := NewExecClient() + p := NewExecClient(nil) out, err := p.ExecuteCommand("iptables --version") out = strings.TrimSuffix(out, "\n") log.Printf("[cni-net] iptable version:%s, err:%v", out, err) diff --git a/platform/os_test.go b/platform/os_test.go index bb32a16e00..221d9036c5 100644 --- a/platform/os_test.go +++ b/platform/os_test.go @@ -5,6 +5,9 @@ import ( "strconv" "strings" "testing" + + "github.com/Azure/azure-container-networking/cni/log" + "go.uber.org/zap" ) func TestMain(m *testing.M) { @@ -13,7 +16,9 @@ func TestMain(m *testing.M) { } func TestGetLastRebootTime(t *testing.T) { - _, err := GetLastRebootTime() + logger := log.CNILogger.With(zap.String("component", "platform")) + p := NewExecClient(logger) + _, err := p.GetLastRebootTime() if err != nil { t.Errorf("GetLastRebootTime failed :%v", err) } diff --git a/platform/os_windows.go b/platform/os_windows.go index 6c5b450a3c..6fe2d2ab19 100644 --- a/platform/os_windows.go +++ b/platform/os_windows.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-container-networking/platform/windows/adapter" "github.com/Azure/azure-container-networking/platform/windows/adapter/mellanox" "github.com/pkg/errors" + "go.uber.org/zap" "golang.org/x/sys/windows" ) @@ -88,28 +89,41 @@ func GetOSInfo() string { } func GetProcessSupport() error { + p := NewExecClient(nil) cmd := fmt.Sprintf("Get-Process -Id %v", os.Getpid()) - _, err := ExecutePowershellCommand(cmd) + _, err := p.ExecutePowershellCommand(cmd) return err } var tickCount = syscall.NewLazyDLL("kernel32.dll").NewProc("GetTickCount64") // GetLastRebootTime returns the last time the system rebooted. -func GetLastRebootTime() (time.Time, error) { +func (p *execClient) GetLastRebootTime() (time.Time, error) { currentTime := time.Now() output, _, err := tickCount.Call() if errno, ok := err.(syscall.Errno); !ok || errno != 0 { - log.Printf("Failed to call GetTickCount64, err: %v", err) + if p.logger != nil { + p.logger.Error("Failed to call GetTickCount64", zap.Error(err)) + } else { + log.Printf("Failed to call GetTickCount64, err: %v", err) + } return time.Time{}.UTC(), err } rebootTime := currentTime.Add(-time.Duration(output) * time.Millisecond).Truncate(time.Second) - log.Printf("Formatted Boot time: %s", rebootTime.Format(time.RFC3339)) + if p.logger != nil { + p.logger.Info("Formatted Boot", zap.String("time", rebootTime.Format(time.RFC3339))) + } else { + log.Printf("Formatted Boot time: %s", rebootTime.Format(time.RFC3339)) + } return rebootTime.UTC(), nil } func (p *execClient) ExecuteCommand(command string) (string, error) { - log.Printf("[Azure-Utils] ExecuteCommand: %q", command) + if p.logger != nil { + p.logger.Info("[Azure-Utils]", zap.String("ExecuteCommand", command)) + } else { + log.Printf("[Azure-Utils] ExecuteCommand: %q", command) + } var stderr, stdout bytes.Buffer @@ -130,33 +144,37 @@ func SetOutboundSNAT(subnet string) error { // ClearNetworkConfiguration clears the azure-vnet.json contents. // This will be called only when reboot is detected - This is windows specific -func ClearNetworkConfiguration() (bool, error) { +func (p *execClient) ClearNetworkConfiguration() (bool, error) { jsonStore := CNIRuntimePath + "azure-vnet.json" - log.Printf("Deleting the json store %s", jsonStore) + p.logger.Info("Deleting the json", zap.String("store", jsonStore)) cmd := exec.Command("cmd", "/c", "del", jsonStore) if err := cmd.Run(); err != nil { - log.Printf("Error deleting the json store %s", jsonStore) + p.logger.Info("Error deleting the json", zap.String("store", jsonStore)) return true, err } return true, nil } -func KillProcessByName(processName string) { - p := NewExecClient() +func (p *execClient) KillProcessByName(processName string) error { cmd := fmt.Sprintf("taskkill /IM %v /F", processName) - p.ExecuteCommand(cmd) + _, err := p.ExecuteCommand(cmd) + return err // nolint } // ExecutePowershellCommand executes powershell command -func ExecutePowershellCommand(command string) (string, error) { +func (p *execClient) ExecutePowershellCommand(command string) (string, error) { ps, err := exec.LookPath("powershell.exe") if err != nil { return "", fmt.Errorf("Failed to find powershell executable") } - log.Printf("[Azure-Utils] %s", command) + if p.logger != nil { + p.logger.Info("[Azure-Utils]", zap.String("command", command)) + } else { + log.Printf("[Azure-Utils] %s", command) + } cmd := exec.Command(ps, command) var stdout bytes.Buffer @@ -174,21 +192,22 @@ func ExecutePowershellCommand(command string) (string, error) { // SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy func SetSdnRemoteArpMacAddress() error { + p := NewExecClient(nil) if sdnRemoteArpMacAddressSet == false { - result, err := ExecutePowershellCommand(GetSdnRemoteArpMacAddressCommand) + result, err := p.ExecutePowershellCommand(GetSdnRemoteArpMacAddressCommand) if err != nil { return err } // Set the reg key if not already set or has incorrect value if result != SDNRemoteArpMacAddress { - if _, err = ExecutePowershellCommand(SetSdnRemoteArpMacAddressCommand); err != nil { + if _, err = p.ExecutePowershellCommand(SetSdnRemoteArpMacAddressCommand); err != nil { log.Printf("Failed to set SDNRemoteArpMacAddress due to error %s", err.Error()) return err } log.Printf("[Azure CNS] SDNRemoteArpMacAddress regKey set successfully. Restarting hns service.") - if _, err := ExecutePowershellCommand(RestartHnsServiceCommand); err != nil { + if _, err := p.ExecutePowershellCommand(RestartHnsServiceCommand); err != nil { log.Printf("Failed to Restart HNS Service due to error %s", err.Error()) return err } @@ -269,7 +288,8 @@ func GetOSDetails() (map[string]string, error) { func GetProcessNameByID(pidstr string) (string, error) { pidstr = strings.Trim(pidstr, "\r\n") cmd := fmt.Sprintf("Get-Process -Id %s|Format-List", pidstr) - out, err := ExecutePowershellCommand(cmd) + p := NewExecClient(nil) + out, err := p.ExecutePowershellCommand(cmd) if err != nil { log.Printf("Process is not running. Output:%v, Error %v", out, err) return "", err diff --git a/platform/os_windows_test.go b/platform/os_windows_test.go index 17cdc6db6f..a6e44d2fa5 100644 --- a/platform/os_windows_test.go +++ b/platform/os_windows_test.go @@ -85,13 +85,13 @@ func TestUpdatePriorityVLANTagIfRequiredIfCurrentValNotEqualDesiredValAndSetRetu } func TestExecuteCommand(t *testing.T) { - out, err := NewExecClient().ExecuteCommand("dir") + out, err := NewExecClient(nil).ExecuteCommand("dir") require.NoError(t, err) require.NotEmpty(t, out) } func TestExecuteCommandError(t *testing.T) { - _, err := NewExecClient().ExecuteCommand("dontaddtopath") + _, err := NewExecClient(nil).ExecuteCommand("dontaddtopath") require.Error(t, err) var xErr *exec.ExitError diff --git a/platform/windows/adapter/mellanox/mellanox.go b/platform/windows/adapter/mellanox/mellanox.go index e7d37cc2e7..7c86d8b13d 100644 --- a/platform/windows/adapter/mellanox/mellanox.go +++ b/platform/windows/adapter/mellanox/mellanox.go @@ -198,7 +198,7 @@ func (m *Mellanox) getRegistryFullPath() (string, error) { return registryKeyPrefix + registryKeySuffix, nil } -// ExecutePowershellCommand executes powershell command +// executePowershellCommand executes powershell command func executePowershellCommand(command string) (string, error) { ps, err := exec.LookPath("powershell.exe") if err != nil { diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go index 1d946e5b0a..dfa052794e 100644 --- a/telemetry/telemetry.go +++ b/telemetry/telemetry.go @@ -82,6 +82,7 @@ type CNIReport struct { InterfaceDetails InterfaceInfo BridgeDetails BridgeInfo Metadata common.Metadata `json:"compute"` + Logger *zap.Logger } type AIMetric struct { diff --git a/telemetry/telemetry_windows.go b/telemetry/telemetry_windows.go index 834ad6b8e2..31a50828db 100644 --- a/telemetry/telemetry_windows.go +++ b/telemetry/telemetry_windows.go @@ -4,9 +4,10 @@ package telemetry import ( - "github.com/Azure/azure-container-networking/platform" "runtime" "strings" + + "github.com/Azure/azure-container-networking/platform" ) const ( @@ -37,7 +38,7 @@ func (report *CNIReport) GetSystemDetails() { } func (report *CNIReport) GetOSDetails() { - p := platform.NewExecClient() + p := platform.NewExecClient(report.Logger) report.OSDetails = OSInfo{OSType: runtime.GOOS} out, err := p.ExecuteCommand(versionCmd) if err == nil { diff --git a/telemetry/telemetrybuffer.go b/telemetry/telemetrybuffer.go index 55f3107b9d..2be650faa9 100644 --- a/telemetry/telemetrybuffer.go +++ b/telemetry/telemetrybuffer.go @@ -57,6 +57,7 @@ type TelemetryBuffer struct { cancel chan bool mutex sync.Mutex logger *zap.Logger + plc platform.ExecClient } // Buffer object holds the different types of reports @@ -72,6 +73,7 @@ func NewTelemetryBuffer(logger *zap.Logger) *TelemetryBuffer { tb.cancel = make(chan bool, 1) tb.connections = make([]net.Conn, 0) tb.logger = logger + tb.plc = platform.NewExecClient(tb.logger) return &tb } @@ -257,7 +259,11 @@ func (tb *TelemetryBuffer) Close() { } if tb.listener != nil { - log.Logf("server close") + if tb.logger != nil { + tb.logger.Info("server close") + } else { + log.Logf("server close") + } tb.listener.Close() } @@ -299,17 +305,36 @@ func WaitForTelemetrySocket(maxAttempt int, waitTimeInMillisecs time.Duration) { } // StartTelemetryService - Kills if any telemetry service runs and start new telemetry service -func StartTelemetryService(path string, args []string) error { - platform.KillProcessByName(TelemetryServiceProcessName) +func (tb *TelemetryBuffer) StartTelemetryService(path string, args []string) error { + err := tb.plc.KillProcessByName(TelemetryServiceProcessName) + if err != nil { + if tb.logger != nil { + tb.logger.Error("Failed to kill process by", zap.String("TelemetryServiceProcessName", TelemetryServiceProcessName), zap.Error(err)) + } else { + log.Logf("[Telemetry] Failed to kill process by telemetryServiceProcessName %s due to %v", TelemetryServiceProcessName, err) + } + } - log.Logf("[Telemetry] Starting telemetry service process :%v args:%v", path, args) + if tb.logger != nil { + tb.logger.Info("Starting telemetry service process", zap.String("path", path), zap.Any("args", args)) + } else { + log.Logf("[Telemetry] Starting telemetry service process :%v args:%v", path, args) + } if err := common.StartProcess(path, args); err != nil { - log.Logf("[Telemetry] Failed to start telemetry service process :%v", err) + if tb.logger != nil { + tb.logger.Error("Failed to start telemetry service process", zap.Error(err)) + } else { + log.Logf("[Telemetry] Failed to start telemetry service process :%v", err) + } return err } - log.Logf("[Telemetry] Telemetry service started") + if tb.logger != nil { + tb.logger.Info("Telemetry service started") + } else { + log.Logf("[Telemetry] Telemetry service started") + } return nil } @@ -320,12 +345,11 @@ func ReadConfigFile(filePath string) (TelemetryConfig, error) { b, err := os.ReadFile(filePath) if err != nil { - log.Logf("[Telemetry] Failed to read telemetry config: %v", err) return config, err } if err = json.Unmarshal(b, &config); err != nil { - log.Logf("[Telemetry] unmarshal failed with %v", err) + return config, err // nolint } return config, err @@ -338,17 +362,29 @@ func (tb *TelemetryBuffer) ConnectToTelemetryService(telemetryNumRetries, teleme for attempt := 0; attempt < 2; attempt++ { if err := tb.Connect(); err != nil { - log.Logf("Connection to telemetry socket failed: %v", err) + if tb.logger != nil { + tb.logger.Error("Connection to telemetry socket failed", zap.Error(err)) + } else { + log.Logf("Connection to telemetry socket failed: %v", err) + } if _, exists := os.Stat(path); exists != nil { - log.Logf("Skip starting telemetry service as file didn't exist") + if tb.logger != nil { + tb.logger.Info("Skip starting telemetry service as file didn't exist") + } else { + log.Logf("Skip starting telemetry service as file didn't exist") + } return } tb.Cleanup(FdName) - StartTelemetryService(path, args) + tb.StartTelemetryService(path, args) // nolint WaitForTelemetrySocket(telemetryNumRetries, time.Duration(telemetryWaitTimeInMilliseconds)) } else { tb.Connected = true - log.Logf("Connected to telemetry service") + if tb.logger != nil { + tb.logger.Info("Connected to telemetry service") + } else { + log.Logf("Connected to telemetry service") + } return } } diff --git a/telemetry/telemetrybuffer_test.go b/telemetry/telemetrybuffer_test.go index 0120d50735..c69aa04aa7 100644 --- a/telemetry/telemetrybuffer_test.go +++ b/telemetry/telemetrybuffer_test.go @@ -154,6 +154,7 @@ func TestReadConfigFile(t *testing.T) { } func TestStartTelemetryService(t *testing.T) { - err := StartTelemetryService("", nil) + tb := NewTelemetryBuffer(nil) + err := tb.StartTelemetryService("", nil) require.Error(t, err) }