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

refactor(pkg, udevevent): decouple udevevent from controller #609

Merged
merged 3 commits into from
Aug 5, 2021

Conversation

z0marlin
Copy link
Contributor

As per conventions, packages in the pkg directory should be independent
of the overlying applications that use them. The udevevent package has
tight coupling with the controller and probe packages from
ndm_daemonset.
Refactor to make udevevent standalone. This includes
moving all application (ndm_daemonset in this case) specific code in
udevevent to appropriate cmd packages and providing an API for the
application code to use the functionalities of udevevent.

This PR aligns with the idea raised in #597

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

Checklist:

As per conventions, packages in the pkg directory should be independent
of the overlying applications that use them. The `udevevent` package has
tight coupling with the `controller` and `probe` packages from
ndm_daemonset.
Refactor to make `udevevent` standalone. This includes
moving all application (ndm_daemonset in this case) specific code in
`udevevent` to appropriate cmd packages and providing an API for the
application code to use the functionalities of `udevevent`.

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@@ -19,10 +19,10 @@ package controller
import (
"sort"

"k8s.io/klog"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reordering as per convention

@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2021

Codecov Report

Merging #609 (8671496) into master (225bbfd) will decrease coverage by 1.72%.
The diff coverage is 17.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
- Coverage   48.11%   46.38%   -1.73%     
==========================================
  Files          79       78       -1     
  Lines        3720     3781      +61     
==========================================
- Hits         1790     1754      -36     
- Misses       1770     1871     +101     
+ Partials      160      156       -4     
Impacted Files Coverage Δ
cmd/ndm_daemonset/controller/probe.go 100.00% <ø> (ø)
cmd/ndm_daemonset/probe/mountprobe.go 20.21% <0.00%> (ø)
pkg/udev/common.go 87.30% <ø> (ø)
cmd/ndm_daemonset/probe/udevprobe.go 48.53% <10.16%> (-15.40%) ⬇️
pkg/udevevent/monitor.go 29.29% <25.39%> (+2.76%) ⬆️
cmd/ndm_daemonset/probe/eventhandler.go 30.55% <0.00%> (-8.34%) ⬇️
cmd/ndm_daemonset/probe/addhandler.go 71.33% <0.00%> (-3.08%) ⬇️

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 225bbfd...8671496. Read the comment docs.

@akhilerm akhilerm added this to Pre-commits and Designs - Due: July 31 2021 in 2.12 Release Tracker - Due Aug 15th. Jul 23, 2021
pkg/udevevent/monitor.go Outdated Show resolved Hide resolved
pkg/udevevent/monitor.go Outdated Show resolved Hide resolved
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.

given some comments

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
 - use action constant strings from pkg udev
 - add inline comments

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.

have a few questions on the new goroutine and channel for passing error messages.

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.

lgtm

@akhilerm akhilerm merged commit 37af3c2 into openebs-archive:master Aug 5, 2021
2.12 Release Tracker - Due Aug 15th. automation moved this from Pre-commits and Designs - Due: July 31 2021 to Done Aug 5, 2021
@z0marlin z0marlin deleted the sizech branch August 9, 2021 13:45
Ab-hishek pushed a commit to Ab-hishek/node-disk-manager that referenced this pull request Aug 13, 2021
…-archive#609)

As per conventions, packages in the pkg directory should be independent
of the overlying applications that use them. The `udevevent` package has
tight coupling with the `controller` and `probe` packages from
ndm_daemonset.

Refactor to make `udevevent` standalone. This includes
moving all application (ndm_daemonset in this case) specific code in
`udevevent` to appropriate cmd packages and providing an API for the
application code to use the functionalities of `udevevent`.

- remove logging from package
- reuse action constants from pkg udev
- add inline comments

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
Ab-hishek pushed a commit to Ab-hishek/node-disk-manager that referenced this pull request Aug 13, 2021
…-archive#609)

As per conventions, packages in the pkg directory should be independent
of the overlying applications that use them. The `udevevent` package has
tight coupling with the `controller` and `probe` packages from
ndm_daemonset.

Refactor to make `udevevent` standalone. This includes
moving all application (ndm_daemonset in this case) specific code in
`udevevent` to appropriate cmd packages and providing an API for the
application code to use the functionalities of `udevevent`.

- remove logging from package
- reuse action constants from pkg udev
- add inline comments

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
akhilerm pushed a commit that referenced this pull request Aug 13, 2021
…614)

As per conventions, packages in the pkg directory should be independent
of the overlying applications that use them. The `udevevent` package has
tight coupling with the `controller` and `probe` packages from
ndm_daemonset.

Refactor to make `udevevent` standalone. This includes
moving all application (ndm_daemonset in this case) specific code in
`udevevent` to appropriate cmd packages and providing an API for the
application code to use the functionalities of `udevevent`.

- remove logging from package
- reuse action constants from pkg udev
- add inline comments

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

Co-authored-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
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.

None yet

4 participants