RestoreItemAction v2 API

[sseago]

Describe the problem/challenge you have
the RestoreItemAction plugin interface needs a method to query the plugin as to whether the returned additionalItems are created and ready (since for some controllers, if ObjectReferences in item spec aren't available at create time, creation errors out)

See #1350 for a discussion of the problem hit and https://github.com/vmware-tanzu/velero/blob/main/design/wait-for-additional-items.md for the feature design, in particular note the new method desired in the RestoreItemAction interface.

Describe the solution you'd like
RestoreItemAction v2 interfae needs the following:

	// AreAdditionalItemsReady allows the ItemAction to communicate whether the passed-in
	// slice of AdditionalItems (previously returned by Execute())
	// are ready. Returns true if all items are ready, and false
	// otherwise. The second return value is an error string if an
	// error occurred.
	AreAdditionalItemsReady(restore *api.Restore, AdditionalItems []ResourceIdentifier) (bool, string)

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

• 👍 for "The project would be better with this feature added"
• 👎 for "This feature will not enhance the project in a meaningful way"

[Lyndon-Li]

I am wondering what RestoreController will do after it gets the return value of AreAdditionalItemsReady as false. Possibly, the RestoreController needs to wait for some arbitrary time? If so, this is not so efficient and not generic enough to fit all cases, right?

If so, why don't we have the restore of the additional items work in a check-wait-return manner, because:

• the restore of an additional item has more information of how to do the check
• the restore of an additional item also has more information & ability to do the wait, it may not be an arbitrary wait for some items but return immediately after the item is ready

Of course, the current restore needs to tell the restore of an additional item that - I want to wait your readiness. This could be set by the RestoreItemAction.

As far as my understanding, this problem comes down to a resource dependency problem. In future, we may be able to build an ultimate solution like this (the suggestions above could be reused and take into the ultimate solution):

• build up a graph of the resource dependencies, this dependencies could across objects, namespaces, or whatever boundaries
• do the restore following the dependencies on by one

[sseago]

See the referenced design doc, but the idea is to call the AreAdditionalItemsReady using the same poll-until-timeout logic we use to wait for the restored CRD to be ready before restoring CRs. Unlike the resource restore priority (which applies to the general restore order -- PVCs, Pods, etc.), additional items reference specific item-level dependencies. For the only plugin that I'm aware of that needs this, the items are actually of the same type. For the OpenShift ImageStreamTag plugin, when one ImageStreamTag references another ImageStreamTag, the tag mentioned in the reference must be restored (and ready) before restoring the current one. The additionalItems aspect of the plugin already guarantees that the Create call will be issued in order, but because there's no waiting, it may not yet show up, so we need this extra wait. Most plugins will just return true in the method since waiting isn't required.

Of course, the current restore needs to tell the restore of an additional item that - I want to wait your readiness. This could be set by the RestoreItemAction.

Note that your suggestion here is also already in the approved/reviewed design doc referenced above. I didn't mention that in the description of this issue, although perhaps I should have, since adding an optional field to that return struct doesn't really require API versioning like the new method does -- we have added optional fields to this struct in the past with no effect on backwards compatibility.

[blackpiglet]

Need to also consider Item Action progress #5108 in RIA v2.

[reasonerjt]

Per discussion, we will combine the effort to support progress monitoring in RIA v2, and we'll make sure #5108 is approved and provide the RIA v2 after v1.10

[sseago]

As noted in the comment above, this also includes the API changes needed by the item action progress monitoring design.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sseago]

This isn't stale -- we already have a PR opened to implement this.