Bug 2307909: Fix disable dr when VR failed validation #369
Conversation
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com> (cherry picked from commit 1fbfec3)
This brings the new Validated condition needed for fixing disable dr when a VR precondition has failed. With this change we can use the new feature in ramen for testing the fix. Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit f888239)
|
@nirs: This pull request references Bugzilla bug 2307909, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@ELENAGER can you review? |
Done using:
% go get github.com/csi-addons/kubernetes-csi-addons@v0.10.0
go: upgraded github.com/csi-addons/kubernetes-csi-addons v0.9.1 => v0.10.0
go: upgraded github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 => v0.0.0-20240827171923-fa2c70bbbfe5
go: upgraded github.com/onsi/ginkgo/v2 v2.20.0 => v2.20.2
go: upgraded github.com/onsi/gomega v1.34.1 => v1.34.2
go: upgraded golang.org/x/sys v0.23.0 => v0.24.0
go: upgraded k8s.io/api v0.31.0 => v0.31.1
go: upgraded k8s.io/apimachinery v0.31.0 => v0.31.1
% go mod tidy
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
(cherry picked from commit c0e382d)
Was forgetten when we replaced minikube volumesnapshot addons. Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit 75aa24b)
The comment is still unclear, but at least easier to read. Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit c8b3512)
- Replace "VR under deletion" with "deleted VR", matching the terminology used in the code. - Replace "detected as missing or deleted" with "is missing" when we know the VR does not exist. Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit b9552c3)
Moved from validateVRStatus() to make room from checking VR VAlidated status. This also make the function easier to understand, keeping the same level of abstraction and getting rid of the uninteresting details. Conflicts: - internal/controller/vrg_volrep.go - adapt to current code using volRep.Namespace/Name. In upstream we use pvc.Namespace/Name, added in RamenDR#1528 Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit bd41cab)
Rename msg to errorMsg and document that this is an error message, if the we could not get the condition value, because it is missing, stale, or unknown. Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit 830f770)
Log after important changes to the system in delete VR flow to make it easier to understand what the system is doing, and how ramen changed the system. New logs: - delete the VR resource - remove annotations from PV Improve logs: - remove annotations, labels, and finalizers from PVC Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit 7d170b5)
When deleting a primary VRG, we wait until the VR Completed condition is met. However if a VR precondition failed, for example using a drpolicy without flattening enabled when the PVC needs flattening, the VR will never complete and the vrg and drpc deletion will never complete. Since csi-addons 0.10.0 we have a new Validated VR condition, set to true if pre conditions are met, and false if not. VR is can be deleted safely in this state, since mirroring was not enabled. This changes modifies deleted VRG processing to check the new VR Validated status. If the condition exist and the condition status is false, validateVRStatus() return true, signaling that the VR is in the desired state, and ramen completes the delete flow. If the VR does not report the Validated condition (e.g. old csi-addon version) or the condition status is true (mirroring in progress), we continue in the normal flow. The VR will be deleted only when the Completed condition status is true. Tested with discovered deployment and vm using a pvc created from a volume snapshot. Signed-off-by: Nir Soffer <nsoffer@redhat.com> Conflicts: - internal/controller/vrg_volrep.go: adapt current code using volRep.Namespace/Name instead of pvc.Namespace/Name added in RamenDR#1528 Signed-off-by: Nir Soffer <nsoffer@redhat.com> (cherry picked from commit 5dfc74a)
nirs
force-pushed
the
release-4.17-vr-validated
branch
from
f98eb0f to
8689b8e
Compare
|
@nirs: This pull request references Bugzilla bug 2307909, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Unit tests failure is unrelated, but I cannot rerun the tests in this repo. |
|
@nirs: This pull request references Bugzilla bug 2307909, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/cc @ELENAGER |
|
@agarwal-mudit: GitHub didn't allow me to request PR reviews from the following users: ELENAGER. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@ELENAGER: changing LGTM is restricted to collaborators In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: ELENAGER, nirs, ShyamsundarR The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@nirs: This pull request references Bugzilla bug 2307909, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: keesturam. Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Tested also with vm. |
|
Should we merge it manually? |
ShyamsundarR
merged commit e35ac96
into
red-hat-storage:release-4.17
|
@nirs: All pull requests linked via external trackers have merged: Bugzilla bug 2307909 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
When deleting a primary VRG, we wait until the VR Completed condition is
met. However if a VR precondition failed, for example using a drpolicy
without flattening enabled when the PVC needs flattening, the VR will
never complete and the vrg and drpc deletion will never complete.
Since csi-addons 0.10.0 we have a new Validated VR condition, set to
true if pre conditions are met, and false if not. VR is can be deleted
safely in this state, since mirroring was not enabled.
This changes modifies deleted VRG processing to check the new VR
Validated status. If the condition exist and the condition status is
false, validateVRStatus() return true, signaling that the VR is in the
desired state, and ramen completes the delete flow.
If the VR does not report the Validated condition (e.g. old csi-addon
version) or the condition status is true (mirroring in progress), we
continue in the normal flow. The VR will be deleted only when the
Completed condition status is true.
This is a backport of:
Tested with:
Image for testing: quay.io/nirsof/ramen-operator:release-4.17-vr-validated-1