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

Improve validation of VR conditions #1643

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+64 −96
Open

Improve validation of VR conditions #1643

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
afa9ef7
Remove stale comment
nirs Nov 11, 2024
e4c00d2
Document conditionState constants
nirs Nov 7, 2024
4b7320b
Always log error message for VR Validated condition
nirs Nov 7, 2024
689837d
Log the error message from the VR condition
nirs Nov 7, 2024
2f34273
Clarify use of error message from isVRConditionMet()
nirs Nov 7, 2024
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
160 changes: 64 additions & 96 deletions internal/controller/vrg_volrep.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,13 +1505,17 @@ func (v *VRGInstance) validateVRValidatedStatus(
volRep *volrep.VolumeReplication,
) (bool, conditionState) {
conditionMet, condState, errorMsg := isVRConditionMet(volRep, volrep.ConditionValidated, metav1.ConditionTrue)
if !conditionMet && condState != conditionMissing {
defaultMsg := "VolumeReplication resource not validated"
v.updatePVCDataReadyConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg,
defaultMsg)
v.updatePVCDataProtectedConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg,
defaultMsg)
v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", defaultMsg, volRep.GetName(), volRep.GetNamespace()))
if !conditionMet {
if errorMsg == "" {
errorMsg = "VolumeReplication resource not validated"
}
// The condition does not exist when using csi-addons < 0.10.0, so we cannot treat this as a failure.
if condState != conditionMissing {
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
}

nirs marked this conversation as resolved.
Show resolved Hide resolved
v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", errorMsg, volRep.GetName(), volRep.GetNamespace()))
}

return conditionMet, condState
Expand All @@ -1523,28 +1527,30 @@ func (v *VRGInstance) validateVRValidatedStatus(
func (v *VRGInstance) validateVRCompletedStatus(pvc *corev1.PersistentVolumeClaim, volRep *volrep.VolumeReplication,
state ramendrv1alpha1.ReplicationState,
) bool {
var (
stateString string
action string
)
conditionMet, _, errorMsg := isVRConditionMet(volRep, volrep.ConditionCompleted, metav1.ConditionTrue)
if !conditionMet {
if errorMsg == "" {
var (
stateString string
action string
)

switch state {
case ramendrv1alpha1.Primary:
stateString = "primary"
action = "promoted"
case ramendrv1alpha1.Secondary:
stateString = "secondary"
action = "demoted"
}

switch state {
case ramendrv1alpha1.Primary:
stateString = "primary"
action = "promoted"
case ramendrv1alpha1.Secondary:
stateString = "secondary"
action = "demoted"
}
errorMsg = fmt.Sprintf("VolumeReplication resource for pvc not %s to %s", action, stateString)
}

conditionMet, _, msg := isVRConditionMet(volRep, volrep.ConditionCompleted, metav1.ConditionTrue)
if !conditionMet {
defaultMsg := fmt.Sprintf("VolumeReplication resource for pvc not %s to %s", action, stateString)
v.updatePVCDataReadyConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
v.updatePVCDataProtectedConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", defaultMsg, volRep.GetName(), volRep.GetNamespace()))
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)

v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", errorMsg, volRep.GetName(), volRep.GetNamespace()))

return false
}
Expand Down Expand Up @@ -1579,27 +1585,25 @@ func (v *VRGInstance) validateAdditionalVRStatusForSecondary(pvc *corev1.Persist
return v.checkResyncCompletionAsSecondary(pvc, volRep)
}

conditionMet, _, msg := isVRConditionMet(volRep, volrep.ConditionDegraded, metav1.ConditionTrue)
conditionMet, _, errorMsg := isVRConditionMet(volRep, volrep.ConditionDegraded, metav1.ConditionTrue)
if !conditionMet {
defaultMsg := "VolumeReplication resource for pvc is not in Degraded condition while resyncing"
v.updatePVCDataProtectedConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
if errorMsg == "" {
errorMsg = "VolumeReplication resource for pvc is not in Degraded condition while resyncing"
}

v.updatePVCDataReadyConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)

v.log.Info(fmt.Sprintf("VolumeReplication resource is not in degraded condition while"+
" resyncing is true (%s/%s)", volRep.GetName(), volRep.GetNamespace()))
v.log.Info(fmt.Sprintf("%s (VolRep %s/%s)", errorMsg, volRep.GetName(), volRep.GetNamespace()))

return false
}

msg = "VolumeReplication resource for the pvc is syncing as Secondary"
msg := "VolumeReplication resource for the pvc is syncing as Secondary"
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonReplicating, msg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonReplicating, msg)

v.log.Info(fmt.Sprintf("VolumeReplication resource for the pvc is syncing as Secondary (%s/%s)",
volRep.GetName(), volRep.GetNamespace()))
v.log.Info(fmt.Sprintf("%s (VolRep %s/%s)", msg, volRep.GetName(), volRep.GetNamespace()))

