Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(controller, api): add label selector to BlockDeviceClaim #397

Merged
merged 5 commits into from
Apr 3, 2020
Merged

feat(controller, api): add label selector to BlockDeviceClaim #397

merged 5 commits into from
Apr 3, 2020

Conversation

akhilerm
Copy link
Contributor

  • add metav1.LabelSelector to BlockDeviceClaim spec
  • updated list method from BDC controller to handle custom label selector.

@akhilerm
feat(api): add label selector to BDC api
33280f2 
add label selector to BDC api.

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm added the pr/hold-review Needs rework. label Mar 29, 2020
@akhilerm akhilerm added this to In progress in NDM Mar 29, 2020
@akhilerm akhilerm added this to Pre-commits and Designs - Due: Mar 31st 2020 in 1.9 Release Tracker - Due Apr 15th. Mar 29, 2020
@akhilerm akhilerm mentioned this pull request Mar 29, 2020
Closed
@akhilerm akhilerm force-pushed the bdc-label-selector branch 2 times, most recently from 0c1f47a to b5dc802 Compare March 31, 2020 06:44
@akhilerm akhilerm changed the title [WIP] feat(controller, api): add label selector to BlockDeviceClaim feat(controller, api): add label selector to BlockDeviceClaim Mar 31, 2020
@akhilerm akhilerm requested a review from kmova March 31, 2020 06:44
@akhilerm akhilerm added pr/release-note PR should be included in release notes and removed pr/hold-review Needs rework. labels Mar 31, 2020
@akhilerm akhilerm moved this from In progress to In Review in NDM Mar 31, 2020
@codecov-io
Copy link

codecov-io commented Mar 31, 2020 •
edited

Codecov Report

Merging #397 into master will decrease coverage by 0.1%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
- Coverage   49.15%   49.05%   -0.11%     
==========================================
  Files          55       55              
  Lines        2429     2436       +7     
==========================================
+ Hits         1194     1195       +1     
- Misses       1140     1143       +3     
- Partials       95       98       +3
Impacted Files Coverage Δ
...er/blockdeviceclaim/blockdeviceclaim_controller.go 38.65% <64.28%> (+0.18%) ⬆️
cmd/ndm_daemonset/probe/udevprobe.go 66.66% <0%> (-1.42%) ⬇️

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 e8f883f...e00b3cb. Read the comment docs.

@kmova kmova requested a review from shovanmaity March 31, 2020 17:29
@akhilerm akhilerm requested a review from kmova April 1, 2020 11:07
@akhilerm akhilerm requested a review from kmova April 1, 2020 18:31
AmitKumarDas
AmitKumarDas reviewed Apr 2, 2020
View reviewed changes
pkg/controller/blockdeviceclaim/blockdeviceclaim_controller_test.go
"custom label and hostname present on bd and selector": {
bdLabels: map[string]string{
ndm.KubernetesHostNameLabel: fakeHostName,
"ndm.io.test": "1234",
Copy link

Choose a reason for hiding this comment

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

It will be good to add tests with values having alphabets & mix of alphabets, numbers & allowed special chars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it okay if we take up in next PR, where we will refactor a bit of the list code and add more test cases. We have a client wrapper here and once a proper selector mechanism is added there, we will move all calls to that client.

@utkarshmani1997 utkarshmani1997 moved this from Pre-commits and Designs - Due: Mar 31st 2020 to RC1 - Due: Apr 5 2020 in 1.9 Release Tracker - Due Apr 15th. Apr 2, 2020
AmitKumarDas
AmitKumarDas previously approved these changes Apr 2, 2020
View reviewed changes
kmova
kmova previously approved these changes Apr 2, 2020
View reviewed changes
@akhilerm
feat(controller): add label selector to list method
181a271 
- add label selector to list method using labels.Selector
- include hostname label in the metav1.LabelSelector from BlockDeviceClaim

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm
fix(unit-test): add unit-test cases for label selector
972027b 
- add test cases for selector in BDC

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm
refact(controller): move selector logic to separate function
ed5a5b0 
move code to create selector from BDC spec to separate function

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm
fix(test): add unit test for new generate label selector method
fde23ae 
add tests for method to generate label selector from BDC spec.

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm dismissed stale reviews from kmova and AmitKumarDas via fde23ae April 2, 2020 18:38
@akhilerm akhilerm requested a review from kmova April 2, 2020 18:39
@kmova kmova merged commit f954106 into openebs-archive:master Apr 3, 2020
NDM automation moved this from In Review to Done Apr 3, 2020
1.9 Release Tracker - Due Apr 15th. automation moved this from RC1 - Due: Apr 5 2020 to Done Apr 3, 2020
akhilerm added a commit to akhilerm/node-disk-manager that referenced this pull request Apr 3, 2020
@akhilerm
feat(controller, api): add label selector to BlockDeviceClaim (openeb…
8949276 
…s-archive#397)

- add label selector to BDC api.
- add label selector to list method using labels.Selector
- include hostname label in the metav1.LabelSelector from BlockDeviceClaim
- add test cases for selector in BDC

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm mentioned this pull request Apr 3, 2020
Merged
akhilerm added a commit to akhilerm/node-disk-manager that referenced this pull request Apr 3, 2020
@akhilerm
feat(controller, api): add label selector to BlockDeviceClaim (openeb…
38fe0ee 
…s-archive#397)

- add label selector to BDC api.
- add label selector to list method using labels.Selector
- include hostname label in the metav1.LabelSelector from BlockDeviceClaim
- add test cases for selector in BDC

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
kmova pushed a commit that referenced this pull request Apr 3, 2020
@akhilerm @kmova
feat(controller, api): add label selector to BlockDeviceClaim (#397)
61a60e0 
- add label selector to BDC api.
- add label selector to list method using labels.Selector
- include hostname label in the metav1.LabelSelector from BlockDeviceClaim
- add test cases for selector in BDC

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm deleted the bdc-label-selector branch April 8, 2020 19:32
@kmova kmova mentioned this pull request Apr 14, 2020
Closed
@Abhinandan-Purkait Abhinandan-Purkait removed this from Done in NDM Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmova kmova kmova approved these changes

@AmitKumarDas AmitKumarDas AmitKumarDas left review comments

@shovanmaity shovanmaity Awaiting requested review from shovanmaity

Assignees
No one assigned
Labels
pr/release-note PR should be included in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@akhilerm @codecov-io @AmitKumarDas @kmova