Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove PowerShell from Windows registry interactions (#2993) #3164

Open
wants to merge 1 commit into
base: release/v1.4
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cns/service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,8 @@ 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
err = platform.SetSdnRemoteArpMacAddress(rootCtx)
if err != nil {
logger.Errorf("Failed to set remote ARP MAC address: %v", err)
return
Expand Down
2 changes: 1 addition & 1 deletion platform/os_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func 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(context.Context) error {
return nil
}

Expand Down
99 changes: 67 additions & 32 deletions platform/os_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import (
"github.com/Azure/azure-container-networking/platform/windows/adapter/mellanox"
"github.com/pkg/errors"
"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"
"golang.org/x/sys/windows/svc"
"golang.org/x/sys/windows/svc/mgr"
)

const (
Expand Down Expand Up @@ -59,17 +62,6 @@ const (
// for vlan tagged arp requests
SDNRemoteArpMacAddress = "12-34-56-78-9a-bc"

// Command to get SDNRemoteArpMacAddress registry key
GetSdnRemoteArpMacAddressCommand = "(Get-ItemProperty " +
"-Path HKLM:\\SYSTEM\\CurrentControlSet\\Services\\hns\\State -Name SDNRemoteArpMacAddress).SDNRemoteArpMacAddress"

// Command to set SDNRemoteArpMacAddress registry key
SetSdnRemoteArpMacAddressCommand = "Set-ItemProperty " +
"-Path HKLM:\\SYSTEM\\CurrentControlSet\\Services\\hns\\State -Name SDNRemoteArpMacAddress -Value \"12-34-56-78-9a-bc\""

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

// Interval between successive checks for mellanox adapter's PriorityVLANTag value
defaultMellanoxMonitorInterval = 30 * time.Second

Expand Down Expand Up @@ -172,31 +164,74 @@ func ExecutePowershellCommand(command string) (string, error) {
return strings.TrimSpace(stdout.String()), nil
}

// SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy
func SetSdnRemoteArpMacAddress() error {
if sdnRemoteArpMacAddressSet == false {
result, err := ExecutePowershellCommand(GetSdnRemoteArpMacAddressCommand)
if err != nil {
return err
// SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy if hns is enabled
func SetSdnRemoteArpMacAddress(ctx context.Context) error {
log.Printf("Setting SDNRemoteArpMacAddress regKey")
// open the registry key
k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SYSTEM\CurrentControlSet\Services\hns\State`, registry.READ|registry.SET_VALUE)
if err != nil {
if errors.Is(err, registry.ErrNotExist) {
return nil
}
return errors.Wrap(err, "could not open registry key")
}
defer k.Close()
// check the key value
if v, _, _ := k.GetStringValue("SDNRemoteArpMacAddress"); v == SDNRemoteArpMacAddress {
log.Printf("SDNRemoteArpMacAddress regKey already set")
return nil // already set
}
if err = k.SetStringValue("SDNRemoteArpMacAddress", SDNRemoteArpMacAddress); err != nil {
return errors.Wrap(err, "could not set registry key")
}
log.Printf("SDNRemoteArpMacAddress regKey set successfully")
log.Printf("Restarting HNS service")
// connect to the service manager
m, err := mgr.Connect()
if err != nil {
return errors.Wrap(err, "could not connect to service manager")
}
defer m.Disconnect() //nolint:errcheck // ignore error
// open the HNS service
service, err := m.OpenService("hns")
if err != nil {
return errors.Wrap(err, "could not access service")
}
defer service.Close()
if err := restartService(ctx, service); err != nil {
return errors.Wrap(err, "could not restart service")
}
log.Printf("HNS service restarted successfully")
return nil
}

// Set the reg key if not already set or has incorrect value
if result != SDNRemoteArpMacAddress {
if _, err = 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 {
log.Printf("Failed to Restart HNS Service due to error %s", err.Error())
return err
}
func restartService(ctx context.Context, s *mgr.Service) error {
// Stop the service
_, err := s.Control(svc.Stop)
if err != nil {
return errors.Wrap(err, "could not stop service")
}
// Wait for the service to stop
ticker := time.NewTicker(500 * time.Millisecond) //nolint:gomnd // 500ms
defer ticker.Stop()
for { // hacky cancellable do-while
status, err := s.Query()
if err != nil {
return errors.Wrap(err, "could not query service status")
}
if status.State == svc.Stopped {
break
}
select {
case <-ctx.Done():
return errors.New("context cancelled")
case <-ticker.C:
}

sdnRemoteArpMacAddressSet = true
}

// Start the service again
if err := s.Start(); err != nil {
return errors.Wrap(err, "could not start service")
}
return nil
}

Expand Down
Loading