mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-01-06 13:26:26 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Updated design to take into account protocol buffer limitations. (#3464)

Browse Source 
Signed-off-by: Scott Seago <sseago@redhat.com>
This commit is contained in:
Scott Seago
2021-06-02 16:37:08 -04:00
committed by GitHub
parent 1c84a52a7d
commit 61c12a8171
1 changed files with 98 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

240
design/wait-for-additional-items.md
View File

@@ -50,54 +50,82 @@ we still have a reference to the additional items (`GroupResource` and
namespaced name), as well as a reference to the `RestoreItemAction`
plugin which required it.
At this point, if the `RestoreItemActionExecuteOutput` includes a
non-nil `AdditionalItemsReadyFunc` we need to call a func similar to
`crdAvailable` which we will call `itemsAvailable`
At this point, if the `RestoreItemActionExecuteOutput`
`WaitForAdditionalItems` field is set to `true` we need to call a func
similar to `crdAvailable` which we will call `itemsAvailable`
https://github.com/vmware-tanzu/velero/blob/main/pkg/restore/restore.go#L623
This func should also be defined within restore.go
Instead of the one minute CRD timeout, we'll use a timeout specific to
waiting for additional items. There will be a new field added to
serverConfig, `additionalItemsReadyTimeout`, with a
`defaultAdditionalItemsReadyTimeout` const set to 10 minutes. In addition,
each plugin will be able to define an override for the global
server-level value, which will be added as another optional field in
the `RestoreItemActionExecuteOutput` struct. Instead of the
`IsUnstructuredCRDReady` call, we'll call the returned
`AdditionalItemsReadyFunc` passing in the same `AdditionalItems` slice
as an argument (with items which failed to restore filtered out). If
this func returns an error, then `itemsAvailable` will
propagate the error, and `restoreItem` will handle it the same way it
handles an error return on restoring an additional item. If the
timeout is reached without ready returning true, velero will continue
on to attempt restore of the current item.
`defaultAdditionalItemsReadyTimeout` const set to 10 minutes. In
addition, each plugin will be able to define an override for the
global server-level value, which will be added as another optional
field in the `RestoreItemActionExecuteOutput` struct. Instead of the
`IsUnstructuredCRDReady` call, we'll call `AreAdditionalItemsReady` on
the plugin, passing in the same `AdditionalItems` slice as an argument
(with items which failed to restore filtered out). If this func
returns an error, then `itemsAvailable` will propagate the error, and
`restoreItem` will handle it the same way it handles an error return
on restoring an additional item. If the timeout is reached without
ready returning true, velero will continue on to attempt restore of
the current item.
### `RestoreItemAction` plugin interface changes
In order to implement the `AreAdditionalItemsReady` plugin func, a new
function will be added to the `RestoreItemAction` interface.
```
type RestoreItemAction interface {
// AppliesTo returns information about which resources this action should be invoked for.
// A RestoreItemAction's Execute function will only be invoked on items that match the returned
// selector. A zero-valued ResourceSelector matches all resources.
AppliesTo() (ResourceSelector, error)
// Execute allows the ItemAction to perform arbitrary logic with the item being restored,
// including mutating the item itself prior to restore. The item (unmodified or modified)
// should be returned, along with an optional slice of ResourceIdentifiers specifying additional
// related items that should be restored, a warning (which will be logged but will not prevent
// the item from being restored) or error (which will be logged and will prevent the item
// from being restored) if applicable.
Execute(input *RestoreItemActionExecuteInput) (*RestoreItemActionExecuteOutput, error)
// 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)
}
```
### `RestoreItemActionExecuteOutput` changes
Two new fields will be added to `RestoreItemActionExecuteOutput`, both
optional. `AdditionalItemsReadyTimeout`, if specified, will override
`serverConfig.additionalItemsReadyTimeout`. If the new func field
`AdditionalItemsReadyFunc` is non-nil, then `restoreItem` will call
optional. `AdditionalItemsReadyTimeout`, if non-zero, will override
`serverConfig.additionalItemsReadyTimeout`. If
`WaitForAdditionalItems` is true, then `restoreItem` will call
`itemsAvailable` which will invoke the plugin func
`AdditionalItemsReadyFunc` and wait until the func returns true or the
timeout is reached. If `AdditionalItemsReadyFunc` is nil (the default
`AreAdditionalItemsReady` and wait until the func returns true or the
timeout is reached. If `WaitForAdditionalItems` is false (the default
case), then current velero behavior will be followed. Existing plugins
which do not need to signal to wait for `AdditionalItems` won't need
to change their `Execute()` functions.
In addition, a new func, `WithItemsWait(readyFunc *func)` will
In addition, a new func, `WithItemsWait()` will
be added to `RestoreItemActionExecuteOutput` similar to
`WithoutRestore()` which will set `AdditionalItemsReadyFunc` to
`readyfunc`. This will allow a plugin to include waiting for
`WithoutRestore()` which will set `WaitForAdditionalItems` to
true. This will allow a plugin to include waiting for
AdditionalItems like this:
```
func AreItemsReady (restore *api.Restore, additionalItems []ResourceIdentifier) (bool, error) {
func AreAdditionalItemsReady (restore *api.Restore, additionalItems []ResourceIdentifier) (bool, string) {
...
return true, nil
return true, ""
}
func (p *RestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) {
...
return velero.NewRestoreItemActionExecuteOutput(input.Item).WithItemsWait(AreItemsReady), nil
return velero.NewRestoreItemActionExecuteOutput(input.Item).WithItemsWait(), nil
}
```
@@ -118,68 +146,6 @@ type RestoreItemActionExecuteOutput struct {
// value is true, AdditionalItems will be ignored.
SkipRestore bool
// AdditionalItemsReadyFunc is a func which returns true if
// the additionalItems passed into the func are
// ready/available. A nil value for this func means that
// velero will not wait for the items to be ready before
// attempting to restore the current item.
AdditionalItemsReadyFunc func(restore *api.Restore, []ResourceIdentifier) (bool, error)
// AdditionalItemsReadyTimeout will override serverConfig.additionalItemsReadyTimeout
// if specified. This value specifies how long velero will wait
// for additional items to be ready before moving on.
AdditionalItemsReadyTimeout *time.Duration
}
// WithoutRestore returns SkipRestore for RestoreItemActionExecuteOutput
func (r *RestoreItemActionExecuteOutput) WithItemsWait(
readyFunc func(*api.Restore, []ResourceIdentifier)
) *RestoreItemActionExecuteOutput {
r.AdditionalItemsReadyFunc = readyFunc
return r
}
```
### Earlier iteration (no longer the current implementation plan)
What follows is the first iteration of the design. Everything from
here is superseded by the content above. The options below require
either breaking backwards compatibility or dealing with runtime
casting and optional interfaces. Adding the func pointer to
`RestoreItemActionExecuteOutput` resolves the problem without
requiring either.
#### `RestoreItemActionExecuteOutput` changes
A new boolean field will be added to
`RestoreItemActionExecuteOutput`. If `WaitForAdditionalItems` is true,
then `restoreItem` will call `itemsAvailable` which will invoke the
plugin func `AreAdditionalItemsReady` and wait until the func returns
true or the timeout is reached. If `WaitForAdditionalItems` is false
(the default case), then current velero behavior will be
followed. Existing plugins which do not need to signal to wait for
`AdditionalItems` won't need to change their `Execute()` functions.
In addition, a new func, `WithItemsWait()` will be added to
`RestoreItemActionExecuteOutput` similar to `WithoutRestore()` which
will set the `WaitForAdditionalItems` bool to `true`.
```
// RestoreItemActionExecuteOutput contains the output variables for the ItemAction's Execution function.
type RestoreItemActionExecuteOutput struct {
// UpdatedItem is the item being restored mutated by ItemAction.
UpdatedItem runtime.Unstructured
// AdditionalItems is a list of additional related items that should
// be restored.
AdditionalItems []ResourceIdentifier
// SkipRestore tells velero to stop executing further actions
// on this item, and skip the restore step. When this field's
// value is true, AdditionalItems will be ignored.
SkipRestore bool
// WaitForAdditionalItems determines whether velero will wait
// until AreAdditionalItemsReady returns true before restoring
// this item. If this field's value is true, then after restoring
@@ -187,65 +153,55 @@ type RestoreItemActionExecuteOutput struct {
// until AreAdditionalItemsReady returns true or the timeout is
// reached. Otherwise, AreAdditionalItemsReady is not called.
WaitForAdditionalItems bool
// AdditionalItemsReadyTimeout will override serverConfig.additionalItemsReadyTimeout
// if specified. This value specifies how long velero will wait
// for additional items to be ready before moving on.
AdditionalItemsReadyTimeout time.Duration
}
```
#### `RestoreItemAction` plugin interface changes
In order to implement the `AreAdditionalItemsReady` plugin func, there
are two different approaches we could take.
The first would be to simply add another entry to the
`RestoreItemAction` interface:
```
type RestoreItemAction interface {
// AppliesTo returns information about which resources this action should be invoked for.
// A RestoreItemAction's Execute function will only be invoked on items that match the returned
// selector. A zero-valued ResourceSelector matches all resources.
AppliesTo() (ResourceSelector, error)
// Execute allows the ItemAction to perform arbitrary logic with the item being restored,
// including mutating the item itself prior to restore. The item (unmodified or modified)
// should be returned, along with an optional slice of ResourceIdentifiers specifying additional
// related items that should be restored, a warning (which will be logged but will not prevent
// the item from being restored) or error (which will be logged and will prevent the item
// from being restored) if applicable.
Execute(input *RestoreItemActionExecuteInput) (*RestoreItemActionExecuteOutput, error)
// 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
AreAdditionalItemsReady(restore *api.Restore, AdditionalItems []ResourceIdentifier) (bool, error)
}
```
The downside of this approach is that it is not backwards compatible,
and every `RestoreItemAction` plugin will have to implement the new
func, simply to return `true` in most cases, since the plugin will
either never return `AdditionalItems` from Execute or not have any
special readiness requirements.
The alternative to this would be to define an additional interface for
the optional func, leaving the `RestoreItemAction` interface alone.
```
type RestoreItemActionReadyCheck interface {
// 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
AreAdditionalItemsReady(restore *api.Restore, AdditionalItems []ResourceIdentifier) (bool, error)
// WithItemsWait returns RestoreItemActionExecuteOutput with WaitForAdditionalItems set to true.
func (r *RestoreItemActionExecuteOutput) WithItemsWait()
) *RestoreItemActionExecuteOutput {
r.WaitForAdditionalItems = true
return r
}
```
In this case, existing plugins which do not need this functionality
can remain as-is, while plugins which want to make use of this
functionality will just need to implement the optional func. With the
optional interface approach, `itemsAvailable` will only wait if the
plugin can be type-asserted to the new interface:
## New design iteration (Feb 2021)
In starting the implementation based on the originally approved
design, I've run into an unexpected snag. When adding the wait func
pointer to the `RestoreItemActionExecuteOutput` struct, I had
forgotten about the protocol buffer message format that's used for
passing args to the plugin methods. Funcs are predefined RPC calls
with autogenerated go code, so we can't just pass a regular golang
func pointer in the struct. I've modified the above design to instead
use an explicit `AreAdditionalItemsReady` func. Since this will break
backwards compatibility with current `RestoreItemAction` plugins,
implementation of this feature should wait until Velero plugin
versioning, as described in
https://github.com/vmware-tanzu/velero/issues/3285 is
implemented. With plugin versioning in place, existing (non-versioned
or 1.0-versioned) `RestoreItemAction` plugins which do not define
`AreAdditionalItemsReady` would be able to coexist with a
to-be-implemented `RestoreItemAction` plugin version 2.0 (or 1.1,
etc.) which defines this new interface method. Without plugin
versioning, implementing this feature would break all existing plugins
until they define `AreAdditionalItemsReady`.
Also note that when moving to the new plugin version, the vast
majority of plugins will probably not need to wait for additional
items. All they will need to do to react to this plugin interface
change would be to define the following in the plugin:
```
if actionWithReadyCheck, ok := action.(RestoreItemActionReadyCheck); ok {
// wait for ready/timeout
} else {
return true, nil
}
func AreAdditionalItemsReady (restore *api.Restore, additionalItems []ResourceIdentifier) (bool, string) {
return true, ""
}
```
As long as they never set `WaitForAdditionalItems` to true, this
function won't be called anyway, but if it is called, there will be no
waiting, since it will always return true.