backport: fix: prevent CNS hanging on HNS restart (#3343) (#3361)

* fix: prevent CNS hanging on HNS restart (#3343)

Signed-off-by: Evan Baker <[email protected]>

* disable gomnd

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

---------

Signed-off-by: Evan Baker <[email protected]>
Signed-off-by: GitHub <[email protected]>

Author: rbtr

.golangci.yml

@@ -17,7 +17,6 @@ linters:
   - gocritic
   - gocyclo
   - gofmt
-  - gomnd
   - goprintffuncname
   - gosimple
   - lll

cns/service/main.go

@@ -798,11 +798,14 @@ func main() {
 	}
 
 	// 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)
+	arpCtx, arpCtxCancel := context.WithTimeout(rootCtx, 30*time.Second)
+	err = platform.SetSdnRemoteArpMacAddress(arpCtx)
 	if err != nil {
 		logger.Errorf("Failed to set remote ARP MAC address: %v", err)
+		arpCtxCancel()
 		return
 	}
+	arpCtxCancel()
 
 	// We are only setting the PriorityVLANTag in 'cns.Direct' mode, because it neatly maps today, to 'isUsingMultitenancy'
 	// In the future, we would want to have a better CNS flag, to explicitly say, this CNS is using multitenancy

platform/os_windows.go

@@ -184,6 +184,18 @@ func (p *execClient) ExecutePowershellCommand(command string) (string, error) {
 
 // SetSdnRemoteArpMacAddress sets the regkey for SDNRemoteArpMacAddress needed for multitenancy if hns is enabled
 func SetSdnRemoteArpMacAddress(ctx context.Context) error {
+	if err := setSDNRemoteARPRegKey(); err != nil {
+		return err
+	}
+	log.Printf("SDNRemoteArpMacAddress regKey set successfully")
+	if err := restartHNS(ctx); err != nil {
+		return err
+	}
+	log.Printf("HNS service restarted successfully")
+	return nil
+}
+
+func setSDNRemoteARPRegKey() 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)
@@ -202,7 +214,10 @@ func SetSdnRemoteArpMacAddress(ctx context.Context) error {
 	if err = k.SetStringValue("SDNRemoteArpMacAddress", SDNRemoteArpMacAddress); err != nil {
 		return errors.Wrap(err, "could not set registry key")
 	}
-	log.Printf("SDNRemoteArpMacAddress regKey set successfully")
+	return nil
+}
+
+func restartHNS(ctx context.Context) error {
 	log.Printf("Restarting HNS service")
 	// connect to the service manager
 	m, err := mgr.Connect()
@@ -216,24 +231,16 @@ func SetSdnRemoteArpMacAddress(ctx context.Context) error {
 		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
-}
-
-func restartService(ctx context.Context, s *mgr.Service) error {
 	// Stop the service
-	_, err := s.Control(svc.Stop)
+	_, err = service.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()
+		status, err := service.Query()
 		if err != nil {
 			return errors.Wrap(err, "could not query service status")
 		}
@@ -247,7 +254,7 @@ func restartService(ctx context.Context, s *mgr.Service) error {
 		}
 	}
 	// Start the service again
-	if err := s.Start(); err != nil {
+	if err := service.Start(); err != nil {
 		return errors.Wrap(err, "could not start service")
 	}
 	return nil