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.

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

Improve validation of VR conditions #1643

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nirs
Copy link
Member

@nirs nirs commented Nov 7, 2024

  • Document conditionState constants
  • Always log error message for VR Validated condition
  • Log the error message from the VR condition
  • Clarify use of error message from isVRConditionMet()

Based on #1639 temporarily.

@nirs nirs changed the title WIP: Improve validation of VR conditions Improve validation of VR conditions Nov 7, 2024
@nirs nirs marked this pull request as ready for review November 11, 2024 13:44
@nirs nirs force-pushed the vr-status-cleanups branch 2 times, most recently from 0ac2eb8 to ce74c3a Compare November 11, 2024 18:20
While changing isVRConditionMet() return values, stale line from the
previous comment was left by mistake.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
With this we will log a helpful message when the condition is missing:

    Failed to get the Validated condition from status of VolumeReplication resource

To ensure that we log a helpful message, we don't log the default
message as we do in other conditions. We use a default message only if
we don't get an error message from isVRConditionMet(), which happens
only when using csi-addon <= 0.10.0.

Because we handle an empty message, we don't need to use the helper for
setting a condition with message or default message.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
When VR condition is not met, we were setting the DataReady and
DataProtected protected pvc conditions using either a default error
message, or the error message from the VR condition. However we always
logged the default message, even we if have a better message from the VR
condition (csi-addons > 0.10.0), or from isVRConditionMet() (e.g.
condition is missing, state, or unknown)

Change all the callers to use the default error message as a fallback
and log the same message we report in the protected pvc conditions.

Fix also the log when the condition is met to use the same message we
report in the protected pvc conditions. This simplifies the code and
ensure that the logs matches the status.

With this change the helper for logging either a message or default
message are eliminated.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Use `errorMsg` for the error message returned from isVRConditionMet(), and
`msg` for the default successful message.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants