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.

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

feat(controller): selectively run probes using allowlist in event msg #601

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

z0marlin
Copy link
Contributor

@z0marlin z0marlin commented Jun 24, 2021

event messages passed in the UdevEventMessageChannel channel are used
to trigger actions in the controller to add/update/delete blockdevices.
In case of addition, the controller runs all the registered probes to
fill certain details of the blockdevice. While all probes need to be run
when adding a new blockdevice, in case of updates, the list of probes to
be run can be narrowed down based on the type of update. For example,
when the mount points of a partition change, mount probe alone is enough
to fetch the new information.
add an allowlist for probes in the event message to inform the
controller what probes are to be run. the controller then runs only the
probes that are enabled and available in the allowlist. leaving the
allowlist empty causes default behaviour which is to run all the enabled
probes.

Signed-off-by: Aditya Jain aditya.jainadityajain.jain@gmail.com

Any additional information for your reviewer? :
This PR is a continuation of #595 and works towards fixing openebs/openebs#3135.

Checklist:

@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2021

Codecov Report

Merging #601 (65ebcc1) into master (eac1027) will increase coverage by 0.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
+ Coverage   49.48%   49.71%   +0.23%     
==========================================
  Files          73       73              
  Lines        3371     3375       +4     
==========================================
+ Hits         1668     1678      +10     
+ Misses       1551     1541      -10     
- Partials      152      156       +4     
Impacted Files Coverage Δ
cmd/ndm_daemonset/controller/probe.go 100.00% <100.00%> (ø)
cmd/ndm_daemonset/probe/eventhandler.go 50.90% <100.00%> (ø)
cmd/ndm_daemonset/probe/addhandler.go 77.31% <0.00%> (+2.06%) ⬆️

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 eac1027...65ebcc1. Read the comment docs.

event messages passed in the `UdevEventMessageChannel` channel are used
to trigger actions in the controller to add/update/delete blockdevices.
In case of addition, the controller runs all the registered probes to
fill certain details of the blockdevice. While all probes need to be run
when adding a new blockdevice, in case of updates, the list of probes to
be run can be narrowed down based on the type of update. For example,
when the mount points of a partition change, mount probe alone is enough
to fetch the new information.
add an allowlist for probes in the event message to inform the
controller what probes are to be run. the controller then runs only the
probes that are enabled and available in the allowlist. leaving the
allowlist empty causes default behaviour which is to run all the enabled
probes.

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
Copy link
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.

@z0marlin Given some comments and need a few changes

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
Copy link
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.

Changes looks good, shall we merge this after #595

@z0marlin
Copy link
Contributor Author

Changes looks good, shall we merge this after #595

anything works. can be merged before it also. and I'll update the other PR. actually it'll be better if we merge this first. I feel that some of the issues being caused in #595 are due to the fact that we are running all probes when we detect a change which in turn is causing a delay and not letting us get the next event. though the chances of this happening are low, we can try and see if running only a single probe helps.

@akhilerm akhilerm merged commit 0746986 into openebs-archive:master Jun 28, 2021
@z0marlin z0marlin deleted the probe-exec-filters branch October 12, 2021 05:18
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for updating mountpoint and capacity of blockdevices without NDM pod restart
3 participants