Skip to content

Commit

Permalink
fix: skip setting SdnRemoteArpMacAddress when hns is not enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
ZetaoZhuang committed Oct 20, 2023
1 parent ce5c12b commit 9323adc
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 12 deletions.
5 changes: 3 additions & 2 deletions cns/service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,8 +724,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
Expand Down
19 changes: 15 additions & 4 deletions platform/mockexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
}

Expand Down
24 changes: 18 additions & 6 deletions platform/os_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 have hns path or not
CheckIfHNSPathExistsCommand = "Test-Path " +
"-Path HKLM:\\SYSTEM\\CurrentControlSet\\Services\\hns\\State"

// Command to restart HNS service
RestartHnsServiceCommand = "Restart-Service -Name hns"

Expand Down Expand Up @@ -190,24 +194,32 @@ 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 {
existed, err := execClient.ExecutePowershellCommand(CheckIfHNSPathExistsCommand)
if err != nil {
log.Printf("Failed to check the existent of hns path due to error %s", err.Error())
return err
}
if existed == "false" {
log.Printf("hns path is not existed, 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
}
Expand Down
36 changes: 36 additions & 0 deletions platform/os_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package platform
import (
"errors"
"os/exec"
"strings"
"testing"

"github.com/Azure/azure-container-networking/platform/windows/adapter/mocks"
Expand Down Expand Up @@ -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
}
Binary file added scripts/vm2_vlan.pcap
Binary file not shown.

0 comments on commit 9323adc

Please sign in to comment.