Kube Object Protection Enhancements

[raghavendra-talur]

Kubeobjects

Simplify the code for easier maintenance and accessibility to new contributors

• Remove the usage of function pointers
  • vrg: remove the use of captureInProgressStatusUpdate #1530
  • vrg: remove the use of captureStartConditionally #1532
• Refactor to have one function for every state (primary, secondary) and action (relocate, failover). We want to make it trivial to understand which code run for at which state of the system. If we have common code running in many states/actions it can move to helpers.
• Comment unexpected behavior (e.g creating a dummy backup to perform a restore). This may not be needed after we fix the BSL issues, since velero will create the related backups.

Correctness

• Don't delete and recreate the BackupStorageLocation for every backup/restore
  • require deleting related BSLs when deleting a vrg
• Disable call to velero hooks before every capture/restore #1544
  • In progress in kubeobjects: disable hooks #1533
• Disable the backup of PVC through velero #1540
• Size updates to the PVC(due to resize) must lead to a new capture of the PVC. Ensure PVC resize does not require backing up the PV metadata again #74
• Check if the cleanup is performed correctly for volsync objects
• Group of type volume needs to used to compose the pvc label selector for the drpc #1726
• Group of type volume should not be sent to velero for processing #1725
• Velero refers to object's annotation/labels to determine hooks to be run even if hooks aren't specified in the backup request. Disable that behavior. #1727
• Don't backup volsync objects/PVCs
• In case relocate, users should terminate the application gracefully. From ramen's perspective, the last write that was performed by the application before the pvc is deleted or ummounted(rbd vs cephfs). The writes will continue to sync until the pvc is gone. The writes don't stop syncing when the relocate operation is started.
• In case of relocate, kubeobjects shouldn't not be captured after the relocate was issued. Only the latest pvc data should be synced. One way to do that would be to stop capturing as soon as the drpc action field to relocate or just before we advertise WaitOnUserToCleanUp.
• [ ]

Understanding current behavior

• Verify what happens to resources which already exist on the target cluster(overwritten or skipped)
• Debug if includeClusterResources == true works
• Is a backupRef necessary in a recoverWorkflow? If yes, can we make it not required?

Better implementation

• We make 2 backups(0 and 1) and store the capture id we need to recover from in the VRG. Find a way to determine the capture id by without using the VRG.
• When using recipes, volumes section or the group section of type "volume" should be able to specify consistency groups.

Logs

• Log more specific logs with only the relevant information for every backup phase. Currently we log all backup/restore properties for every reconcile
• Don't log huge and unneeded values (e.g. complete CA base64 value). Huge values makes it harder to find the needed details.

Errors

• Include relevant context (e.g. validationErrors when backup or restore fail with phase ValidationFailed)
• Use actual phase names from velero (e.g. ValidationFailed) instead of fake names (backupValidationFailed) that are not searchable (code use "backup" + string(Phase))
• Don't log an error and return it - we want one log with all the context for every error. Minimize the time to find the relevant context when debugging.

Testing

• Add a test in integration tests/e2e that can perform backup and restore without the hub

Names

• We have "backup" and "restore" (velero), "capture" and "recover", and "protect". Use one term for the same thing inside ramen. Since we already use "restore" (e.g. for pvcs) using "backup" and "restore" seems like the best way.

Recipe

• Implement check hooks
• Implement sync hooks
• Implement exec hooks
• Reconcile the recipe CRD to match the latest API used by the other components