8000 Implement PVC deselect/delete for PVCs protected by VGR by ShyamsundarR · Pull Request #2095 · RamenDR/ramen · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implement PVC deselect/delete for PVCs protected by VGR #2095

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

Merged
merged 10 commits into from
Jun 18, 2025

Conversation

ShyamsundarR
Copy link
Member
@ShyamsundarR ShyamsundarR commented Jun 15, 2025

Implements pvcUnprotectVolGroupRep which removes protection for a PVC when VRG
is Primary and is protected by VGR in a CG.

The unprotection works as follows:

  • Sets the CG label value to an empty string, ensuring the PVC is deselected
    from the CG that it belongs to
  • Ensures the PVC is no longer reported as part of the VGR status as protected
  • Deletes the VGR if it was protecting only this PVC
  • Removes PV/PVC protection metadata

As the order ensures that the last action for the PVC is to remove its CG
label, the entire workflow is idempotent across multiple reconcile loops.

Further, all errors/progress is reported into the PVC DataReady condition.

An additional change is made to pvcsUnprotectVolGroupRep to set VRG for requeue
only if reconcileMissingVGR returns a requeue. If it reports a missing VGR and
no requeue, then further processing of the VGR is not required, and neither is
a requeue required, as VGR is cleaned up and PV/PVC is also cleaned up as
required.

Also corrects getStorageClass to use a PVC to determine what
its SC is, and corrects createVR to pass in a PVC to getStorageClass
instead of getStorageClass relying on finding the PVC in the list of
PVCs to protect. Thereby allowing common routines to fetch SC and related
classes given a PVC, even when these are not in the selected PVC lists, addressing
cases where a PVC is deselected.

@ShyamsundarR ShyamsundarR requested a review from ELENAGER June 15, 2025 15:29
@ShyamsundarR ShyamsundarR requested a review from BenamarMk as a code owner June 15, 2025 15:29
@ShyamsundarR
Copy link
Member Author

Built on pr #2086


return nil
}

Copy link
Member
@ELENAGER ELENAGER Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this and the next code snippet maybe we could consider using a shared function, something like this:

func shouldSkipPVCAction(spec ramendrv1alpha1.VolRepGroupSpec, log logr.Logger, action string) bool {
	if spec.ReplicationState != ramendrv1alpha1.Primary ||
		spec.PrepareForFinalSync ||
		spec.RunFinalSync {
		log.Info(
			fmt.Sprintf("PVC %s skipped", action),
			"replicationstate", spec.ReplicationState,
			"finalsync", spec.PrepareForFinalSync || spec.RunFinalSync,
		)
		return true
	}
	return false
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked here

commit 1203605 updated DRPC orchestration to ensure that VRG is
Secondary on the cluster before workload is cleaned up, which includes
PVCs.

As a result in VRG orchestration, additional stricter latches are added
to determine when to process PVC deletion or deselection as a separate
operation from workload deletion due to DR actions.

IOW, PVCs that are deleted/deseected when VRG is primary and not in preparing
or executing final sync states, are user deleted PVCs that do not need
further protection.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Some minor corrections to handling PVC deselect/delete for VR use cases

- Remove VolumeUnprotectionEnabledForAsyncVolRep as it is not required any
further, instead if DRPC sets the ramen config option VolumeUnprotectionEnabled
VRG can act on PVC deselect/delete as DRPC orchestration would be stricter

- Cleanup of protected PVCs status is not required at the end of PVC
deselection, as the called PVC deselection functions for VR/VS are expected
to cleanup protected PVCs

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
If VRG update fails due to conflict, we need to get the resource
again before retrying. This was not happening causing intermitent
failures in the related test.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Currently a missing VR, or a requeue return from reconcileMissingVR, in
the flow dealing with PVC deselection or deletion, sets requeue as
true for the VRG. This causes the PVC to not be removed from the protected
PVC status output in pvcUnprotectVolRep.

If reconcileMissingVR reports a missing VR, then processing the PVC
further should stop, and a requeue for the VRG should only be setup
when it returns requeue as true. This is corrected with this commit.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
Implements pvcUnprotectVolGroupRep which removes protection for a PVC when VRG
is Primary and is protected by VGR in a CG.

The unprotection works as follows:
- Sets the CG label value to an empty string, ensuring the PVC is deselected
from the CG that it belongs to
- Ensures the PVC is no longer reported as part of the VGR status as protected
- Deletes the VGR if it was protecting only this PVC
- Removes PV/PVC protection metadata

As the order ensures that the last action for the PVC is to remove its CG
label, the entire workflow is idempotent across multiple reconcile loops.

Further, all errors/progress is reported into the PVC DataReady condition.

An additional change is made to pvcsUnprotectVolGroupRep to set VRG for requeue
only if reconcileMissingVGR returns a requeue. If it reports a missing VGR and
no requeue, then further processing of the VGR is not required, and neither is
a requeue required, as VGR is cleaned up and PV/PVC is also cleaned up as
required.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
If a PVC was deselected, its label value for CG would revert
to an empty string. When reselected, prior to deselection
orchestration completion, the label is not updated with the new
value.

This commit corrects the behaviour for deselected PVCs and retains
not updating the label for deleted PVCs that should not get their
CG label value updated, as removal of the PVC on delete would be
progressing.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
When creating VR or VGR for a PVC, to fetch the SC for the PVC the
PVC is looked up in the list of PVCs that need VR protection.

The functions to determine SC for PVC most often have a PVC that
needs inspection. This also breaks when a PVC is deselected, as it
will not be in the list of PVCs that need DR protection.

Updating determination of SC for a PVC, that may or may not be in
the list of PVCs that need protection, to instead take the PVC itself
as a reference.

The one condition where a PVC needs to be looked up comes in createVR
where the VR name is the same as the PVC. In this case lookup PVCs
to protect and pass the same for SC determination.

Overall, this corrects getStorageClass to use a PVC to determine what
its SC is, and corrects createVR to pass in a PVC to getStorageClass
instead of getStorageClass relying on finding the PVC in the list of
PVCs to protect.

Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
@ShyamsundarR
Copy link
Member Author

Issues not addressed on this PR and in related PRs: #2095, #2096, #2086 are tracked in #2098

@ShyamsundarR
Copy link
Member Author

e2e update:

  • Tested deselect, delete, reselect for PVCs and ensured PV/PVCs were clean on unprotection and VGR was updated with the correct PVC lists for protection

Copy link
Member
@BenamarMk BenamarMk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BenamarMk BenamarMk merged commit 23ffe92 into RamenDR:main Jun 18, 2025
23 checks passed
@ShyamsundarR ShyamsundarR deleted the pvc-deselect-vgr branch June 18, 2025 21:20
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.

3 participants
0