Skip to content

recipe: inverseOp addition for exec hooks #2031

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 2 commits into from
May 16, 2025

Conversation

asn1809
Copy link
Member

@asn1809 asn1809 commented May 9, 2025

Adding implementation for inverse operation if provided for the exec hook. The changes include:

  1. Move RecipeElements to util so that it can be re-used without import cycle and update all the references.
  2. If the exec hook fails and inverseOp is specified, get the matching captureSpec or recoverSpec from the recipeElements and execute the hook.
  3. If the inverseOp matching recoverSpec or captureSpec is not found, then return error.
  4. In case captureSpec or recoverSpec is found, create a temp ExecHook and then call the functions which obtains all the relevant pods and then execute the commands.

@asn1809 asn1809 requested a review from raghavendra-talur May 9, 2025 10:02
RestoreFailOn string
}

type PvcSelector struct {
Copy link
Member

Choose a reason for hiding this comment

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

This is a duplicate of this in vrg_pvc_selector.go with another function getPVCSelector(). Need to check if the code can be simplified.

for _, execPod := range execPods {
err := executeCommand(&execPod, e.Hook, e.Scheme, log)
if err != nil && getOpHookOnError(e.Hook) == defaultOnErrorValue {
return fmt.Errorf("error executing exec hook: %w", err)
log.Error(err, "error executing command on pod", "pod", execPod.PodName,
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to account for essential step and workflow on error conditions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just had a little more analysis on this topic and IMO, hook/ops/onError (fail/continue) should be considered instead of workflow.failOn and hook.essential as the combination will be for overall workflow execution. Please correct me if wrong.

@asn1809 asn1809 force-pushed the asn-exec-inverse-op branch 4 times, most recently from 3841dd4 to cd70af8 Compare May 13, 2025 18:46
Adding implementation for inverse operation if provided for the
exec hook. The changes include:
1. Move RecipeElements to util so that it can be re-used without import
   cycle and update all the references.
2. If the exec hook fails and inverseOp is specified, get the
   matching captureSpec or recoverSpec from the recipeElements and
   execute the hook.
3. If the inverseOp matching recoverSpec or captureSpec is not found,
   then return error.
4. In case captureSpec or recoverSpec is found, create a temp ExecHook
   and then call the functions which obtains all the relevant pods and
   then execute the commands.

Modified the implementation to:
1. Send the recipe expanded version embedded in RecipeElements,
2. When executing inverseOp, refer to the recipe and get the hook spec
   needed for hook execution.
3. Execute inverseOp hook.

Signed-off-by: Annaraya Narasagond <annarayanarasagond@gmail.com>
@asn1809 asn1809 force-pushed the asn-exec-inverse-op branch from cd70af8 to c6738ee Compare May 16, 2025 10:31
@asn1809 asn1809 marked this pull request as ready for review May 16, 2025 17:19
@raghavendra-talur raghavendra-talur merged commit 16c2270 into RamenDR:main May 16, 2025
20 of 21 checks passed
@asn1809 asn1809 deleted the asn-exec-inverse-op branch May 17, 2025 09:40
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.

2 participants