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

Fix PVC deletion with CephFS (volsync) #132

Closed

Conversation

nirs
Copy link

@nirs nirs commented Aug 30, 2023

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 default merge reconcile method
works without extra steps.

Tested on top of release 4.13.

Upstream issue:

(not tracked in bugzilla)

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)
@openshift-ci
Copy link

openshift-ci bot commented Aug 30, 2023

@nirs: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Fix PVC deletion with CephFS (volsync)

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.

@openshift-ci
Copy link

openshift-ci bot commented Aug 30, 2023

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link

openshift-ci bot commented Aug 30, 2023

@nirs: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

Fix PVC deletion with CephFS (volsync)

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.

@nirs
Copy link
Author

nirs commented Sep 12, 2023

Already merged by #134

@nirs nirs closed this Sep 12, 2023
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.

1 participant