-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix PVC deletion with CephFS (volsync) #132
Conversation
When we create a protected PVC we need to keep the OCM annotations (and maybe others) so they exist in the replicationdestination spec. When restoring a PVC during relocate, we need to include the stored annotations in the new PVC. This is required to keep the PVC owned by OCM. The first step is adding annotation to the type. The change includes also the generated files affected by this change. Signed-off-by: Nir Soffer <[email protected]> (cherry picked from commit 1d60ac9)
The golangci-lint does not allow adding more code in ensurePVCFromSnapshot() due to complexity. Lets move out the boring details of updating PVC labels to a new helper that will be used also for updating annotations. When pvc.Label is nil, do *not* share the map between the PVC and the ProtectedPVC. This is very fragile and inconsistent. Now we either update the existing map or allocating a new map owned by the PVC. Signed-off-by: Nir Soffer <[email protected]> (cherry picked from commit f887141)
Same as labels, copy the annotations from the rddepc protected PVC to the new PVC. Tested by new block in the existing test. Signed-off-by: Nir Soffer <[email protected]> (cherry picked from commit 6463a32)
To ensure that OCM owns the volsync protected PVC when the application is deleted or DR is disabled, copy the OCM annotations from the PVC to the protectedPVC. The annotations are propagated to the target clusters and used to create the PVC during failover and relocate. Tested by adding a new section in existing test. The test ensures that we copy only the OCM annotations and drop other annotations. Fixes: RamenDR#1033 Signed-off-by: Nir Soffer <[email protected]> (cherry picked from commit f19c8ed)
This annotation is added to the application PVC on the source relocate. At the time the annotation is added the protectedPVCs were already propagated to the target clusters, so it is never propagated to the target clusters. However if the annotation will be propagated to the target clusters it can break things, so now we filter it out when copying the PVC annotation to the protected PVC. Signed-off-by: Nir Soffer <[email protected]> (cherry picked from commit 825a0de)
@nirs: No Bugzilla bug is referenced in the title of this pull request. 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/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nirs The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@nirs: No Bugzilla bug is referenced in the title of this pull request. 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/test-infra repository. |
Already merged by #134 |
This change fix ownership of the application PVCs after relocate or failover.
In 4.13 this issue was solved by adding more permissions to ACM, and using the special
mergeAndOwn
reconcile method. With this change using the defaultmerge
reconcile methodworks without extra steps.
Tested on top of release 4.13.
Upstream issue:
(not tracked in bugzilla)