Retry backup/restore completion/finalizing status patching to unstuck inprogress backups/restores

[kaovilai]

Signed-off-by: Tiger Kaovilai tkaovila@redhat.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #7207

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.

[codecov]

Codecov Report

Attention: Patch coverage is 3.33333% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 58.72%. Comparing base (33633d8) to head (1587186).
Report is 4 commits behind head on main.

Files	Patch %	Lines
pkg/client/retry.go	0.00%	20 Missing ⚠️
pkg/util/kube/client.go	0.00%	6 Missing ⚠️
pkg/controller/backup_controller.go	0.00%	0 Missing and 1 partial ⚠️
pkg/controller/backup_finalizer_controller.go	0.00%	0 Missing and 1 partial ⚠️
pkg/controller/restore_controller.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7845      +/-   ##
==========================================
- Coverage   58.79%   58.72%   -0.07%     
==========================================
  Files         345      345              
  Lines       28764    28785      +21     
==========================================
- Hits        16911    16905       -6     
- Misses      10425    10451      +26     
- Partials     1428     1429       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kaovilai]

double check my math if this backoff is enough time to solve linked issue.
https://pkg.go.dev/k8s.io/client-go@v0.29.0/util/retry#pkg-variables

var DefaultBackoff = wait.Backoff{
	Steps:    4,
	Duration: 10 * time.Millisecond,
	Factor:   5.0,
	Jitter:   0.1,
}

[kaovilai]

Received feedback to expand retry to other status patches and potentially for backup too. Will ask for feedback in issue.

[Missxiaoguo]

We are working on making each k8s API client call to be retriable in case of internal server errors caused by temporary API outages. The client-go offers the capability to implement a custom middleware transport for adding extra behavior or processing to the default http transport (https://github.com/kubernetes/client-go/blob/master/transport/config.go#L68). We are utilizing it to wrap the default http request with retries for specific error types https://github.com/openshift-kni/lifecycle-agent/pull/548/files#diff-99516c035075962960ff61611a293268a97b3a6535639e92580d0ef8de6eb8cf.
Adding retry logic as middleware enhances the resilience of each API client call. Not sure if your team considers this solution!

[kaovilai]

That's cool. Will check, thanks @Missxiaoguo

[sseago]

I'm thinking that we want to be careful here. We absolutely want retry on the status transitions away from InProgress and towards terminal states, but if we retry for up to 2 minutes for item restore, then a bad APIServer could cause a Restore to hang for hours/days for a big restore, when what we want to see there is mark the error and move on.

[Missxiaoguo]

I'm thinking that we want to be careful here. We absolutely want retry on the status transitions away from InProgress and towards terminal states, but if we retry for up to 2 minutes for item restore, then a bad APIServer could cause a Restore to hang for hours/days for a big restore, when what we want to see there is mark the error and move on.

yeah, my understanding is that the hang depends on the duration of API server outage regardless of small or big restore. The difference with a big restore is more underlying requests based on the number of resources. But it's very reasonable for your guys to be cautious!

[kaovilai]

Trying Requeue + Fail without velero pod restart in another PR and we can discuss if we want this retry PR later.

[kaovilai]

From community meeting, this PR is currently considered too specific and not applicable to most users and for the users that it applies to, backups during apiserver outage is considered risky.

[kaovilai]

Requeue approach at #7863

[kaovilai]

closing as this won't get merged, we will discuss a better solution in the future. Downstream we do favor #7863 as that would "retry" via requeue for a longer time period.