Default FA action is reboot, and we error given any other action

Limit the usage of FAs to reboot action only
medik8s · Jul 26, 2023 · 496b5d7 · 496b5d7
1 parent a5f4761
commit 496b5d7
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 6 deletions.
diff --git a/controllers/fenceagentsremediation_controller.go b/controllers/fenceagentsremediation_controller.go
@@ -158,7 +158,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
 		r.Log.Error(err, "Fence Agent response was a failure", "CR's Name", req.Name)
 		return emptyResult, err
 	}
-	if outputErr != "" || outputRes != SuccessFAResponse {
+	if outputErr != "" || outputRes != SuccessFAResponse+"\n" {
 		// response wasn't failure or sucesss message
 		err := fmt.Errorf("unknown fence agent response - expecting `%s` response, but we received `%s`", SuccessFAResponse, outputRes)
 		r.Log.Error(err, "Fence Agent response wasn't a success message", "CR's Name", req.Name)
@@ -168,7 +168,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct
 }
 
 // buildFenceAgentParams collects the FAR's parameters for the node based on FAR CR, and if the CR is missing parameters
-// or the CR's name don't match nodeParamter name then return an error
+// or the CR's name don't match nodeParamter name or it has an action which is different than reboot, then return an error
 func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, error) {
 	logger := ctrl.Log.WithName("build-fa-parameters")
 	if far.Spec.NodeParameters == nil || far.Spec.SharedParameters == nil {
@@ -182,10 +182,15 @@ func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, erro
 		if paramName != parameterActionName {
 			fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, paramVal)
 		} else if paramVal != parameterActionValue {
-			logger.Info("FAR doesn't support any other action than reboot", "action", paramVal)
+			// --action attribute was selected but it is differemt than reboot
+			err := errors.New("FAR doesn't support any other action than reboot")
+			logger.Error(err, "can't build CR with this action attribute", "action", paramVal)
+			return nil, err
 		}
 	}
-	// ensure the FA uses the reboot action
+	// if --action attribute was not selected, then it's default value is reboot
+	// https://github.com/ClusterLabs/fence-agents/blob/main/lib/fencing.py.py#L103
+	// Therefore we can safely add the reboot action regardless if it was initially added into the CR
 	fenceAgentParams = appendParamToSlice(fenceAgentParams, parameterActionName, parameterActionValue)
 
 	// append node parameters

diff --git a/controllers/fenceagentsremediation_controller_test.go b/controllers/fenceagentsremediation_controller_test.go
@@ -55,7 +55,6 @@ var _ = Describe("FAR Controller", func() {
 	invalidShareParam := map[v1alpha1.ParameterName]string{
 		"--username": "admin",
 		"--password": "password",
-		"--action":   "",
 		"--ip":       "192.168.111.1",
 		"--lanplus":  "",
 	}

diff --git a/test/e2e/far_e2e_test.go b/test/e2e/far_e2e_test.go
@@ -107,7 +107,7 @@ var _ = Describe("FAR E2e", func() {
 				checkRemediation(nodeName, nodeBootTimeBefore)
 			})
 			It("should successfully remediate the second node", func() {
-				checkRemediation(nodeName,nodeBootTimeBefore)
+				checkRemediation(nodeName, nodeBootTimeBefore)
 			})
 		})
 	})