From b4b5e1ebf1e99065c54353ffd7aca4513fbb6d0d Mon Sep 17 00:00:00 2001 From: oraz Date: Sun, 12 Jan 2025 16:39:07 +0200 Subject: [PATCH 1/3] Verify one occurrence of agent's parameter at buildFenceAgentParams Use the first apperance of agent's parameter and allow adding the action parameter with value reboot while processing the parameters at buildFenceAgentParams function. Fix missing event print and improve comments --- .../fenceagentsremediation_controller.go | 35 +++++---- .../fenceagentsremediation_controller_test.go | 76 ++++++++++++------- 2 files changed, 68 insertions(+), 43 deletions(-) diff --git a/controllers/fenceagentsremediation_controller.go b/controllers/fenceagentsremediation_controller.go index 196cb747..08519f05 100644 --- a/controllers/fenceagentsremediation_controller.go +++ b/controllers/fenceagentsremediation_controller.go @@ -248,7 +248,7 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct switch far.Spec.RemediationStrategy { case v1alpha1.ResourceDeletionRemediationStrategy, "": // Basically RemediationStrategy should be set to ResourceDeletion strategy as the default strategy. - // However it will be empty when the CS was created when ResourceDeletion strategy was the only strategy. + // However, it will be empty when the CS was created when ResourceDeletion strategy was the only strategy. // In this case, the empty strategy should be treated as if ResourceDeletion strategy selected. r.Log.Info("Remediation strategy is ResourceDeletion which explicitly deletes resources - manually deleting workload", "Node Name", req.Name) commonEvents.NormalEvent(r.Recorder, node, utils.EventReasonDeleteResources, utils.EventMessageDeleteResources) @@ -338,7 +338,7 @@ func getNodeName(far *v1alpha1.FenceAgentsRemediation) string { } // 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 nodeParameter name or it has an action which is different than reboot, then return an error +// or the CR's name don't match nodeParameter name, or it has an action which is different from 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 { @@ -347,33 +347,40 @@ func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, erro return nil, err } var fenceAgentParams []string - // add shared parameters except the action parameter + fenceAgentParamNames := make(map[v1alpha1.ParameterName]bool) + + // append shared parameters for paramName, paramVal := range far.Spec.SharedParameters { - if paramName != parameterActionName { - fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, paramVal) - } else if paramVal != parameterActionValue { - // --action attribute was selected but it is different than reboot + if paramName == parameterActionName && paramVal != parameterActionValue { + // --action parameter with a differnet value from reboot is not supported 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 + } else if _, exist := fenceAgentParamNames[paramName]; !exist { + fenceAgentParamNames[paramName] = true + fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, paramVal) } } - // if --action attribute was not selected, then its 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) + nodeName := getNodeName(far) // append node parameters - nodeName := v1alpha1.NodeName(getNodeName(far)) for paramName, nodeMap := range far.Spec.NodeParameters { - if nodeVal, isFound := nodeMap[nodeName]; isFound { - fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, nodeVal) + if nodeVal, isFound := nodeMap[v1alpha1.NodeName(nodeName)]; isFound { + if _, exist := fenceAgentParamNames[paramName]; !exist { + fenceAgentParamNames[paramName] = true + fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, nodeVal) + } } else { err := errors.New(errorMissingNodeParams) logger.Error(err, "Missing matching nodeParam and CR's name") return nil, err } } + // Add the reboot action with its default value - https://github.com/ClusterLabs/fence-agents/blob/main/lib/fencing.py.py#L103 + if _, exist := fenceAgentParamNames[parameterActionName]; !exist { + logger.Info("`action` parameter is missing, so we add it with the default value of `reboot`") + fenceAgentParams = appendParamToSlice(fenceAgentParams, parameterActionName, parameterActionValue) + } return fenceAgentParams, nil } diff --git a/controllers/fenceagentsremediation_controller_test.go b/controllers/fenceagentsremediation_controller_test.go index 883962e2..86dfe439 100644 --- a/controllers/fenceagentsremediation_controller_test.go +++ b/controllers/fenceagentsremediation_controller_test.go @@ -64,8 +64,7 @@ var _ = Describe("FAR Controller", func() { node *corev1.Node underTestFAR = &v1alpha1.FenceAgentsRemediation{} ) - - invalidShareParam := map[v1alpha1.ParameterName]string{ + noActionShareParam := map[v1alpha1.ParameterName]string{ "--username": "admin", "--password": "password", "--ip": "192.168.111.1", @@ -78,6 +77,14 @@ var _ = Describe("FAR Controller", func() { "--ip": "192.168.111.1", "--lanplus": "", } + testShareParamTwice := map[v1alpha1.ParameterName]string{ + "--username": "admin", + "--ipport": "600", + "--password": "password", + "--action": "reboot", + "--ip": "192.168.111.1", + "--lanplus": "", + } testNodeParam := map[v1alpha1.ParameterName]map[v1alpha1.NodeName]string{ "--ipport": { "master-0": "6230", @@ -88,24 +95,22 @@ var _ = Describe("FAR Controller", func() { "worker-2": "6235", }, } - Context("Functionality", func() { BeforeEach(func() { plogs.Clear() underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) }) - Context("buildFenceAgentParams", func() { - When("FAR include different action than reboot", func() { - It("should succeed with a warning", func() { - invalidValTestFAR := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, invalidShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) - invalidShareString, err := buildFenceAgentParams(invalidValTestFAR) + When("FAR CR misses the action parameter", func() { + It("should succeed and add the action parameter with value reboot", func() { + testFARNoAction := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, noActionShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + noActionShareString, err := buildFenceAgentParams(testFARNoAction) Expect(err).NotTo(HaveOccurred()) underTestFAR.ObjectMeta.Name = workerNode validShareString, err := buildFenceAgentParams(underTestFAR) Expect(err).NotTo(HaveOccurred()) // Eventually buildFenceAgentParams would return the same shareParam - Expect(invalidShareString).To(ConsistOf(validShareString)) + Expect(noActionShareString).To(ConsistOf(validShareString)) }) }) When("FAR CR's name doesn't match a node name", func() { @@ -122,6 +127,23 @@ var _ = Describe("FAR Controller", func() { Expect(buildFenceAgentParams(underTestFAR)).Error().NotTo(HaveOccurred()) }) }) + When("FAR CR includes the 'ipport' parameter twice", func() { + It("should succeed with ipport `6233`", func() { + doublePortTestFAR := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParamTwice, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + Expect(buildFenceAgentParams(doublePortTestFAR)).Error().NotTo(HaveOccurred()) + Eventually(func(g Gomega) { + g.Expect(storedCommand).To(ConsistOf([]string{ + "fence_ipmilan", + "--lanplus", + "--password=password", + "--username=admin", + "--action=reboot", + "--ip=192.168.111.1", + "--ipport=600"})) + }, timeoutPreRemediation, pollInterval).Should(Succeed()) + + }) + }) }) }) @@ -130,7 +152,7 @@ var _ = Describe("FAR Controller", func() { conditionStatusPointer := func(status metav1.ConditionStatus) *metav1.ConditionStatus { return &status } BeforeEach(func() { - // Create two VAs and two pods, and at the end clean them up with DeferCleanup + // Create two pods and at the end clean them up with DeferCleanup testPod := createRunningPod("far-test-1", testPodName, workerNode) DeferCleanup(cleanupTestedResources, testPod) @@ -179,7 +201,6 @@ var _ = Describe("FAR Controller", func() { By("Verifying correct conditions for successful remediation") verifyRemediationConditions( underTestFAR, - workerNode, conditionStatusPointer(metav1.ConditionFalse), // ProcessingTypeStatus conditionStatusPointer(metav1.ConditionTrue), // FenceAgentActionSucceededTypeStatus conditionStatusPointer(metav1.ConditionTrue)) // SucceededTypeStatus @@ -191,7 +212,7 @@ var _ = Describe("FAR Controller", func() { underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) }) When("node name is stored in remediation name", func() { - It("should have finalizer, taint, while the two VAs and one pod will be deleted", testSuccessfulRemediation) + It("should have finalizer and taint, while the tested pod will be deleted", testSuccessfulRemediation) }) //remediation is created from escalation remediation supporting same kind template When("node name is stored in remediation's annotation", func() { @@ -199,7 +220,7 @@ var _ = Describe("FAR Controller", func() { underTestFAR.Name = fmt.Sprintf("%s-%s", workerNode, "pseudo-random-test-sufix") underTestFAR.Annotations = map[string]string{"remediation.medik8s.io/node-name": workerNode} }) - It("should have finalizer, taint, while the two VAs and one pod will be deleted", testSuccessfulRemediation) + It("should have finalizer and taint, while the tested pod will be deleted", testSuccessfulRemediation) }) }) @@ -210,7 +231,7 @@ var _ = Describe("FAR Controller", func() { underTestFAR = getFenceAgentsRemediation(dummyNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) }) - It("should not have a finalizer nor taint, while the two VAs and one pod will remain", func() { + It("should not have a finalizer nor taint, while the tested pod will remain", func() { By("Not finding a matching node to FAR CR's name") Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: underTestFAR.Name}, node)).To(Not(Succeed())) @@ -233,7 +254,6 @@ var _ = Describe("FAR Controller", func() { By("Verifying correct conditions for unsuccessful remediation") verifyRemediationConditions( underTestFAR, - dummyNode, conditionStatusPointer(metav1.ConditionFalse), // ProcessingTypeStatus conditionStatusPointer(metav1.ConditionFalse), // FenceAgentActionSucceededTypeStatus conditionStatusPointer(metav1.ConditionFalse)) // SucceededTypeStatus @@ -246,7 +266,6 @@ var _ = Describe("FAR Controller", func() { BeforeEach(func() { plogs.Clear() node = utils.GetNode("", workerNode) - underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) }) @@ -328,7 +347,6 @@ var _ = Describe("FAR Controller", func() { By("Verifying correct conditions for un-successful remediation") verifyRemediationConditions( underTestFAR, - workerNode, conditionStatusPointer(metav1.ConditionFalse), // ProcessingTypeStatus conditionStatusPointer(metav1.ConditionFalse), // FenceAgentActionSucceededTypeStatus conditionStatusPointer(metav1.ConditionFalse)) // SucceededTypeStatus @@ -358,7 +376,6 @@ var _ = Describe("FAR Controller", func() { By("Verifying correct conditions for un-successful remediation") verifyRemediationConditions( underTestFAR, - workerNode, conditionStatusPointer(metav1.ConditionFalse), // ProcessingTypeStatus conditionStatusPointer(metav1.ConditionFalse), // FenceAgentActionSucceededTypeStatus conditionStatusPointer(metav1.ConditionFalse)) // SucceededTypeStatus @@ -427,7 +444,6 @@ var _ = Describe("FAR Controller", func() { By("Verifying correct conditions for successful remediation") verifyRemediationConditions( underTestFAR, - workerNode, conditionStatusPointer(metav1.ConditionFalse), // ProcessingTypeStatus conditionStatusPointer(metav1.ConditionTrue), // FenceAgentActionSucceededTypeStatus conditionStatusPointer(metav1.ConditionTrue)) // SucceededTypeStatus @@ -537,7 +553,7 @@ func verifyPodExists(podName string) { } // verifyStatusCondition checks if the status condition is not set, and if it is set then it has an expected value -func verifyStatusCondition(far *v1alpha1.FenceAgentsRemediation, nodeName, conditionType string, conditionStatus *metav1.ConditionStatus) { +func verifyStatusCondition(far *v1alpha1.FenceAgentsRemediation, conditionType string, conditionStatus *metav1.ConditionStatus) { Eventually(func(g Gomega) { //g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(far), far)).To(Succeed()) condition := meta.FindStatusCondition(far.Status.Conditions, conditionType) @@ -569,13 +585,15 @@ func verifyPreRemediationSucceed(underTestFAR *v1alpha1.FenceAgentsRemediation, } func verifyEvent(eventType, eventReason, eventMessage string) { - By(fmt.Sprintf(eventExist, eventReason)) + eventText := fmt.Sprintf(eventExist, eventReason) + By(eventText) isEventMatch := isEventOccurred(eventType, eventReason, eventMessage) ExpectWithOffset(1, isEventMatch).To(BeTrue()) } func verifyNoEvent(eventType, eventReason, eventMessage string) { - By(fmt.Sprintf(eventNotExist, eventReason)) + eventText := fmt.Sprintf(eventNotExist, eventReason) + By(eventText) isEventMatch := isEventOccurred(eventType, eventReason, eventMessage) ExpectWithOffset(1, isEventMatch).To(BeFalse()) } @@ -618,14 +636,14 @@ func clearEvents() { log.Info("Cleanup: events list is empty") } -func verifyRemediationConditions(far *v1alpha1.FenceAgentsRemediation, nodeName string, processingTypeConditionStatus, fenceAgentSuccededTypeConditionStatus, succededTypeConditionStatus *metav1.ConditionStatus) { +func verifyRemediationConditions(far *v1alpha1.FenceAgentsRemediation, processingTypeConditionStatus, fenceAgentSuccededTypeConditionStatus, succededTypeConditionStatus *metav1.ConditionStatus) { EventuallyWithOffset(1, func(g Gomega) { - ut := &v1alpha1.FenceAgentsRemediation{} - g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(far), ut)).To(Succeed()) - g.Expect(ut.Status.LastUpdateTime).ToNot(BeNil()) - verifyStatusCondition(ut, nodeName, commonConditions.ProcessingType, processingTypeConditionStatus) - verifyStatusCondition(ut, nodeName, utils.FenceAgentActionSucceededType, fenceAgentSuccededTypeConditionStatus) - verifyStatusCondition(ut, nodeName, commonConditions.SucceededType, succededTypeConditionStatus) + farCR := &v1alpha1.FenceAgentsRemediation{} + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(far), farCR)).To(Succeed()) + g.Expect(farCR.Status.LastUpdateTime).ToNot(BeNil()) + verifyStatusCondition(farCR, commonConditions.ProcessingType, processingTypeConditionStatus) + verifyStatusCondition(farCR, utils.FenceAgentActionSucceededType, fenceAgentSuccededTypeConditionStatus) + verifyStatusCondition(farCR, commonConditions.SucceededType, succededTypeConditionStatus) }) } From 88e14f43559852a9b856f4e705db73ccacdfada0 Mon Sep 17 00:00:00 2001 From: oraz Date: Sun, 12 Jan 2025 16:48:02 +0200 Subject: [PATCH 2/3] Introduce CredentialParameters spec field The field will be used to identify the parameters we fetch from Kubernetes Secret --- api/v1alpha1/fenceagentsremediation_types.go | 8 +++++++- api/v1alpha1/zz_generated.deepcopy.go | 5 +++++ ...ce-agents-remediation.clusterserviceversion.yaml | 10 ++++++++++ ...ediation.medik8s.io_fenceagentsremediations.yaml | 7 +++++++ ....medik8s.io_fenceagentsremediationtemplates.yaml | 7 +++++++ ...ediation.medik8s.io_fenceagentsremediations.yaml | 13 ++++++++++--- ....medik8s.io_fenceagentsremediationtemplates.yaml | 13 ++++++++++--- ...ce-agents-remediation.clusterserviceversion.yaml | 10 ++++++++++ 8 files changed, 66 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/fenceagentsremediation_types.go b/api/v1alpha1/fenceagentsremediation_types.go index f0c21605..2594976c 100644 --- a/api/v1alpha1/fenceagentsremediation_types.go +++ b/api/v1alpha1/fenceagentsremediation_types.go @@ -81,7 +81,8 @@ type FenceAgentsRemediationSpec struct { //+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) + // 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"` @@ -89,6 +90,11 @@ type FenceAgentsRemediationSpec struct { //+operator-sdk:csv:customresourcedefinitions:type=spec NodeParameters map[ParameterName]map[NodeName]string `json:"nodeparameters,omitempty"` + // CredentialParameters are passed to the fencing agent according to the node that is fenced, and the parameters + // values are fetched from a known secret + //+operator-sdk:csv:customresourcedefinitions:type=spec + CredentialParameters []ParameterName `json:"credentialparameters,omitempty"` + // RemediationStrategy is the remediation method for unhealthy nodes. // Currently, it could be either "OutOfServiceTaint" or "ResourceDeletion". // ResourceDeletion will iterate over all pods related to the unhealthy node and delete them. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 58b17c94..9efc967c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -114,6 +114,11 @@ func (in *FenceAgentsRemediationSpec) DeepCopyInto(out *FenceAgentsRemediationSp (*out)[key] = outVal } } + if in.CredentialParameters != nil { + in, out := &in.CredentialParameters, &out.CredentialParameters + *out = make([]ParameterName, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FenceAgentsRemediationSpec. diff --git a/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml b/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml index 34f82019..d672ee60 100644 --- a/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml +++ b/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml @@ -80,6 +80,11 @@ spec: have a fence_ prefix. displayName: Agent path: agent + - description: CredentialParameters are passed to the fencing agent according + to the node that is fenced, and the parameters values are fetched from a + known secret + displayName: Credential Parameters + path: credentialparameters - description: NodeParameters are passed to the fencing agent according to the node that is fenced, since they are node specific displayName: Node Parameters @@ -136,6 +141,11 @@ spec: have a fence_ prefix. displayName: Agent path: template.spec.agent + - description: CredentialParameters are passed to the fencing agent according + to the node that is fenced, and the parameters values are fetched from a + known secret + displayName: Credential Parameters + path: template.spec.credentialparameters - description: NodeParameters are passed to the fencing agent according to the node that is fenced, since they are node specific displayName: Node Parameters diff --git a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml index 2c43d825..1337d7d3 100644 --- a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml +++ b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml @@ -50,6 +50,13 @@ spec: It should have a fence_ prefix. pattern: fence_.+ type: string + credentialparameters: + description: CredentialParameters are passed to the fencing agent + according to the node that is fenced, and the parameters values + are fetched from a known secret + items: + type: string + type: array nodeparameters: additionalProperties: additionalProperties: diff --git a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml index 45cfd331..648a9dad 100644 --- a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml +++ b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml @@ -58,6 +58,13 @@ spec: It should have a fence_ prefix. pattern: fence_.+ type: string + credentialparameters: + description: CredentialParameters are passed to the fencing + agent according to the node that is fenced, and the parameters + values are fetched from a known secret + items: + type: string + type: array nodeparameters: additionalProperties: additionalProperties: diff --git a/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml b/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml index 3533e534..85a34ec0 100644 --- a/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml +++ b/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml @@ -48,6 +48,13 @@ spec: It should have a fence_ prefix. pattern: fence_.+ type: string + credentialparameters: + description: |- + CredentialParameters are passed to the fencing agent according to the node that is fenced, and the parameters + values are fetched from a known secret + items: + type: string + type: array nodeparameters: additionalProperties: additionalProperties: @@ -82,9 +89,9 @@ spec: 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) + 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 diff --git a/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml b/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml index b196f9f9..d3666a87 100644 --- a/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml +++ b/config/crd/bases/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml @@ -56,6 +56,13 @@ spec: It should have a fence_ prefix. pattern: fence_.+ type: string + credentialparameters: + description: |- + CredentialParameters are passed to the fencing agent according to the node that is fenced, and the parameters + values are fetched from a known secret + items: + type: string + type: array nodeparameters: additionalProperties: additionalProperties: @@ -91,9 +98,9 @@ spec: 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) + 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 diff --git a/config/manifests/bases/fence-agents-remediation.clusterserviceversion.yaml b/config/manifests/bases/fence-agents-remediation.clusterserviceversion.yaml index d75d98b4..b8a24a3b 100644 --- a/config/manifests/bases/fence-agents-remediation.clusterserviceversion.yaml +++ b/config/manifests/bases/fence-agents-remediation.clusterserviceversion.yaml @@ -35,6 +35,11 @@ spec: have a fence_ prefix. displayName: Agent path: agent + - description: CredentialParameters are passed to the fencing agent according + to the node that is fenced, and the parameters values are fetched from a + known secret + displayName: Credential Parameters + path: credentialparameters - description: NodeParameters are passed to the fencing agent according to the node that is fenced, since they are node specific displayName: Node Parameters @@ -91,6 +96,11 @@ spec: have a fence_ prefix. displayName: Agent path: template.spec.agent + - description: CredentialParameters are passed to the fencing agent according + to the node that is fenced, and the parameters values are fetched from a + known secret + displayName: Credential Parameters + path: template.spec.credentialparameters - description: NodeParameters are passed to the fencing agent according to the node that is fenced, since they are node specific displayName: Node Parameters From dce8e246fe91cbd931f3f6136471337857727850 Mon Sep 17 00:00:00 2001 From: oraz Date: Sun, 12 Jan 2025 17:04:25 +0200 Subject: [PATCH 3/3] Fetch credentials from CredentialsSecretName secret User needs to create a secret with the name of the unhealthy node with all the credential parameters, and values, so FAR pod can fetch them when building the agent command --- Makefile | 5 +- README.md | 6 +- ...nts-remediation.clusterserviceversion.yaml | 8 ++ ...on.medik8s.io_fenceagentsremediations.yaml | 12 +- ...8s.io_fenceagentsremediationtemplates.yaml | 10 +- config/rbac/role.yaml | 8 ++ .../fenceagentsremediation_controller.go | 82 ++++++++++-- .../fenceagentsremediation_controller_test.go | 123 +++++++++++++----- 8 files changed, 202 insertions(+), 52 deletions(-) diff --git a/Makefile b/Makefile index 6b1c099e..31494357 100644 --- a/Makefile +++ b/Makefile @@ -23,6 +23,8 @@ ENVTEST_K8S_VERSION = 1.28 # OCP Version: for OKD bundle community OCP_VERSION = 4.14 +# Run unit tests once unless this parameter is set +REPEAT_TIMES ?= 1 # update for major version updates to YQ_VERSION! see https://github.com/mikefarah/yq YQ_API_VERSION = v4 YQ_VERSION = v4.44.2 @@ -196,7 +198,8 @@ test: test-no-verify ## Generate and format code, run tests, generate manifests # --vv: If set, emits with maximal verbosity - includes skipped and pending tests. test-no-verify: go-verify manifests generate fmt vet fix-imports envtest ginkgo # Generate and format code, and run tests KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(ENVTEST_DIR)/$(ENVTEST_VERSION) -p path)" \ - $(GINKGO) -r --keep-going --randomize-all --require-suite --vv --coverprofile cover.out ./api/... ./pkg/... ./controllers/... + $(GINKGO) -r --keep-going --randomize-all --require-suite --vv --coverprofile cover.out --repeat=$(REPEAT_TIMES) \ + ./api/... ./pkg/... ./controllers/... .PHONY: bundle-run bundle-run: operator-sdk create-ns ## Run bundle image. Default NS is "openshift-workload-availability", redefine OPERATOR_NAMESPACE to override it. diff --git a/README.md b/README.md index 0653a967..4442c035 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,7 @@ The FAR CR, `FenceAgentsRemediation`, is created by the admin and is used to tri The CR includes the following parameters: * `agent` - fence agent name. File name which is validated (by kubebuilder and Webhook) against a list of supported agents in the FAR pod. +* `credentialarameters` - credential parameters for accessing the node to be remediated. * `sharedparameters` - cluster wide parameters for executing the fence agent. * `nodeparameters` - node specific parameters for executing the fence agent. * `retrycount` - number of times to retry the fence agent in case of failure. The default is 5. @@ -209,9 +210,10 @@ spec: retrycount: 5 retryinterval: "5s" timeout: "60s" + credentialparameters: + --password sharedparameters: --username: "admin" - --password: "password" --lanplus: "" --action: "reboot" --ip: "192.168.111.1" @@ -223,7 +225,7 @@ spec: worker-0: "6233" worker-1: "6234" worker-2: "6235" - remediationStrategy: ResourceDeletion + remediationStrategy: OutOfServiceTaint ``` ## Tests diff --git a/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml b/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml index d672ee60..d8f88c87 100644 --- a/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml +++ b/bundle/manifests/fence-agents-remediation.clusterserviceversion.yaml @@ -216,6 +216,14 @@ spec: - list - update - watch + - apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml index 1337d7d3..10860bbf 100644 --- a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml +++ b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediations.yaml @@ -51,9 +51,9 @@ spec: pattern: fence_.+ type: string credentialparameters: - description: CredentialParameters are passed to the fencing agent - according to the node that is fenced, and the parameters values - are fetched from a known secret + description: |- + CredentialParameters are passed to the fencing agent according to the node that is fenced, and the parameters + values are fetched from a known secret items: type: string type: array @@ -91,9 +91,9 @@ spec: 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) + 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 diff --git a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml index 648a9dad..b681e954 100644 --- a/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml +++ b/bundle/manifests/fence-agents-remediation.medik8s.io_fenceagentsremediationtemplates.yaml @@ -59,8 +59,8 @@ spec: pattern: fence_.+ type: string credentialparameters: - description: CredentialParameters are passed to the fencing - agent according to the node that is fenced, and the parameters + description: |- + CredentialParameters are passed to the fencing agent according to the node that is fenced, and the parameters values are fetched from a known secret items: type: string @@ -100,9 +100,9 @@ spec: 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) + 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 diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a09bb297..43c839ae 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -22,6 +22,14 @@ rules: - list - update - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/controllers/fenceagentsremediation_controller.go b/controllers/fenceagentsremediation_controller.go index 08519f05..2fd25c71 100644 --- a/controllers/fenceagentsremediation_controller.go +++ b/controllers/fenceagentsremediation_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/go-logr/logr" @@ -28,6 +29,7 @@ import ( commonEvents "github.com/medik8s/common/pkg/events" commonResources "github.com/medik8s/common/pkg/resources" + corev1 "k8s.io/api/core/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,12 +48,14 @@ import ( const ( // errors - errorMissingParams = "nodeParameters or sharedParameters or both are missing, and they cannot be empty" - errorMissingNodeParams = "node parameter is required, and cannot be empty" + errorMissingParams = "nodeParameters or sharedParameters or both are missing, and they cannot be empty" + errorMissingNodeParams = "node parameter is required, and cannot be empty" + errorFailFetchingSecret = "failed to fetch secret key `%s` from secret `%s` at namespace `%s`: %w" SuccessFAResponse = "Success: Rebooted" parameterActionName = "--action" parameterActionValue = "reboot" + DefaultSecretName = "defaultSecret" ) // FenceAgentsRemediationReconciler reconciles a FenceAgentsRemediation object @@ -73,6 +77,7 @@ func (r *FenceAgentsRemediationReconciler) SetupWithManager(mgr ctrl.Manager) er //+kubebuilder:rbac:groups=storage.k8s.io,resources=volumeattachments,verbs=get;list;watch;delete //+kubebuilder:rbac:groups=core,resources=pods/exec,verbs=create //+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;update;delete;deletecollection +//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch;update;delete //+kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch //+kubebuilder:rbac:groups=fence-agents-remediation.medik8s.io,resources=fenceagentsremediations,verbs=get;list;watch;create;update;patch;delete @@ -226,14 +231,15 @@ func (r *FenceAgentsRemediationReconciler) Reconcile(ctx context.Context, req ct } r.Log.Info("Build fence agent command line", "Fence Agent", far.Spec.Agent, "Node Name", node.Name) - faParams, err := buildFenceAgentParams(far) + faParams, err := buildFenceAgentParams(far, ctx, r.Client) if err != nil { - r.Log.Error(err, "Invalid shared or node parameters from CR", "Node Name", node.Name, "CR Name", req.Name) + r.Log.Error(err, "Invalid credential/shared/node parameter from CR", "Node Name", node.Name, "CR Name", req.Name) return emptyResult, nil } cmd := append([]string{far.Spec.Agent}, faParams...) - r.Log.Info("Execute the fence agent", "Fence Agent", far.Spec.Agent, "Node Name", node.Name, "FAR uid", far.GetUID()) + //TODO remove the below change after testing + r.Log.Info("Execute the fence agent", "Fence Agent", far.Spec.Agent, "Node Name", node.Name, "FAR uid", far.GetUID(), "command", cmd) r.Executor.AsyncExecute(ctx, far.GetUID(), cmd, far.Spec.RetryCount, far.Spec.RetryInterval.Duration, far.Spec.Timeout.Duration) commonEvents.NormalEvent(r.Recorder, far, utils.EventReasonFenceAgentExecuted, utils.EventMessageFenceAgentExecuted) return emptyResult, nil @@ -337,9 +343,51 @@ func getNodeName(far *v1alpha1.FenceAgentsRemediation) string { return far.GetName() } +// fetchSecret fetches a secret and returns an error on failure +func fetchSecret(ctx context.Context, c client.Client, secretKeyObj client.ObjectKey) (*corev1.Secret, error) { + secret := &corev1.Secret{} + if err := c.Get(ctx, secretKeyObj, secret); err != nil { + return nil, err + } + return secret, nil +} + +// resolveParameterValueFromSecret resolve parameter value from secret if exist, otherwise returns an error +func resolveParameterValueFromSecret(ctx context.Context, c client.Client, far *v1alpha1.FenceAgentsRemediation, paramName v1alpha1.ParameterName) (string, error) { + var secret *corev1.Secret + var err error + logger := ctrl.Log.WithName("resolve-value-from-secret") + secretName := far.Name + // fetch secret/node name from remediation's annotation if present + if annotatedName, exist := far.Annotations["remediation.medik8s.io/node-name"]; exist { + secretName = annotatedName + } + secret, err = fetchSecret(ctx, c, client.ObjectKey{Name: secretName, Namespace: far.Namespace}) + if err != nil { + if apiErrors.IsNotFound(err) { + // when there is no specefic secret, then we try the default secret + logger.Info("secret is missing - try default secret", "secret name", secretName, "default secret", DefaultSecretName, "namespace", far.Namespace, "paramter mame", string(paramName)) + secretName = DefaultSecretName + secret, err = fetchSecret(ctx, c, client.ObjectKey{Name: secretName, Namespace: far.Namespace}) + } + if err != nil { + logger.Error(err, "failed to fetch secret", "secret name", secretName, "namespace", far.Namespace) + return "", fmt.Errorf(errorFailFetchingSecret, paramName, secretName, far.Namespace, err) + } + } + // Extract the secret value + secretValue, exists := secret.Data[string(paramName)] + if !exists { + return "", fmt.Errorf("secret key `%s` was not found in secret `%s` at namespace `%s`", paramName, secretName, far.Namespace) + } + logger.Info("found a value from secret", "secret name", secretName, "paramter mame", string(paramName), "secret value", secretValue) + + return string(secretValue), nil +} + // 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 nodeParameter name, or it has an action which is different from reboot, then return an error -func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, error) { +func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation, ctx context.Context, c client.Client) ([]string, error) { logger := ctrl.Log.WithName("build-fa-parameters") if far.Spec.NodeParameters == nil || far.Spec.SharedParameters == nil { err := errors.New(errorMissingParams) @@ -376,6 +424,20 @@ func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, erro return nil, err } } + + // append credential parameters + for _, paramName := range far.Spec.CredentialParameters { + resolvedVal, err := resolveParameterValueFromSecret(ctx, c, far, paramName) + if err != nil { + return nil, fmt.Errorf("failed to resolve credential parameter: %w", err) + } else { + if _, exist := fenceAgentParamNames[paramName]; !exist { + fenceAgentParamNames[paramName] = true + fenceAgentParams = appendParamToSlice(fenceAgentParams, paramName, resolvedVal) + } + } + } + // Add the reboot action with its default value - https://github.com/ClusterLabs/fence-agents/blob/main/lib/fencing.py.py#L103 if _, exist := fenceAgentParamNames[parameterActionName]; !exist { logger.Info("`action` parameter is missing, so we add it with the default value of `reboot`") @@ -386,10 +448,14 @@ func buildFenceAgentParams(far *v1alpha1.FenceAgentsRemediation) ([]string, erro // appendParamToSlice appends parameters in a key-value manner, when value can be empty func appendParamToSlice(fenceAgentParams []string, paramName v1alpha1.ParameterName, paramVal string) []string { + stringParam := string(paramName) + if !strings.HasPrefix(stringParam, "--") { + stringParam = "--" + stringParam + } if paramVal != "" { - fenceAgentParams = append(fenceAgentParams, fmt.Sprintf("%s=%s", paramName, paramVal)) + fenceAgentParams = append(fenceAgentParams, fmt.Sprintf("%s=%s", stringParam, paramVal)) } else { - fenceAgentParams = append(fenceAgentParams, string(paramName)) + fenceAgentParams = append(fenceAgentParams, stringParam) } return fenceAgentParams } diff --git a/controllers/fenceagentsremediation_controller_test.go b/controllers/fenceagentsremediation_controller_test.go index 86dfe439..2487a8e3 100644 --- a/controllers/fenceagentsremediation_controller_test.go +++ b/controllers/fenceagentsremediation_controller_test.go @@ -61,9 +61,21 @@ var ( var _ = Describe("FAR Controller", func() { var ( - node *corev1.Node - underTestFAR = &v1alpha1.FenceAgentsRemediation{} + node *corev1.Node + underTestFAR = &v1alpha1.FenceAgentsRemediation{} + credentialSecret = &corev1.Secret{} ) + + testCredentialParam := []v1alpha1.ParameterName{ + "--pass", + "--pass2", + } + invalidCredentialParam := []v1alpha1.ParameterName{ + "--pass", + "--no-pass", + } + emptyCredentialParam := []v1alpha1.ParameterName{} + noActionShareParam := map[v1alpha1.ParameterName]string{ "--username": "admin", "--password": "password", @@ -98,16 +110,16 @@ var _ = Describe("FAR Controller", func() { Context("Functionality", func() { BeforeEach(func() { plogs.Clear() - underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, emptyCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) }) Context("buildFenceAgentParams", func() { When("FAR CR misses the action parameter", func() { It("should succeed and add the action parameter with value reboot", func() { - testFARNoAction := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, noActionShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) - noActionShareString, err := buildFenceAgentParams(testFARNoAction) + testFARNoAction := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, noActionShareParam, testNodeParam, emptyCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) + noActionShareString, err := buildFenceAgentParams(testFARNoAction, context.Background(), k8sClient) Expect(err).NotTo(HaveOccurred()) underTestFAR.ObjectMeta.Name = workerNode - validShareString, err := buildFenceAgentParams(underTestFAR) + validShareString, err := buildFenceAgentParams(underTestFAR, context.Background(), k8sClient) Expect(err).NotTo(HaveOccurred()) // Eventually buildFenceAgentParams would return the same shareParam Expect(noActionShareString).To(ConsistOf(validShareString)) @@ -116,7 +128,7 @@ var _ = Describe("FAR Controller", func() { When("FAR CR's name doesn't match a node name", func() { It("should fail", func() { underTestFAR.ObjectMeta.Name = dummyNode - _, err := buildFenceAgentParams(underTestFAR) + _, err := buildFenceAgentParams(underTestFAR, context.Background(), k8sClient) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(errors.New(errorMissingNodeParams))) }) @@ -124,24 +136,23 @@ var _ = Describe("FAR Controller", func() { When("FAR CR's name does match a node name", func() { It("should succeed", func() { underTestFAR.ObjectMeta.Name = workerNode - Expect(buildFenceAgentParams(underTestFAR)).Error().NotTo(HaveOccurred()) + Expect(buildFenceAgentParams(underTestFAR, context.Background(), k8sClient)).Error().NotTo(HaveOccurred()) }) }) When("FAR CR includes the 'ipport' parameter twice", func() { It("should succeed with ipport `6233`", func() { - doublePortTestFAR := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParamTwice, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) - Expect(buildFenceAgentParams(doublePortTestFAR)).Error().NotTo(HaveOccurred()) - Eventually(func(g Gomega) { - g.Expect(storedCommand).To(ConsistOf([]string{ - "fence_ipmilan", - "--lanplus", - "--password=password", - "--username=admin", - "--action=reboot", - "--ip=192.168.111.1", - "--ipport=600"})) - }, timeoutPreRemediation, pollInterval).Should(Succeed()) - + doublePortTestFAR := getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParamTwice, testNodeParam, emptyCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) + Expect(buildFenceAgentParams(doublePortTestFAR, context.Background(), k8sClient)).Error().NotTo(HaveOccurred()) + // Eventually(func(g Gomega) { + // g.Expect(storedCommand).To(ConsistOf([]string{ + // "fence_ipmilan", + // "--lanplus", + // "--password=password", + // "--username=admin", + // "--action=reboot", + // "--ip=192.168.111.1", + // "--ipport=600"})) + // }, timeoutPreRemediation, pollInterval).Should(Succeed()) }) }) }) @@ -150,7 +161,7 @@ var _ = Describe("FAR Controller", func() { Context("Reconcile with ResourceDeletion strategy", func() { farRemediationTaint := utils.CreateRemediationTaint() conditionStatusPointer := func(status metav1.ConditionStatus) *metav1.ConditionStatus { return &status } - + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, testCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) BeforeEach(func() { // Create two pods and at the end clean them up with DeferCleanup testPod := createRunningPod("far-test-1", testPodName, workerNode) @@ -164,6 +175,9 @@ var _ = Describe("FAR Controller", func() { // Create node, and FAR CR, and at the end clean them up with DeferCleanup Expect(k8sClient.Create(context.Background(), node)).To(Succeed()) DeferCleanup(k8sClient.Delete, context.Background(), node) + credentialSecret = getSecret(workerNode) + Expect(k8sClient.Create(context.Background(), credentialSecret)).To(Succeed()) + DeferCleanup(k8sClient.Delete, context.Background(), credentialSecret) Expect(k8sClient.Create(context.Background(), underTestFAR)).To(Succeed()) DeferCleanup(func() { @@ -185,6 +199,8 @@ var _ = Describe("FAR Controller", func() { Eventually(func(g Gomega) { g.Expect(storedCommand).To(ConsistOf([]string{ "fence_ipmilan", + "--pass=abc", + "--pass2=abc2", "--lanplus", "--password=password", "--username=admin", @@ -209,7 +225,7 @@ var _ = Describe("FAR Controller", func() { } BeforeEach(func() { node = utils.GetNode("", workerNode) - underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, testCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) }) When("node name is stored in remediation name", func() { It("should have finalizer and taint, while the tested pod will be deleted", testSuccessfulRemediation) @@ -222,13 +238,47 @@ var _ = Describe("FAR Controller", func() { }) It("should have finalizer and taint, while the tested pod will be deleted", testSuccessfulRemediation) }) + }) + When("creating invalid FAR secret parameter", func() { + BeforeEach(func() { + node = utils.GetNode("", workerNode) + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, invalidCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) + }) + + It("should have a finalizer and taint, while the tested pod will remain", func() { + underTestFAR = verifyPreRemediationSucceed(underTestFAR, defaultNamespace, &farRemediationTaint) + // TODO: catch the error message? + + // Eventually(func(g Gomega) { + // g.Expect(storedCommand).To(ConsistOf([]string{ + // "fence_ipmilan", + // "--lanplus", + // "--password=password", + // "--username=admin", + // "--action=reboot", + // "--ip=192.168.111.1", + // "--ipport=6233"})) + // }, timeoutPreRemediation, pollInterval).Should(Succeed()) + + By("Still having one test pod") + verifyPodExists(testPodName) + + By("Verifying correct conditions for unsuccessful remediation") + verifyRemediationConditions( + underTestFAR, + conditionStatusPointer(metav1.ConditionTrue), // ProcessingTypeStatus + conditionStatusPointer(metav1.ConditionFalse), // FenceAgentActionSucceededTypeStatus + conditionStatusPointer(metav1.ConditionFalse)) // SucceededTypeStatus + verifyNoEvent(corev1.EventTypeNormal, utils.EventReasonFenceAgentSucceeded, utils.EventMessageFenceAgentSucceeded) + verifyNoEvent(corev1.EventTypeNormal, utils.EventReasonNodeRemediationCompleted, utils.EventReasonNodeRemediationCompleted) + }) }) When("creating invalid FAR CR Name", func() { BeforeEach(func() { node = utils.GetNode("", workerNode) - underTestFAR = getFenceAgentsRemediation(dummyNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + underTestFAR = getFenceAgentsRemediation(dummyNode, fenceAgentIPMI, testShareParam, testNodeParam, emptyCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) }) It("should not have a finalizer nor taint, while the tested pod will remain", func() { @@ -266,7 +316,7 @@ var _ = Describe("FAR Controller", func() { BeforeEach(func() { plogs.Clear() node = utils.GetNode("", workerNode) - underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.ResourceDeletionRemediationStrategy) + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, emptyCredentialParam, v1alpha1.ResourceDeletionRemediationStrategy) }) When("CR is deleted in between fence agent retries", func() { @@ -417,7 +467,7 @@ var _ = Describe("FAR Controller", func() { When("creating valid FAR CR", func() { BeforeEach(func() { node = utils.GetNode("", workerNode) - underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, v1alpha1.OutOfServiceTaintRemediationStrategy) + underTestFAR = getFenceAgentsRemediation(workerNode, fenceAgentIPMI, testShareParam, testNodeParam, emptyCredentialParam, v1alpha1.OutOfServiceTaintRemediationStrategy) }) It("should have finalizer, both remediation taint and out-of-service taint, and at the end they will be deleted", func() { @@ -465,13 +515,14 @@ var _ = Describe("FAR Controller", func() { }) // getFenceAgentsRemediation assigns the input to the FenceAgentsRemediation -func getFenceAgentsRemediation(nodeName, agent string, sharedparameters map[v1alpha1.ParameterName]string, nodeparameters map[v1alpha1.ParameterName]map[v1alpha1.NodeName]string, strategy v1alpha1.RemediationStrategyType) *v1alpha1.FenceAgentsRemediation { +func getFenceAgentsRemediation(nodeName, agent string, sharedparameters map[v1alpha1.ParameterName]string, nodeparameters map[v1alpha1.ParameterName]map[v1alpha1.NodeName]string, credentialparameters []v1alpha1.ParameterName, strategy v1alpha1.RemediationStrategyType) *v1alpha1.FenceAgentsRemediation { return &v1alpha1.FenceAgentsRemediation{ ObjectMeta: metav1.ObjectMeta{Name: nodeName, Namespace: defaultNamespace}, Spec: v1alpha1.FenceAgentsRemediationSpec{ - Agent: agent, - SharedParameters: sharedparameters, - NodeParameters: nodeparameters, + Agent: agent, + SharedParameters: sharedparameters, + NodeParameters: nodeparameters, + CredentialParameters: credentialparameters, // Set the retry count to the minimum for the majority of the tests RetryCount: 1, RetryInterval: metav1.Duration{Duration: 5 * time.Second}, @@ -481,6 +532,18 @@ func getFenceAgentsRemediation(nodeName, agent string, sharedparameters map[v1al } } +// getSecret assigns the input to the FenceAgentsRemediation +func getSecret(nodeName string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName, Namespace: defaultNamespace}, + Data: map[string][]byte{ + "--pass": []byte("abc"), + "--pass2": []byte("abc2"), + }, + Type: corev1.SecretType("Opaque"), + } +} + // buildPod builds a dummy pod func buildPod(containerName, podName, nodeName string) *corev1.Pod { pod := &corev1.Pod{}