OCPEDGE-1169: fix: device selection only by filter

[jakobmoellerdev]

This changes our device selection methodology to try to unify all selection criteria into the filters.

Previously, we had a complex flow with getNewDevicesToBeAdded and filters together.
This now changes this so that all filter criteria are purely handled through filterDevices.

It achieves this by:

1. Creating a new filter function to verify that a block device must be part of the deviceSelector if the deviceSelector is present, otherwise this is a no-op filter
2. Introducing VerifyMandatoryDevicePaths(devs FilteredBlockDevices, paths[]string) error as a shortcut to determine if a given path is already setup. This can be used together with .Spec.storage.deviceClasses[].deviceSelector.Paths to ensure presence of mandatory paths. For optional paths this check is simply skipped

[openshift-ci-robot]

@jakobmoellerdev: This pull request references OCPEDGE-1169 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.17.0" version, but no target version was set.

In response to this:

This changes our device selection methodology to try to unify all selection criteria into the filters.

Previously, we had a complex flow with getNewDevicesToBeAdded and filters together.
This now changes this so that all filter criteria are purely handled through filterDevices.

It achieves this by:

1. Creating a new filter function to verify that a block device must be part of the deviceSelector if the deviceSelector is present, otherwise this is a no-op filter
2. Introducing CheckAvailableOrAlreadySetupPaths(devs []string) error on FilteredBlockDevices as a shortcut to determine if a given path is already setup

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakobmoellerdev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jakobmoellerdev]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jakobmoellerdev]

/test all

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 82.72727% with 19 lines in your changes missing coverage. Please review.

Project coverage is 58.52%. Comparing base (b71b1c6) to head (4354446).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   57.78%   58.52%   +0.74%     
==========================================
  Files          55       55              
  Lines        4285     4282       -3     
==========================================
+ Hits         2476     2506      +30     
+ Misses       1559     1533      -26     
+ Partials      250      243       -7     

Files	Coverage Δ
internal/controllers/vgmanager/lvm/lvm.go	99.08% <100.00%> (ø)
internal/controllers/vgmanager/wipe_devices.go	72.88% <66.66%> (ø)
internal/controllers/vgmanager/filter/filter.go	82.35% <88.46%> (-0.41%)	⬇️
internal/controllers/vgmanager/controller.go	72.22% <83.33%> (+0.93%)	⬆️
internal/controllers/vgmanager/lsblk/lsblk.go	0.00% <0.00%> (ø)
internal/controllers/vgmanager/devices.go	79.00% <85.41%> (-1.00%)	⬇️

... and 5 files with indirect coverage changes

[jakobmoellerdev]

/test all

[jakobmoellerdev]

/test unit-test

[openshift-ci-robot]

@jakobmoellerdev: This pull request references OCPEDGE-1169 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the spike to target the "4.17.0" version, but no target version was set.

In response to this:

This changes our device selection methodology to try to unify all selection criteria into the filters.

Previously, we had a complex flow with getNewDevicesToBeAdded and filters together.
This now changes this so that all filter criteria are purely handled through filterDevices.

It achieves this by:

1. Creating a new filter function to verify that a block device must be part of the deviceSelector if the deviceSelector is present, otherwise this is a no-op filter
2. Introducing VerifyMandatoryDevicePaths(devs FilteredBlockDevices, paths[]string) error as a shortcut to determine if a given path is already setup. This can be used together with .Spec.storage.deviceClasses[].deviceSelector.Paths to ensure presence of mandatory paths. For optional paths this check is simply skipped

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[suleymanakbas91]

/lgtm

[jakobmoellerdev]

/test e2e-aws
/test e2e-aws-single-node

[openshift-ci]

@jakobmoellerdev: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.