Remove snapshotVolumes flag necessity for volume policy snapshot action

[shubham-pampattiwar]

Thank you for contributing to Velero!

Please add a summary of your change

Remove snapshotVolumes flag necessity for volume policy snapshot action

Does your change fix a particular issue?

Fixes https://github.com/vmware-tanzu/velero/issues/7782

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.
• [ ] Updated the corresponding documentation in site/content/docs/main.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 58.65%. Comparing base (0d85a64) to head (9f10b0e). Report is 4 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #7786 +/- ## ========================================== - Coverage 58.66% 58.65% -0.01% ========================================== Files 344 344 Lines 28731 28730 -1 ========================================== - Hits 16854 16853 -1 Misses 10448 10448 Partials 1429 1429 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Lyndon-Li]

@shubham-pampattiwar Thanks for the PR, we are making good progress on the understanding of the problem. But I am afraid there are still gaps to fix it:

1. We cannot simply check VolumePolicy != nil when we want to ignore snapshotVolume. VolumePolicy != nil means there are some conditions, however, it doesn't mean these conditions vote for snapshot action. Instead, I think we need to purely call VolumeHelperImpl.ShouldPerformSnapshot in all the places we need to check the snapshot action and inside VolumeHelperImpl.ShouldPerformSnapshot we also consider the snapshotVolume flag --- if we find a met condition voting for snapshot action, we ignore the flag; otherwise, we return snapshotVolume. This is just like what we do in GetVolumesForFSBackup for defaultVolumesToFsBackup
2. Besides backup, we have some places in restore that checks snapshotVolume, i.e., https://github.com/vmware-tanzu/velero/blob/7563a453fb962382c7b1c456ddb2a6645cdd9493/pkg/restore/pv_restorer.go#L56. We need to do some test to decide if we can simply remove it or use more reliable way to check
3. For backup, we have some more places to cover:

https://github.com/vmware-tanzu/velero/blob/7563a453fb962382c7b1c456ddb2a6645cdd9493/pkg/backup/item_backupper.go#L410 https://github.com/vmware-tanzu/velero/blob/7563a453fb962382c7b1c456ddb2a6645cdd9493/pkg/controller/backup_controller.go#L508

4. Looks like this is a gap during the design discussion, as I see the design is aligned with the current code, so we also need to change the design as well.

If you need a live discussion or need any help from us, just let us know. Thanks.

[blackpiglet]

/cc @anshulahuja98 @vmware-tanzu/velero-maintainers

[Lyndon-Li]

@shubham-pampattiwar One more follow up for 1 in the above comment: If no matching condition for snapshot action and snapshotVolume is set to true, we still need to check defaultVolumesToFsBackup flag to decide whether ShouldPerformSnapshot should return true. Am I right?

[shubham-pampattiwar]

closing this PR in favor of https://github.com/vmware-tanzu/velero/pull/7794 Thanks @blackpiglet cc: @Lyndon-Li @reasonerjt