Skip to content

feat(blockdevicefilter): add filter for empty block device tag label value#500

Merged
kmova merged 4 commits intoopenebs-archive:masterfrom
ajeetrai7:bd_filter
Nov 4, 2020
Merged

feat(blockdevicefilter): add filter for empty block device tag label value#500
kmova merged 4 commits intoopenebs-archive:masterfrom
ajeetrai7:bd_filter

Conversation

@ajeetrai7
Copy link
Copy Markdown
Contributor

@ajeetrai7 ajeetrai7 commented Oct 8, 2020

This PR adds a filter that does not select
BD that has empty block device tag value.
Ref: openebs/openebs#3139

Signed-off-by: ajeetrai707 ajeetrai707@gmail.com

Pull Request template

Why is this PR required? What issue does it fix?:

What this PR does?:

Does this PR require any upgrade changes?:

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

…value

This commit adds a filter that does not select
BD that has empty block device tag value.
Ref: openebs/openebs#3139

Signed-off-by: ajeetrai707 <ajeetrai707@gmail.com>
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #500 into master will decrease coverage by 4.31%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #500      +/-   ##
==========================================
- Coverage   51.47%   47.15%   -4.32%     
==========================================
  Files          73       73              
  Lines        5076     3274    -1802     
==========================================
- Hits         2613     1544    -1069     
+ Misses       2213     1573     -640     
+ Partials      250      157      -93     
Impacted Files Coverage Δ
pkg/select/blockdevice/filters.go 17.69% <33.33%> (+0.42%) ⬆️
pkg/udev/listentry.go 0.00% <0.00%> (-59.26%) ⬇️
api-service/node/services/hugepages.go 0.00% <0.00%> (-58.34%) ⬇️
pkg/sysfs/device.go 0.00% <0.00%> (-52.95%) ⬇️
cmd/ndm_daemonset/probe/customtagprobe.go 28.12% <0.00%> (-51.47%) ⬇️
cmd/ndm_daemonset/probe/sysfsprobe.go 0.00% <0.00%> (-48.47%) ⬇️
...m_daemonset/controller/disk_to_device_convertor.go 0.00% <0.00%> (-46.16%) ⬇️
pkg/smart/diskinfo.go 0.00% <0.00%> (-37.15%) ⬇️
cmd/ndm_daemonset/probe/usedbyprobe.go 15.66% <0.00%> (-36.77%) ⬇️
pkg/smart/satadevice.go 0.00% <0.00%> (-28.89%) ⬇️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e81357...05733c8. Read the comment docs.

@akhilerm akhilerm self-requested a review October 9, 2020 07:59
@akhilerm akhilerm added the Hacktoberfest Issue / PR is part of Hacktoberfest label Oct 9, 2020
Comment thread pkg/select/blockdevice/filters.go Outdated
Comment thread pkg/select/blockdevice/filters.go Outdated
Copy link
Copy Markdown
Contributor

@akhilerm akhilerm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AJEETRAI707 , given a few comments.

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm requested review from kmova and sonasingh46 November 2, 2020 09:25
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
},
},
},
wantedNoofBDs: 4,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is it allowed to pass a BD tag with empty value in BDC.

Shouldn't this be 0? because "" will always be excluded.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc: @akhilerm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is it allowed to pass a BD tag with empty value in BDC.

Yes. it is allowed, because its a valid label selector.

Shouldn't this be 0? because "" will always be excluded.

Actually 2 levels of filtering happens. The original BD List that we get here, will be after a label selection as part of the list call. So those selectors will already be applied. But if those selectors are applied, we still need to remove the empty tag, thats being done in this filter.

So the tests assume that already a selection has happened with the list call. And the output of the tests will be therefore including the BDs that would have been removed by the select call. Thats why the additional 4 BDs come.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check this comment , you can see how the filter works.

@kmova kmova merged commit 63d03dd into openebs-archive:master Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Hacktoberfest Issue / PR is part of Hacktoberfest hacktoberfest-accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants