Add WaitForReady flag to check container readiness state before exec a hook

[Ripolin]

Thank you for contributing to Velero!

Before running a post restore hook, Velero test the targeted container to check if it's running or not. To me it should be better to check if targeted container is ready to run restore commands.

Please indicate you've done the following:

• Accepted the DCO. Commits without the DCO will delay acceptance.
• Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
• Updated the corresponding documentation in site/content/docs/main.

[kaovilai]

Need changelog file

Created a changelog file or added /kind changelog-not-required as a comment on this pull request.

[kaovilai]

I think it is possible for someone to have a workload such that it isn't ready until the hook runs to do something such as adding data.

Perhaps a new kind of hook could be reasonable.. prehookready? thoughts?

[kaovilai]

ie. this change can break an existing workload that is only ready by manually running a script when prehook is run on restore.

[sseago]

@kaovilai @Ripolin or perhaps instead of a new hook type, a new bool field added to ExecRestoreHook -- something like WaitForReady and then the logic could wait for running vs. ready depending on this field.

https://github.com/vmware-tanzu/velero/blob/main/pkg/apis/velero/v1/restore_types.go#L194

[shubham-pampattiwar]

Having something like WaitForReady in ExecRestoreHook API would make sense I believe.

[Ripolin]

Having something like WaitForReady in ExecRestoreHook API would make sense I believe.

Like that 1e211ae ?

[sseago]

@Ripolin Yes, I was thinking something like that. A couple things, though:

1. We'd need to update the restore hooks documentation to show this new field: https://github.com/vmware-tanzu/velero/blob/main/site/content/docs/main/restore-hooks.md#exec-restore-hooks
2. Also, note that restore hooks can be specified using annotations, so we'd probably want to handle this field there as well.

[sseago]

Since there can be multiple restore hooks, it looks like setting WaitForReady to true for only one hook has same effect as setting it for all -- since velero will wait until the container is ready for all hooks to run. I think this is probably fine as long as it's documented -- @shubham-pampattiwar what do you think?

[Ripolin]

@Ripolin Yes, I was thinking something like that. A couple things, though:

1. We'd need to update the restore hooks documentation to show this new field: https://github.com/vmware-tanzu/velero/blob/main/site/content/docs/main/restore-hooks.md#exec-restore-hooks
2. Also, note that restore hooks can be specified using annotations, so we'd probably want to handle this field there as well.

Done both

[shubham-pampattiwar]

@sseago yes I agree, the new behavior needs to be documented.

[codecov]

Codecov Report

Merging #6918 (e5af7f5) into main (ad114f8) will increase coverage by 0.01%.
The diff coverage is 89.65%.

@@            Coverage Diff             @@
##             main    #6918      +/-   ##
==========================================
+ Coverage   60.78%   60.80%   +0.01%     
==========================================
  Files         250      250              
  Lines       26629    26649      +20     
==========================================
+ Hits        16187    16204      +17     
- Misses       9293     9295       +2     
- Partials     1149     1150       +1     

Files	Coverage Δ
internal/hook/wait_exec_hook_handler.go	73.09% <100.00%> (+1.32%)	⬆️
internal/hook/item_hook_handler.go	86.94% <83.33%> (-0.45%)	⬇️

[blackpiglet]

@Ripolin
Could you help to squash the commits?

[Ripolin]

@Ripolin Could you help to squash the commits?

Squash and rebase done !