fix: skip setting SdnRemoteArpMacAddress when hns is not enabled (#2315)

 fix: skip setting SdnRemoteArpMacAddress when hns is not enabled

Signed-off-by: ZetaoZhuang <[email protected]>

Author: ZetaoZhuang

cns/service/main.go

@@ -725,8 +725,9 @@ func main() {
 		}
 	}
 
-	// Setting the remote ARP MAC address to 12-34-56-78-9a-bc on windows for external traffic
-	err = platform.SetSdnRemoteArpMacAddress()
+	// Setting the remote ARP MAC address to 12-34-56-78-9a-bc on windows for external traffic if HNS is enabled
+	execClient := platform.NewExecClient(nil)
+	err = platform.SetSdnRemoteArpMacAddress(execClient)
 	if err != nil {
 		logger.Errorf("Failed to set remote ARP MAC address: %v", err)
 		return

platform/mockexec.go

@@ -6,11 +6,15 @@ import (
 )
 
 type MockExecClient struct {
-	returnError    bool
-	setExecCommand execCommandValidator
+	returnError                bool
+	setExecCommand             execCommandValidator
+	powershellCommandResponder powershellCommandResponder
 }
 
-type execCommandValidator func(string) (string, error)
+type (
+	execCommandValidator       func(string) (string, error)
+	powershellCommandResponder func(string) (string, error)
+)
 
 // ErrMockExec - mock exec error
 var ErrMockExec = errors.New("mock exec error")
@@ -37,11 +41,18 @@ func (e *MockExecClient) SetExecCommand(fn execCommandValidator) {
 	e.setExecCommand = fn
 }
 
+func (e *MockExecClient) SetPowershellCommandResponder(fn powershellCommandResponder) {
+	e.powershellCommandResponder = fn
+}
+
 func (e *MockExecClient) ClearNetworkConfiguration() (bool, error) {
 	return true, nil
 }
 
-func (e *MockExecClient) ExecutePowershellCommand(_ string) (string, error) {
+func (e *MockExecClient) ExecutePowershellCommand(cmd string) (string, error) {
+	if e.powershellCommandResponder != nil {
+		return e.powershellCommandResponder(cmd)
+	}
 	return "", nil
 }
 

platform/os_linux.go

@@ -144,7 +144,7 @@ func (p *execClient) KillProcessByName(processName string) error {
 
 // SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy
 // This operation is specific to windows OS
-func SetSdnRemoteArpMacAddress() error {
+func SetSdnRemoteArpMacAddress(_ ExecClient) error {
 	return nil
 }
 

platform/os_windows.go

@@ -68,6 +68,10 @@ const (
 	SetSdnRemoteArpMacAddressCommand = "Set-ItemProperty " +
 		"-Path HKLM:\\SYSTEM\\CurrentControlSet\\Services\\hns\\State -Name SDNRemoteArpMacAddress -Value \"12-34-56-78-9a-bc\""
 
+	// Command to check if system has hns state path or not
+	CheckIfHNSStatePathExistsCommand = "Test-Path " +
+		"-Path HKLM:\\SYSTEM\\CurrentControlSet\\Services\\hns\\State"
+
 	// Command to restart HNS service
 	RestartHnsServiceCommand = "Restart-Service -Name hns"
 
@@ -190,24 +194,33 @@ func (p *execClient) ExecutePowershellCommand(command string) (string, error) {
 	return strings.TrimSpace(stdout.String()), nil
 }
 
-// SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy
-func SetSdnRemoteArpMacAddress() error {
-	p := NewExecClient(nil)
+// SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy if hns is enabled
+func SetSdnRemoteArpMacAddress(execClient ExecClient) error {
+	exists, err := execClient.ExecutePowershellCommand(CheckIfHNSStatePathExistsCommand)
+	if err != nil {
+		errMsg := fmt.Sprintf("Failed to check the existent of hns state path due to error %s", err.Error())
+		log.Printf(errMsg)
+		return errors.Errorf(errMsg)
+	}
+	if strings.EqualFold(exists, "false") {
+		log.Printf("hns state path does not exist, skip setting SdnRemoteArpMacAddress")
+		return nil
+	}
 	if sdnRemoteArpMacAddressSet == false {
-		result, err := p.ExecutePowershellCommand(GetSdnRemoteArpMacAddressCommand)
+		result, err := execClient.ExecutePowershellCommand(GetSdnRemoteArpMacAddressCommand)
 		if err != nil {
 			return err
 		}
 
 		// Set the reg key if not already set or has incorrect value
 		if result != SDNRemoteArpMacAddress {
-			if _, err = p.ExecutePowershellCommand(SetSdnRemoteArpMacAddressCommand); err != nil {
+			if _, err = execClient.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 := p.ExecutePowershellCommand(RestartHnsServiceCommand); err != nil {
+			if _, err := execClient.ExecutePowershellCommand(RestartHnsServiceCommand); err != nil {
 				log.Printf("Failed to Restart HNS Service due to error %s", err.Error())
 				return err
 			}

platform/os_windows_test.go

@@ -3,6 +3,7 @@ package platform
 import (
 	"errors"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/Azure/azure-container-networking/platform/windows/adapter/mocks"
@@ -98,3 +99,38 @@ func TestExecuteCommandError(t *testing.T) {
 	assert.ErrorAs(t, err, &xErr)
 	assert.Equal(t, 1, xErr.ExitCode())
 }
+
+func TestSetSdnRemoteArpMacAddress_hnsNotEnabled(t *testing.T) {
+	mockExecClient := NewMockExecClient(false)
+	// testing skip setting SdnRemoteArpMacAddress when hns not enabled
+	mockExecClient.SetPowershellCommandResponder(func(_ string) (string, error) {
+		return "False", nil
+	})
+	err := SetSdnRemoteArpMacAddress(mockExecClient)
+	assert.NoError(t, err)
+	assert.Equal(t, false, sdnRemoteArpMacAddressSet)
+
+	// testing the scenario when there is an error in checking if hns is enabled or not
+	mockExecClient.SetPowershellCommandResponder(func(_ string) (string, error) {
+		return "", errTestFailure
+	})
+	err = SetSdnRemoteArpMacAddress(mockExecClient)
+	assert.ErrorAs(t, err, &errTestFailure)
+	assert.Equal(t, false, sdnRemoteArpMacAddressSet)
+}
+
+func TestSetSdnRemoteArpMacAddress_hnsEnabled(t *testing.T) {
+	mockExecClient := NewMockExecClient(false)
+	// happy path
+	mockExecClient.SetPowershellCommandResponder(func(cmd string) (string, error) {
+		if strings.Contains(cmd, "Test-Path") {
+			return "True", nil
+		}
+		return "", nil
+	})
+	err := SetSdnRemoteArpMacAddress(mockExecClient)
+	assert.NoError(t, err)
+	assert.Equal(t, true, sdnRemoteArpMacAddressSet)
+	// reset sdnRemoteArpMacAddressSet
+	sdnRemoteArpMacAddressSet = false
+}