return true
}
Expand All @@ -1608,58 +1612,60 @@ func (v *VRGInstance) validateAdditionalVRStatusForSecondary(pvc *corev1.Persist
func (v *VRGInstance) checkResyncCompletionAsSecondary(pvc *corev1.PersistentVolumeClaim,
volRep *volrep.VolumeReplication,
) bool {
conditionMet, _, msg := isVRConditionMet(volRep, volrep.ConditionResyncing, metav1.ConditionFalse)
conditionMet, _, errorMsg := isVRConditionMet(volRep, volrep.ConditionResyncing, metav1.ConditionFalse)
if !conditionMet {
defaultMsg := "VolumeReplication resource for pvc not syncing as Secondary"
v.updatePVCDataReadyConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
if errorMsg == "" {
errorMsg = "VolumeReplication resource for pvc not syncing as Secondary"
}

v.updatePVCDataProtectedConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)

v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", defaultMsg, volRep.GetName(), volRep.GetNamespace()))
v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", errorMsg, volRep.GetName(), volRep.GetNamespace()))

return false
}

conditionMet, _, msg = isVRConditionMet(volRep, volrep.ConditionDegraded, metav1.ConditionFalse)
conditionMet, _, errorMsg = isVRConditionMet(volRep, volrep.ConditionDegraded, metav1.ConditionFalse)
if !conditionMet {
defaultMsg := "VolumeReplication resource for pvc is not syncing and is degraded as Secondary"
v.updatePVCDataReadyConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
if errorMsg == "" {
errorMsg = "VolumeReplication resource for pvc is not syncing and is degraded as Secondary"
}

v.updatePVCDataProtectedConditionHelper(pvc.Namespace, pvc.Name, VRGConditionReasonError, msg,
defaultMsg)
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonError, errorMsg)

v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", defaultMsg, volRep.GetName(), volRep.GetNamespace()))
v.log.Info(fmt.Sprintf("%s (VolRep: %s/%s)", errorMsg, volRep.GetName(), volRep.GetNamespace()))

return false
}

msg = "VolumeReplication resource for the pvc as Secondary is in sync with Primary"
msg := "VolumeReplication resource for the pvc as Secondary is in sync with Primary"
v.updatePVCDataReadyCondition(pvc.Namespace, pvc.Name, VRGConditionReasonReplicated, msg)
v.updatePVCDataProtectedCondition(pvc.Namespace, pvc.Name, VRGConditionReasonDataProtected, msg)

v.log.Info(fmt.Sprintf("data sync completed as both degraded and resyncing are false for"+
" secondary VolRep (%s/%s)", volRep.GetName(), volRep.GetNamespace()))
v.log.Info(fmt.Sprintf("%s (VolRep %s/%s)", msg, volRep.GetName(), volRep.GetNamespace()))

return true
}

type conditionState string

const (
// Not found.
conditionMissing = conditionState("missing")
conditionStale = conditionState("stale")
// Found but its observed generation does not match the object generation.
conditionStale = conditionState("stale")
// Found, not stale, but its value is "Unknown".
conditionUnknown = conditionState("unknown")
conditionKnown = conditionState("known")
// Found, not stale, and the value is "True" or "False"
conditionKnown = conditionState("known")
)

// isVRConditionMet check if condition is met.
// Returns 3 values:
// - met: true if the condition status matches the desired status, otherwise false
// - state: one of (conditionMissing, conditionStale, conditionUnknown, conditionKnown)
// generation, and its value is not unknown.
// - errorMsg: error message describing why the condition is not met
func isVRConditionMet(volRep *volrep.VolumeReplication,
conditionType string,
Expand Down Expand Up @@ -1697,25 +1703,6 @@ func isVRConditionMet(volRep *volrep.VolumeReplication,
return met, conditionKnown, ""
}

// Disabling unparam linter as currently every invokation of this
// function sends reason as VRGConditionReasonError and the linter
// complains about this function always receiving the same reason.
func (v *VRGInstance) updatePVCDataReadyConditionHelper(
namespace string,
name string,
reason string, //nolint: unparam
message,
defaultMessage string,
) {
if message != "" {
v.updatePVCDataReadyCondition(namespace, name, reason, message)

return
}

v.updatePVCDataReadyCondition(namespace, name, reason, defaultMessage)
}

func (v *VRGInstance) updatePVCDataReadyCondition(pvcNamespace, pvcName, reason, message string) {
protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName)
if protectedPVC == nil {
Expand All @@ -1725,25 +1712,6 @@ func (v *VRGInstance) updatePVCDataReadyCondition(pvcNamespace, pvcName, reason,
setPVCDataReadyCondition(protectedPVC, reason, message, v.instance.Generation)
}

// Disabling unparam linter as currently every invokation of this
// function sends reason as VRGConditionReasonError and the linter
// complains about this function always receiving the same reason.
func (v *VRGInstance) updatePVCDataProtectedConditionHelper(
namespace string,
name string,
reason string, //nolint: unparam
message,
defaultMessage string,
) {
if message != "" {
v.updatePVCDataProtectedCondition(namespace, name, reason, message)

return
}

v.updatePVCDataProtectedCondition(namespace, name, reason, defaultMessage)
}

func (v *VRGInstance) updatePVCDataProtectedCondition(pvcNamespace, pvcName, reason, message string) {
protectedPVC := v.findProtectedPVC(pvcNamespace, pvcName)
if protectedPVC == nil {
Expand Down