Rework running Fence Agent command

api/v1alpha1/fenceagentsremediation_types.go

@@ -28,14 +28,6 @@ const (
 	FARFinalizer string = "fence-agents-remediation.medik8s.io/far-finalizer"
 	// Taints
 	FARNoExecuteTaintKey = "medik8s.io/fence-agents-remediation"
-	// FenceAgentActionSucceededType is the condition type used to signal whether the Fence Agent action was succeeded successfully or not
-	FenceAgentActionSucceededType = "FenceAgentActionSucceeded"
-	// condition messages
-	RemediationFinishedNodeNotFoundConditionMessage = "FAR CR name doesn't match a node name"
-	RemediationInterruptedByNHCConditionMessage     = "Node Healthcheck timeout annotation has been set"
-	RemediationStartedConditionMessage              = "FAR CR was found, its name matches one of the cluster nodes, and a finalizer was set to the CR"
-	FenceAgentSucceededConditionMessage             = "FAR taint was added and the fence agent command has been created and executed successfully"
-	RemediationFinishedSuccessfullyConditionMessage = "The unhealthy node was fully remediated (it was tainted, fenced using FA and all the node resources have been deleted)"
 )
 
 // ConditionsChangeReason represents the reason of updating the some or all the conditions
@@ -63,6 +55,25 @@ type FenceAgentsRemediationSpec struct {
 	//+operator-sdk:csv:customresourcedefinitions:type=spec
 	Agent string `json:"agent"`
 
+	// RetryCount is the number of times the fencing agent will be executed
+	//+kubebuilder:default:=5
+	//+operator-sdk:csv:customresourcedefinitions:type=spec
+	RetryCount int `json:"retrycount,omitempty"`
+
+	// RetryInterval is the interval between each fencing agent execution
+	//+kubebuilder:default:="5s"
+	//+kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
+	//+kubebuilder:validation:Type=string
+	//+operator-sdk:csv:customresourcedefinitions:type=spec
+	RetryInterval metav1.Duration `json:"retryinterval,omitempty"`
+
+	// Timeout is the timeout for each fencing agent execution
+	//+kubebuilder:default:="60s"
+	//+kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
+	//+kubebuilder:validation:Type=string
+	//+operator-sdk:csv:customresourcedefinitions:type=spec
+	Timeout metav1.Duration `json:"timeout,omitempty"`
+
 	// SharedParameters are passed to the fencing agent regardless of which node is about to be fenced (i.e., they are common for all the nodes)
 	//+operator-sdk:csv:customresourcedefinitions:type=spec
 	SharedParameters map[ParameterName]string `json:"sharedparameters,omitempty"`

bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml

@@ -76,10 +76,19 @@ spec:
           node that is fenced, since they are node specific
         displayName: Node Parameters
         path: nodeparameters
+      - description: RetryCount is the number of times the fencing agent will be executed
+        displayName: Retry Count
+        path: retrycount
+      - description: RetryInterval is the interval between each fencing agent execution
+        displayName: Retry Interval
+        path: retryinterval
       - description: SharedParameters are passed to the fencing agent regardless of
           which node is about to be fenced (i.e., they are common for all the nodes)
         displayName: Shared Parameters
         path: sharedparameters
+      - description: Timeout is the timeout for each fencing agent execution
+        displayName: Timeout
+        path: timeout
       statusDescriptors:
       - description: 'Represents the observations of a FenceAgentsRemediation''s current
           state. Known .status.conditions.type are: "Processing", "FenceAgentActionSucceeded",
@@ -112,10 +121,19 @@ spec:
           node that is fenced, since they are node specific
         displayName: Node Parameters
         path: template.spec.nodeparameters
+      - description: RetryCount is the number of times the fencing agent will be executed
+        displayName: Retry Count
+        path: template.spec.retrycount
+      - description: RetryInterval is the interval between each fencing agent execution
+        displayName: Retry Interval
+        path: template.spec.retryinterval
       - description: SharedParameters are passed to the fencing agent regardless of
           which node is about to be fenced (i.e., they are common for all the nodes)
         displayName: Shared Parameters
         path: template.spec.sharedparameters
+      - description: Timeout is the timeout for each fencing agent execution
+        displayName: Timeout
+        path: template.spec.timeout
       version: v1alpha1
   description: "Fence Agents Remediation Operator (FAR)\n\nThe operator will remediate/fence
     a node when a FenceAgentsRemediation (far) custom resource exists with the node's

bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml

@@ -50,13 +50,29 @@ spec:
                 description: NodeParameters are passed to the fencing agent according
                   to the node that is fenced, since they are node specific
                 type: object
+              retrycount:
+                default: 5
+                description: RetryCount is the number of times the fencing agent will
+                  be executed
+                type: integer
+              retryinterval:
+                default: 5s
+                description: RetryInterval is the interval between each fencing agent
+                  execution
+                pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                type: string
               sharedparameters:
                 additionalProperties:
                   type: string
                 description: SharedParameters are passed to the fencing agent regardless
                   of which node is about to be fenced (i.e., they are common for all
                   the nodes)
                 type: object
+              timeout:
+                default: 60s
+                description: Timeout is the timeout for each fencing agent execution
+                pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                type: string
             required:
             - agent
             type: object

bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml

@@ -60,13 +60,30 @@ spec:
                           according to the node that is fenced, since they are node
                           specific
                         type: object
+                      retrycount:
+                        default: 5
+                        description: RetryCount is the number of times the fencing
+                          agent will be executed
+                        type: integer
+                      retryinterval:
+                        default: 5s
+                        description: RetryInterval is the interval between each fencing
+                          agent execution
+                        pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                        type: string
                       sharedparameters:
                         additionalProperties:
                           type: string
                         description: SharedParameters are passed to the fencing agent
                           regardless of which node is about to be fenced (i.e., they
                           are common for all the nodes)
                         type: object
+                      timeout:
+                        default: 60s
+                        description: Timeout is the timeout for each fencing agent
+                          execution
+                        pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                        type: string
                     required:
                     - agent
                     type: object

config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml

@@ -48,13 +48,29 @@ spec:
                 description: NodeParameters are passed to the fencing agent according
                   to the node that is fenced, since they are node specific
                 type: object
+              retrycount:
+                default: 5
+                description: RetryCount is the number of times the fencing agent will
+                  be executed
+                type: integer
+              retryinterval:
+                default: 5s
+                description: RetryInterval is the interval between each fencing agent
+                  execution
+                pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                type: string
               sharedparameters:
                 additionalProperties:
                   type: string
                 description: SharedParameters are passed to the fencing agent regardless
                   of which node is about to be fenced (i.e., they are common for all
                   the nodes)
                 type: object
+              timeout:
+                default: 60s
+                description: Timeout is the timeout for each fencing agent execution
+                pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                type: string
             required:
             - agent
             type: object

config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml

@@ -58,13 +58,30 @@ spec:
                           according to the node that is fenced, since they are node
                           specific
                         type: object
+                      retrycount:
+                        default: 5
+                        description: RetryCount is the number of times the fencing
+                          agent will be executed
+                        type: integer
+                      retryinterval:
+                        default: 5s
+                        description: RetryInterval is the interval between each fencing
+                          agent execution
+                        pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                        type: string
                       sharedparameters:
                         additionalProperties:
                           type: string
                         description: SharedParameters are passed to the fencing agent
                           regardless of which node is about to be fenced (i.e., they
                           are common for all the nodes)
                         type: object
+                      timeout:
+                        default: 60s
+                        description: Timeout is the timeout for each fencing agent
+                          execution
+                        pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
+                        type: string
                     required:
                     - agent
                     type: object

config/manifests/bases/fence-agents-remediation.clusterserviceversion.yaml

@@ -35,10 +35,19 @@ spec:
           node that is fenced, since they are node specific
         displayName: Node Parameters
         path: nodeparameters
+      - description: RetryCount is the number of times the fencing agent will be executed
+        displayName: Retry Count
+        path: retrycount
+      - description: RetryInterval is the interval between each fencing agent execution
+        displayName: Retry Interval
+        path: retryinterval
       - description: SharedParameters are passed to the fencing agent regardless of
           which node is about to be fenced (i.e., they are common for all the nodes)
         displayName: Shared Parameters
         path: sharedparameters
+      - description: Timeout is the timeout for each fencing agent execution
+        displayName: Timeout
+        path: timeout
       statusDescriptors:
       - description: 'Represents the observations of a FenceAgentsRemediation''s current
           state. Known .status.conditions.type are: "Processing", "FenceAgentActionSucceeded",
@@ -71,10 +80,19 @@ spec:
           node that is fenced, since they are node specific
         displayName: Node Parameters
         path: template.spec.nodeparameters
+      - description: RetryCount is the number of times the fencing agent will be executed
+        displayName: Retry Count
+        path: template.spec.retrycount
+      - description: RetryInterval is the interval between each fencing agent execution
+        displayName: Retry Interval
+        path: template.spec.retryinterval
       - description: SharedParameters are passed to the fencing agent regardless of
           which node is about to be fenced (i.e., they are common for all the nodes)
         displayName: Shared Parameters
         path: template.spec.sharedparameters
+      - description: Timeout is the timeout for each fencing agent execution
+        displayName: Timeout
+        path: template.spec.timeout
       version: v1alpha1
   description: "Fence Agents Remediation Operator (FAR)\n\nThe operator will remediate/fence
     a node when a FenceAgentsRemediation (far) custom resource exists with the node's

controllers/controllers_suite_test.go

@@ -17,9 +17,12 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -35,31 +38,79 @@ import (
 
 	//+kubebuilder:scaffold:imports ## https://github.com/kubernetes-sigs/kubebuilder/issues/1487 ?
 	"github.com/medik8s/fence-agents-remediation/api/v1alpha1"
+	"github.com/medik8s/fence-agents-remediation/pkg/cli"
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
 const defaultNamespace = "default"
 
-var k8sClient client.Client
-var k8sManager manager.Manager
-var testEnv *envtest.Environment
-var ctx context.Context
-var cancel context.CancelFunc
-var mocksExecuter *mockExecuter
+var (
+	k8sClient  client.Client
+	k8sManager manager.Manager
+	testEnv    *envtest.Environment
+	ctx        context.Context
+	cancel     context.CancelFunc
+
+	plogs *peekLogger
+
+	storedCommand []string
+	mockError     error
+	forcedDelay   time.Duration
+)
+
+// peekLogger allows to inspect operator's log for testing purpose.
+type peekLogger struct {
+	logs []string
+}
+
+func (p *peekLogger) Write(b []byte) (n int, err error) {
+	n, err = GinkgoWriter.Write(b)
+	if err != nil {
+		return n, err
+	}
+	p.logs = append(p.logs, string(b))
+	return n, err
+}
+
+func (p *peekLogger) Contains(s string) bool {
+	for _, log := range p.logs {
+		if strings.Contains(log, s) {
+			return true
+		}
+	}
+	return false
+}
+
+// CountOccurences returns number of occurences of string s in logs.
+func (p *peekLogger) CountOccurences(s string) int {
+	count := 0
+	for _, log := range p.logs {
+		if strings.Contains(log, s) {
+			count++
+		}
+	}
+	return count
+}
+
+func (p *peekLogger) Clear() {
+	p.logs = make([]string, 0)
+}
 
 func TestControllers(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Controllers Suite")
 }
 
 var _ = BeforeSuite(func() {
+	plogs = &peekLogger{logs: make([]string, 0)}
 	opts := zap.Options{
+		DestWriter:  plogs,
 		Development: true,
 		TimeEncoder: zapcore.RFC3339NanoTimeEncoder,
 	}
-	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseFlagOptions(&opts)))
+	logf.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
@@ -82,15 +133,16 @@ var _ = BeforeSuite(func() {
 	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
-	mocksExecuter = newMockExecuter()
+
+	executor, _ := cli.NewFakeExecuter(k8sClient, controlledRun)
 
 	os.Setenv("DEPLOYMENT_NAMESPACE", defaultNamespace)
 
 	err = (&FenceAgentsRemediationReconciler{
 		Client:   k8sClient,
 		Log:      k8sManager.GetLogger().WithName("test far reconciler"),
 		Scheme:   k8sManager.GetScheme(),
-		Executor: mocksExecuter,
+		Executor: executor,
 	}).SetupWithManager(k8sManager)
 	Expect(err).NotTo(HaveOccurred())
 
@@ -108,3 +160,17 @@ var _ = AfterSuite(func() {
 	err := testEnv.Stop()
 	Expect(err).NotTo(HaveOccurred())
 })
+
+func controlledRun(ctx context.Context, command []string) (stdout, stderr string, err error) {
+	storedCommand = command
+	if forcedDelay > 0 {
+		select {
+		case <-ctx.Done():
+			return "", "", fmt.Errorf("context expired")
+		case <-time.After(forcedDelay):
+			log.Info("forced delay expired", "delay", forcedDelay)
+		}
+	}
+
+	return "", "", mockError
+}