adding name of resource that were not restored on applicationrestore.go #1389
Conversation
|
Can one of the admins verify this patch? |
| if resource.Status != storkapi.ApplicationRestoreStatusSuccessful { | ||
| restore.Status.Status = storkapi.ApplicationRestoreStatusPartialSuccess | ||
| restore.Status.Reason = "Volumes were restored successfully. Some existing resources were not replaced" | ||
| restore.Status.Reason = fmt.Sprintf("Volumes were restored successfully. Some existing resources were not replaced, resource: %s", resource.Name) |
There was a problem hiding this comment.
This is the overall status of the restore. So we are avoiding giving details of the specific resource. We do have resource level status, where the retained status is maintained. If we add the resource name here, it will confusing that only because of this resource alone, it is partial success.
There was a problem hiding this comment.
@siva-portworx I was thinking on that, it would require to change our Struct, changing the field restore.Status.reason to a slice of Strings, filling that list with the non-restored objects (with a previous check, if all of them failed, just inform that) and non stopping the for loop at the first occurrence as we currently do. All that sounds like lot of effort. I see, probably a message of where the user can find more details would be useful.
3 participants
What type of PR is this?
improvement
What this PR does / why we need it:
On pkg/applicationmanager/controllers/applicationrestore.go line 1608 we need to check which resource were not successfully restored.
Does this PR change a user-facing CRD or CLI?:
no
restore.Status.Reason = fmt.Sprintf("Volumes were restored successfully. Some existing resources were not replaced, resource: %s", resource.Name)Does this change need to be cherry-picked to a release branch?:
no