Address comment

pkg/testcase/upgrademanual.go

@@ -12,7 +12,9 @@ import (
 )
 
 const (
-	server = "server"
+	server  = "server"
+	status  = "status"
+	restart = "restart"
 )
 
 // TestUpgradeClusterManual upgrades the cluster "manually".
@@ -67,53 +69,36 @@ func upgradeProduct(product, nodeType, installType, ip string) error {
 		shared.LogLevel("error", "error running cmd on %s %s: %v", nodeType, ip, err)
 		return err
 	}
-	status := []shared.ServiceAction{
-		{
-			Service:  product,
-			Action:   "status",
-			NodeType: nodeType,
-		},
+
+	actions := []shared.ServiceAction{
+		{Service: product, Action: restart, NodeType: nodeType, ExplicitDelay: 180},
+		{Service: product, Action: status, NodeType: nodeType, ExplicitDelay: 60},
 	}
 
 	if product == "rke2" {
 		shared.LogLevel("info", "Waiting for 2 mins after installing upgrade...")
 		time.Sleep(2 * time.Minute)
-		ms := shared.NewManageService(3, 3)
-		restart := []shared.ServiceAction{
-			{
-				Service:  product,
-				Action:   "restart",
-				NodeType: nodeType,
-			},
-		}
-		output, err := ms.ManageService(ip, restart)
+		ms := shared.NewManageService(3, 30)
+		output, err := ms.ManageService(ip, actions)
 		if output != "" {
 			Expect(output).To(ContainSubstring("active "),
-				fmt.Sprintf("error starting %s service for %s node ip: %s", product, nodeType, ip))
+				fmt.Sprintf("error running %s service %s on %s node: %s", product, restart, nodeType, ip))
 		}
-		Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("error starting %s service on %s", product, ip))
-
-		shared.LogLevel("info", "Waiting for 3 mins after restarting service...")
-		time.Sleep(3 * time.Minute)
-
-		output, err = ms.ManageService(ip, status)
-		if output != "" {
-			Expect(output).To(ContainSubstring("active "),
-				fmt.Sprintf("error checking status %s service for %s node ip: %s", product, nodeType, ip))
+		if err != nil {
+			return err
 		}
-		Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("error checking status %s service on %s", product, ip))
 	}
 
 	if product == "k3s" {
-		shared.LogLevel("info", "Waiting for 1 mins after installing upgrade...")
-		time.Sleep(1 * time.Minute)
-		ms := shared.NewManageService(3, 1)
-		output, err := ms.ManageService(ip, status)
+		ms := shared.NewManageService(3, 10)
+		output, err := ms.ManageService(ip, []shared.ServiceAction{actions[1]})
 		if output != "" {
 			Expect(output).To(ContainSubstring("active "),
-				fmt.Sprintf("error starting %s service for %s node ip: %s", product, nodeType, ip))
+				fmt.Sprintf("error running %s service %s on %s node: %s", product, status, nodeType, ip))
+		}
+		if err != nil {
+			return err
 		}
-		Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("error starting %s service on %s", product, ip))
 	}
 
 	return nil

shared/manageservice.go

@@ -9,19 +9,22 @@ import (
 )
 
 type ManageService struct {
-	MaxRetries uint
-	RetryDelay time.Duration
+	MaxRetries    uint
+	RetryDelay    time.Duration
+	ExplicitDelay time.Duration
 }
 
 // ServiceAction represents a service operation to perform on a given node type.
 type ServiceAction struct {
-	Service  string
-	Action   string
-	NodeType string
+	Service       string
+	Action        string
+	NodeType      string
+	ExplicitDelay time.Duration
 }
 
 // NewManageService creates a ManageService instance with the given maxRetries and retryDelay parameters.
-// maxRetries/retryDelay are used in order to retry the service operation in case of failure or delays.
+// maxRetries number of maximum retries on failed command response.
+// retryDelay time duration after which the failed command will be retried.
 func NewManageService(maxRetries uint, retryDelay time.Duration) *ManageService {
 	return &ManageService{
 		MaxRetries: maxRetries,
@@ -35,9 +38,12 @@ func NewManageService(maxRetries uint, retryDelay time.Duration) *ManageService
 //
 // If action is "rotate", it will rotate the certificate for the given service.
 // If action is "status", it will return the status of the service.
+//
+// ip IP of the node where the systemctl service call is performed.
+// actions slice of ServiceAction struct contains all the service actions.
 func (ms *ManageService) ManageService(ip string, actions []ServiceAction) (string, error) {
 	for _, act := range actions {
-		LogLevel("debug", "Performing %s %s service on node: %s", act.Action, act.Service, ip)
+		LogLevel("info", "Running %s %s service on node: %s", act.Action, act.Service, ip)
 		switch act.Action {
 		case "rotate":
 			rotateErr := CertRotate(act.Service, ip)
@@ -65,7 +71,13 @@ func (ms *ManageService) ManageService(ip string, actions []ServiceAction) (stri
 				return "", fmt.Errorf("command build failed for %s: %w", ip, err)
 			}
 
-			LogLevel("debug", "Command: %s on %s@%s", cmd, svcName, ip)
+			if act.ExplicitDelay > 0 {
+				LogLevel("info", "Waiting for %ss before running systemctl %s %s on node: %s",
+					act.ExplicitDelay.String(), act.Action, act.Service, ip)
+				<-time.After(act.ExplicitDelay * time.Second)
+			}
+
+			LogLevel("debug", "Command: %s on node: %s", cmd, ip)
 			output, err := ms.execute(cmd, ip)
 			if err != nil {
 				return "", fmt.Errorf("action %s failed on %s: %w", act.Action, ip, err)
@@ -75,8 +87,11 @@ func (ms *ManageService) ManageService(ip string, actions []ServiceAction) (stri
 				LogLevel("debug", "service %s output: \n %s", act.Action, output)
 				return strings.TrimSpace(output), nil
 			}
-
-			LogLevel("debug", "Completed performing %s %s service on node: %s\nOutput: %s", act.Action, svcName, ip, strings.TrimSpace(output))
+			LogLevel("info", "Finished running systemctl %s %s on node: %s\n", act.Action, svcName, ip)
+			if output != "" {
+				LogLevel("debug", "Output: %s", strings.TrimSpace(output))
+			}
+
 		}
 	}