Iba plugins

[sseago]

Thank you for contributing to Velero!

Please add a summary of your change

Implements ItemBlockAction plugins for internal types:

1. pod
2. PVC
3. ServiceAccount

In addition to returning the related items corresponding to existing BIA plugins (PVC for pod, PV for PVC, etc.), the PVC ItemBlockAction also returns the pods mounting the PVC to handle the case of an RWX volume that's mounted by multiple pods. With this IBA plugin, once the ItemBlock functionality is added, pre- and post- hooks on pods mounting the same PVC will be coordinated.

Does your change fix a particular issue?

Second PR for #7900 (but not the complete feature)

Please indicate you've done the following:

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

[sseago]

PR is currently tagged as a draft PR until #8026 is merged, since this PR branch depends on that one. PR lists 2 commits, but one of these is the commit from #8026, which will go away once that one is merged and this PR is rebased. Please limit review comments here to the "Internal ItemBlockAction plugins" commit -- comments on the other commit should be made on PR #8026.

[codecov]

Codecov Report

Attention: Patch coverage is 69.03553% with 61 lines in your changes missing coverage. Please review.

Project coverage is 58.76%. Comparing base (cc32375) to head (1228b41).
Report is 18 commits behind head on main.

Files	Patch %	Lines
pkg/cmd/server/plugin/plugin.go	0.00%	32 Missing ⚠️
pkg/itemblock/actions/pvc_action.go	72.72%	9 Missing and 3 partials ⚠️
pkg/util/actionhelpers/rbac.go	65.00%	7 Missing ⚠️
pkg/itemblock/actions/service_account_action.go	71.42%	4 Missing and 2 partials ⚠️
pkg/itemblock/actions/pod_action.go	73.33%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8054      +/-   ##
==========================================
- Coverage   58.77%   58.76%   -0.02%     
==========================================
  Files         358      364       +6     
  Lines       30070    30188     +118     
==========================================
+ Hits        17674    17739      +65     
- Misses      10949    10996      +47     
- Partials     1447     1453       +6     

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

[Lyndon-Li]

@sseago
since there are following PRs, what is the consequence after this PR is merged? Will the backup/restore still work?

[sseago]

@Lyndon-Li Yes. This PR adds new plugins, registers the plugins, but the backup workflow logic will never call them. The changes to the BIA plugins here are just refactoring to pull out common logic (in cases where BIA additional items and IBA related items are generated with similar code), but BIA behavior itself will not change. I will do some more testing in my dev cluster before moving this out of draft, though. (also, note that the PR E2E tests pass with these changes in the PR).

[Lyndon-Li]

@sseago
Some thoughts on the code file organization:

1. At present, the plugin-action code files are put in pkg/backup/actions for backup actions and pkg/restore/actions for restore actions. So for pkg/itemblock/actions could we follow the same pattern? e.g., put them into pkg/backup/actions/itemblock?
2. For pkg/actionhelpters, it doesn't looks like a top level package that provide dedicate functionalities, so is it better to move it to internal/actionhelpters or pkg/util/actionhelpers?

[sseago]

@sseago Some thoughts on the code file organization:

1. At present, the plugin-action code files are put in pkg/backup/actions for backup actions and pkg/restore/actions for restore actions. So for pkg/itemblock/actions could we follow the same pattern? e.g., put them into pkg/backup/actions/itemblock?

I had initially considered that. However, I also realized that we may need ItemBlock functionality on restore in the future when we go to implement VolumeGroupSnapshot support, so it probably makes sense to create a v2 ItemBlockAction with an added GetRestoreRelatedItems func, since they will probably share code with the backup func. Keeping it out of backup gives us this extra flexibility.

Also, since pkg/backup/actions/csi is a subdir of backup/actions which also holds BIA plugins, I think it might be confusing to have another subdir of backup/actions which does not contain BIA plugins but instead contains a completely different plugin type.

2. For pkg/actionhelpters, it doesn't looks like a top level package that provide dedicate functionalities, so is it better to move it to internal/actionhelpters or pkg/util/actionhelpers?

That's a good idea. I think pkg/util makes the most sense, but internal would work too

[Lyndon-Li]

1. At present, the plugin-action code files are put in pkg/backup/actions for backup actions and pkg/restore/actions for restore actions. So for pkg/itemblock/actions could we follow the same pattern? e.g., put them into pkg/backup/actions/itemblock?

I had initially considered that. However, I also realized that we may need ItemBlock functionality on restore in the future when we go to implement VolumeGroupSnapshot support, so it probably makes sense to create a v2 ItemBlockAction with an added GetRestoreRelatedItems func, since they will probably share code with the backup func. Keeping it out of backup gives us this extra flexibility.

Yes, I also realize that pkg/itemblock/actions may be reused by restore. But I think we eventually can make the backup specific code into pkg/backup/actions, restore specific code into pkg/restore/actions, and shared code into something like pkg/util. So we probably can move all the code currently in pkg/itemblock/actions to pkg/backup/actions; and when we want to add itemblock for restore, we can refactor the code to split the shared code. Does it work?

[sseago]

1. At present, the plugin-action code files are put in pkg/backup/actions for backup actions and pkg/restore/actions for restore actions. So for pkg/itemblock/actions could we follow the same pattern? e.g., put them into pkg/backup/actions/itemblock?

I had initially considered that. However, I also realized that we may need ItemBlock functionality on restore in the future when we go to implement VolumeGroupSnapshot support, so it probably makes sense to create a v2 ItemBlockAction with an added GetRestoreRelatedItems func, since they will probably share code with the backup func. Keeping it out of backup gives us this extra flexibility.

Yes, I also realize that pkg/itemblock/actions may be reused by restore. But I think we eventually can make the backup specific code into pkg/backup/actions, restore specific code into pkg/restore/actions, and shared code into something like pkg/util. So we probably can move all the code currently in pkg/itemblock/actions to pkg/backup/actions; and when we want to add itemblock for restore, we can refactor the code to split the shared code. Does it work?

The issue is that my current thinking is that if/when we need to expand ItemBlock functionality to restore, the best way to handle this on the plugin side will be to create an ItemBlockActionv2 plugin version which simply adds a new API method GetRestoreRelatedItems rather than going through the overhead of a completely new plugin type. That means that the files under pkg/itemblock/actions themselves will have both backup and restore methods in them, so there's really nothing left to put under pkg/backup/actions. The other issue is that right now pkg/backup/actions (both in the main dir and in subdirs) are all BackupItemActions -- I think it would be more confusing to mix different plugin types under actions. "backup/actions" is where we put BackupItemActions, "restore/actions" is where we put RestoreItemActions, and "itemblock/actions" is where we put ItemBlockActions (which will probably end up being used for both backup and restore).

[sseago]

In addition to the above, throughout the backup code, Velero tends to use the word "actions" to mean "BackupItemActions" (for example the "resolvedActions" field in backup.Request) -- so that will add to the confusion if itemblockaction files are added to the "backup/actions" dir.