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(probe): add mount change detection to mount probe #595

Merged
merged 29 commits into from
Jul 8, 2021
Merged

feat(probe): add mount change detection to mount probe #595

merged 29 commits into from
Jul 8, 2021

Conversation

z0marlin
Copy link
Contributor

@z0marlin z0marlin commented Jun 8, 2021 •
edited
Loading

Detect mount point and filesystem changes.
The corresponding design document for this PR can be found in #594

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

Checklist:

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2021 •
edited
Loading

Codecov Report

Merging #595 (e2cf87a) into master (e9ce0d7) will decrease coverage by 0.27%.
The diff coverage is 37.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
- Coverage   48.71%   48.44%   -0.28%     
==========================================
  Files          73       79       +6     
  Lines        3377     3720     +343     
==========================================
+ Hits         1645     1802     +157     
- Misses       1576     1758     +182     
- Partials      156      160       +4
Impacted Files Coverage Δ
cmd/ndm_daemonset/controller/probe.go 100.00% <ø> (ø)
cmd/ndm_daemonset/probe/changehandler.go 0.00% <0.00%> (ø)
cmd/ndm_daemonset/probe/eventhandler.go 38.88% <0.00%> (-1.12%) ⬇️
cmd/ndm_daemonset/probe/udevprobe.go 65.57% <0.00%> (+3.14%) ⬆️
pkg/features/features.go 100.00% <ø> (ø)
pkg/mount/libmount/parse.go 0.00% <0.00%> (ø)
cmd/ndm_daemonset/probe/mountprobe.go 20.21% <7.93%> (-28.28%) ⬇️
pkg/mount/libmount/filesystem.go 26.08% <26.08%> (ø)
pkg/mount/libmount/mount_table.go 35.48% <35.48%> (ø)
pkg/epoll/epoll.go 64.47% <64.47%> (ø)
... and 12 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 e9ce0d7...e2cf87a. Read the comment docs.

sonatype-lift[bot]
sonatype-lift bot reviewed Jun 10, 2021
View reviewed changes
cmd/ndm_daemonset/probe/eventhandler.go Outdated

if isAllBlockDevices(msg.Devices) {
for _, bd := range pe.Controller.BDHierarchy {
err = pe.updateBlockDevice(&bd)
Copy link

Choose a reason for hiding this comment

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

G601: Implicit memory aliasing in for loop.
(at-me in a reply with help or ignore)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ignore

@z0marlin z0marlin force-pushed the mountch branch 2 times, most recently from a4c6a56 to 0583c80 Compare June 10, 2021 11:48
sonatype-lift[bot]
sonatype-lift bot reviewed Jun 10, 2021
View reviewed changes
pkg/epoll/epoll.go

func (e *Epoll) Close() {
e.Stop()
syscall.Close(e.epfd)
Copy link

Choose a reason for hiding this comment

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

G104: Errors unhandled.
(at-me in a reply with help or ignore)

akhilerm
akhilerm reviewed Jun 11, 2021
View reviewed changes
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 Provided some initial comments on the PR

@akhilerm
Copy link
Contributor

@z0marlin Can you make this available via a feature gate, so epoll related code is not executed in normal workflow.

@z0marlin z0marlin marked this pull request as ready for review June 13, 2021 16:23
@z0marlin z0marlin force-pushed the mountch branch 4 times, most recently from de8fc09 to 608d7a0 Compare June 13, 2021 17:19
@akhilerm akhilerm self-requested a review June 14, 2021 05:07
akhilerm
akhilerm reviewed Jun 14, 2021
View reviewed changes
pkg/epoll/epoll.go Outdated Show resolved Hide resolved
akhilerm
akhilerm reviewed Jun 14, 2021
View reviewed changes
pkg/epoll/epoll.go Outdated Show resolved Hide resolved
@z0marlin
remove focus from mount-point change detection integration test
8b1eee6 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
fix(pkg, libmount): add missing nil pointer checks and refactor
1f60f29 
- add nil pointer checks
- update mount file format constant names
- return appropriate errors in `MounTab.AddFilesystem()`
- handle err returned by options in `NewMountTab()`
- remove unused fields in `Filesystem`

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
test(pkg, libmount): add package tests
e995ea6 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(probe): add log messages for change events
2e153dc 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(pkg, libmount): add method to list sources in MountTabDiff
dc7f993 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
fix(probe): add source filters for mount tab and fix updation
4f81416 
- add additional filters to filter out unnecessary changes in mount tab
diff
- don't clear `FileSystem` when updating an unmounted blockdevices

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
test(integration-tests): rename functions in mount-change-tests
2ffd5ba 
rename functions in mount-change-tests to avoid confusion and conflicts
with existing functions in the package

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
docs(pkg, libmount): add inline documentation in package libmount
15cdfaf 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
fix(pkg, libmount): recover from recoverable errros while parsing
90c7a8a 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(pkg, libmount): move libmount under mount
b0c9e0c 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(probe): don't specify log level for logs in changehandler
6532b54 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(pkg, libmount): rename error according to go conventions
141ac18 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
chore: add changelog and update ndm deploy yaml
803dee9 
- add changelog
- update ndm yaml with the new feature gate ("MountChangeDetection")

Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
refactor(probe): log unexpected issue when handling change event
586fa67 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
@z0marlin
chore(probe): remove FIXME
596f40b 
remove FIXME since openebs-archive#602 was merged

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

lgtm-com bot commented Jul 8, 2021

This pull request introduces 1 alert when merging 596f40b into e9ce0d7 - view on LGTM.com

new alerts:

  • 1 for Missing error check

akhilerm
akhilerm previously approved these changes Jul 8, 2021
View reviewed changes
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.

@z0marlin
chore(deploy): update operator manifest
e2cf87a 
Signed-off-by: Aditya Jain <aditya.jainadityajain.jain@gmail.com>
akhilerm
akhilerm approved these changes Jul 8, 2021
View reviewed changes
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

@lgtm-com
Copy link

lgtm-com bot commented Jul 8, 2021

This pull request introduces 1 alert when merging e2cf87a into e9ce0d7 - view on LGTM.com

new alerts:

  • 1 for Missing error check

@prateekpandey14 prateekpandey14 moved this from RC1 - Due: Jul 3 2021 to RC2 - Due: Jul 8 2021 in 2.11 Release Tracker - Due July 15th. Jul 8, 2021
@prateekpandey14 prateekpandey14 moved this from RC2 - Due: Jul 8 2021 to RC1 - Due: Jul 3 2021 in 2.11 Release Tracker - Due July 15th. Jul 8, 2021
@akhilerm akhilerm merged commit ae3f0b6 into openebs-archive:master Jul 8, 2021
2.11 Release Tracker - Due July 15th. automation moved this from RC1 - Due: Jul 3 2021 to Done Jul 8, 2021
@akhilerm akhilerm mentioned this pull request Jul 14, 2021
Merged
@z0marlin z0marlin deleted the mountch branch October 12, 2021 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift[bot] sonatype-lift left review comments

@akhilerm akhilerm akhilerm approved these changes

@RealHarshThakur RealHarshThakur RealHarshThakur left review comments

@kmova kmova Awaiting requested review from kmova

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@z0marlin @codecov-commenter @akhilerm @RealHarshThakur