Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Two Reasons for Updating and Stopping Remediation #76

Merged
merged 5 commits into from
Aug 16, 2023
Merged

Add Two Reasons for Updating and Stopping Remediation #76

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9126ce4
Add two reasons for updating and stoping remediation
razo7 Aug 14, 2023
8005d8a
Don't stop update of condition from false to true
razo7 Aug 14, 2023
6f62b11
Better reason for NHC annotation signal
razo7 Aug 15, 2023
01241d6
Separate condition message for RemediationFinishedNodeNotFound and Re…
razo7 Aug 15, 2023
c0e69f2
Switch case usage for conditionMessage of a failure
razo7 Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions controllers/fenceagentsremediation_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,17 @@ func updateConditions(reason v1alpha1.ConditionsChangeReason, currentConditions
processingConditionStatus = metav1.ConditionFalse
fenceAgentActionSucceededConditionStatus = metav1.ConditionFalse
succeededConditionStatus = metav1.ConditionFalse
if reason == v1alpha1.RemediationFinishedNodeNotFound {
// Different reasons share the same effect to the conditions, but they have different message
switch reason {
case v1alpha1.RemediationFinishedNodeNotFound:
conditionMessage = v1alpha1.RemediationFinishedNodeNotFoundConditionMessage
} else {
case v1alpha1.RemediationInterruptedByNHC:
conditionMessage = v1alpha1.RemediationInterruptedByNHCConditionMessage
default:
err := fmt.Errorf("unknown ConditionsChangeReason:%s", reason)
log.Error(err, "couldn't update FAR Status Conditions")
return err
}

case v1alpha1.RemediationStarted:
processingConditionStatus = metav1.ConditionTrue
fenceAgentActionSucceededConditionStatus = metav1.ConditionUnknown
Expand Down
4 changes: 2 additions & 2 deletions controllers/fenceagentsremediation_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ var _ = Describe("FAR Controller", func() {
testVADeletion(vaName2, resourceDeletionWasTriggered)
testPodDeletion(testPodName, resourceDeletionWasTriggered)

By("Having Succeed condition set to true")
By("Having Succeeded, FenceAgentActionSucceeded conditions set to true, and Processing set to false")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a general note: I would avoid being too detailed in the these comments, and use something more generic like "Verifying correct conditions". The details are very readable in the actual test code, and in the test logs in case a test fails. That way you don't need to update the comment ever time the test changes 🙂

verifyStatusCondition(workerNode, commonConditions.ProcessingType, conditionStatusPointer(metav1.ConditionFalse))
verifyStatusCondition(workerNode, v1alpha1.FenceAgentActionSucceededType, conditionStatusPointer(metav1.ConditionTrue))
verifyStatusCondition(workerNode, commonConditions.SucceededType, conditionStatusPointer(metav1.ConditionTrue))
Expand Down Expand Up @@ -199,7 +199,7 @@ var _ = Describe("FAR Controller", func() {
testVADeletion(vaName2, resourceDeletionWasTriggered)
testPodDeletion(testPodName, resourceDeletionWasTriggered)

By("Not having any condition set")
By("Having all three conditions set to false")
verifyStatusCondition(dummyNode, commonConditions.ProcessingType, conditionStatusPointer(metav1.ConditionFalse))
verifyStatusCondition(dummyNode, v1alpha1.FenceAgentActionSucceededType, conditionStatusPointer(metav1.ConditionFalse))
verifyStatusCondition(dummyNode, commonConditions.SucceededType, conditionStatusPointer(metav1.ConditionFalse))
Expand Down