Skip to content

Commit

Permalink
Merge pull request #124 from clobrano/common-event-node-not-found-0
Browse files Browse the repository at this point in the history
Use Common Event API for Remediation cannot start due to node not found
  • Loading branch information
openshift-merge-bot[bot] authored Dec 8, 2024
2 parents 678120a + 4d385c2 commit d6259c5
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 37 deletions.
36 changes: 18 additions & 18 deletions controllers/machinedeletionremediation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ const (
noMachineAnnotationError = "failed to find openshift machine annotation on node name: %s"
invalidValueMachineAnnotationError = "failed to extract Machine Name and Machine Namespace from machine annotation on the node for node name: %s"
failedToDeleteMachineError = "failed to delete machine of node name: %s"
nodeNotFoundErrorMsg = "failed to fetch node"
machineNotFoundErrorMsg = "failed to fetch machine of node"
nodeNotFoundErrorMsg = "could not get the node"
machineNotFoundErrorMsg = "could not get node's machine"
noControllerOwnerErrorMsg = "ignoring remediation of the machine: the machine has no controller owner"
// Cluster Provider messages
machineDeletedOnCloudProviderMessage = "Machine will be deleted and the unhealthy node replaced. This is a Cloud cluster provider: the new node is expected to have a new name"
Expand All @@ -71,13 +71,13 @@ const (
type conditionChangeReason string

const (
remediationStarted conditionChangeReason = "RemediationStarted"
remediationTimedOutByNhc conditionChangeReason = "RemediationStoppedByNHC"
remediationFinishedMachineDeleted conditionChangeReason = "MachineDeleted"
remediationSkippedNodeNotFound conditionChangeReason = "RemediationSkippedNodeNotFound"
remediationSkippedMachineNotFound conditionChangeReason = "RemediationSkippedMachineNotFound"
remediationSkippedNoControllerOwner conditionChangeReason = "RemediationSkippedNoControllerOwner"
remediationFailed conditionChangeReason = "RemediationFailed"
remediationStarted conditionChangeReason = "RemediationStarted"
remediationTimedOutByNhc conditionChangeReason = "RemediationStoppedByNHC"
remediationFinishedMachineDeleted conditionChangeReason = "MachineDeleted"
remediationCannotStartNodeNotFound conditionChangeReason = "RemediationCannotStartNodeNotFound"
remediationCannotStartMachineNotFound conditionChangeReason = "RemediationCannotStartMachineNotFound"
remediationCannotStartNoControllerOwner conditionChangeReason = "RemediationCannotStartNoControllerOwner"
remediationFailed conditionChangeReason = "RemediationFailed"
)

var (
Expand Down Expand Up @@ -156,11 +156,11 @@ func (r *MachineDeletionRemediationReconciler) Reconcile(ctx context.Context, re
// situation. An error is returned only if it does not match the following custom errors, or
// updateConditions fails.
if err == nodeNotFoundError {
commonevents.WarningEvent(r.Recorder, mdr, string(remediationSkippedNodeNotFound), nodeNotFoundErrorMsg)
_, err = r.updateConditions(remediationSkippedNodeNotFound, mdr)
commonevents.GetTargetNodeFailed(r.Recorder, mdr)
_, err = r.updateConditions(remediationCannotStartNodeNotFound, mdr)
} else if err == machineNotFoundError {
commonevents.WarningEvent(r.Recorder, mdr, string(remediationSkippedMachineNotFound), machineNotFoundErrorMsg)
_, err = r.updateConditions(remediationSkippedMachineNotFound, mdr)
commonevents.WarningEvent(r.Recorder, mdr, string(remediationCannotStartMachineNotFound), machineNotFoundErrorMsg)
_, err = r.updateConditions(remediationCannotStartMachineNotFound, mdr)
} else if err == unrecoverableError {
commonevents.WarningEvent(r.Recorder, mdr, string(remediationFailed), unrecoverableError.Error())
_, err = r.updateConditions(remediationFailed, mdr)
Expand Down Expand Up @@ -233,8 +233,8 @@ func (r *MachineDeletionRemediationReconciler) Reconcile(ctx context.Context, re

if !hasControllerOwner(machine) {
log.Info(noControllerOwnerErrorMsg, "machine", machine.GetName(), "remediation name", mdr.Name)
commonevents.WarningEvent(r.Recorder, mdr, string(remediationSkippedNoControllerOwner), noControllerOwnerErrorMsg)
_, err = r.updateConditions(remediationSkippedNoControllerOwner, mdr)
commonevents.WarningEvent(r.Recorder, mdr, string(remediationCannotStartNoControllerOwner), noControllerOwnerErrorMsg)
_, err = r.updateConditions(remediationCannotStartNoControllerOwner, mdr)
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -418,9 +418,9 @@ func (r *MachineDeletionRemediationReconciler) updateConditions(reason condition
processingConditionStatus = metav1.ConditionFalse
succeededConditionStatus = metav1.ConditionTrue
case remediationTimedOutByNhc,
remediationSkippedNoControllerOwner,
remediationSkippedNodeNotFound,
remediationSkippedMachineNotFound,
remediationCannotStartNoControllerOwner,
remediationCannotStartNodeNotFound,
remediationCannotStartMachineNotFound,
remediationFailed:
processingConditionStatus = metav1.ConditionFalse
succeededConditionStatus = metav1.ConditionFalse
Expand Down
38 changes: 19 additions & 19 deletions controllers/machinedeletionremediation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,11 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
verifyMachineNotDeleted(workerNodeMachineName)
verifyMachineNotDeleted(masterNodeMachineName)
verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedNodeNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedNodeNotFound}})
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartNodeNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartNodeNotFound}})
verifyConditionUnset(commonconditions.PermanentNodeDeletionExpectedType)
verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedNodeNotFound", "failed to fetch node", true},
{v1.EventTypeWarning, "RemediationCannotStart", "Could not get remediation target Node", true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand All @@ -162,12 +162,12 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
verifyMachineNotDeleted(workerNodeMachineName)
verifyMachineNotDeleted(masterNodeMachineName)
verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
// Cluster provider is not set in this test
{commonconditions.PermanentNodeDeletionExpectedType, metav1.ConditionUnknown, v1alpha1.MachineDeletionOnUndefinedProviderReason}})
verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedNoControllerOwner", noControllerOwnerErrorMsg, true},
{v1.EventTypeWarning, string(remediationCannotStartNoControllerOwner), noControllerOwnerErrorMsg, true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand All @@ -184,12 +184,12 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
verifyMachineNotDeleted(workerNodeMachineName)
verifyMachineNotDeleted(masterNodeMachineName)
verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
// Cluster provider is not set in this test
{commonconditions.PermanentNodeDeletionExpectedType, metav1.ConditionUnknown, v1alpha1.MachineDeletionOnUndefinedProviderReason}})
verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedNoControllerOwner", noControllerOwnerErrorMsg, true},
{v1.EventTypeWarning, "RemediationCannotStartNoControllerOwner", noControllerOwnerErrorMsg, true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand All @@ -207,13 +207,13 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
verifyMachineNotDeleted(workerNodeMachineName)
verifyMachineNotDeleted(masterNodeMachineName)
verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedNoControllerOwner},
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartNoControllerOwner},
// Cluster provider is not set in this test
{commonconditions.PermanentNodeDeletionExpectedType, metav1.ConditionUnknown, v1alpha1.MachineDeletionOnUndefinedProviderReason}})

verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedNoControllerOwner", noControllerOwnerErrorMsg, true},
{v1.EventTypeWarning, "RemediationCannotStartNoControllerOwner", noControllerOwnerErrorMsg, true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand Down Expand Up @@ -366,11 +366,11 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
}, 30*time.Second, 1*time.Second).Should(BeTrue())

verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedNodeNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedNodeNotFound}})
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartNodeNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartNodeNotFound}})
verifyConditionUnset(commonconditions.PermanentNodeDeletionExpectedType)
verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedNodeNotFound", "failed to fetch node", true},
{v1.EventTypeWarning, "RemediationCannotStart", "Could not get remediation target Node", true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand Down Expand Up @@ -452,17 +452,17 @@ var _ = Describe("Machine Deletion Remediation CR", func() {
Expect(k8sClient.Update(context.Background(), masterNode)).ToNot(HaveOccurred())
})

It("failed to fetch machine error", func() {
It("failed to get machine error", func() {
Eventually(func() bool {
return plogs.Contains(machineNotFoundErrorMsg)
}, 30*time.Second, 1*time.Second).Should(BeTrue())

verifyConditionsMatch([]expectedCondition{
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationSkippedMachineNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationSkippedMachineNotFound}})
{commonconditions.ProcessingType, metav1.ConditionFalse, remediationCannotStartMachineNotFound},
{commonconditions.SucceededType, metav1.ConditionFalse, remediationCannotStartMachineNotFound}})
verifyConditionUnset(commonconditions.PermanentNodeDeletionExpectedType)
verifyEvents([]expectedEvent{
{v1.EventTypeWarning, "RemediationSkippedMachineNotFound", "failed to fetch machine of node", true},
{v1.EventTypeWarning, "RemediationCannotStartMachineNotFound", "could not get node's machine", true},
{v1.EventTypeNormal, "RemediationStarted", "Remediation started", false},
})
})
Expand Down

0 comments on commit d6259c5

Please sign in to comment.