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

refactor: refactor controller to use dynamic client and use better ratelimiter for controlled exponential backoff #186

Merged
merged 13 commits into from
Jun 8, 2022
Merged

refactor: refactor controller to use dynamic client and use better ratelimiter for controlled exponential backoff #186

merged 13 commits into from
Jun 8, 2022

Conversation

abhilashshetty04
Copy link
Contributor

@abhilashshetty04 abhilashshetty04 commented Apr 22, 2022 •
edited
Loading

Signed-off-by: Abhilash Shetty abhilashshetty@abhilashshetty-mbp1.local

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

What this PR does?:

  1. Replaced codes which implemented code generation and used dynamic client to watch lvmvolume, lvmnode and lvmsnapshot cr
  2. Replaced builder pattern by one func implementation which would create controller struct and return.
  3. Added additional loggers to help debug event triggers.

Snapshot busylooping Fix:
We have changed the ratelimiter which was used in snapshot controller. This ratelimiter requeues failed items after 5 secs for first 12 attempts. Then objects are requeued after 30 secs. I had unloaded dm-snapshot kernel module to reproduce this. Attached logs from the relevant node plugin below.

retrylogs.txt

Issues:

@abhilashshetty04 abhilashshetty04 changed the title [WIP] refactor{vol controller}: refactored controller to use dynamic client rather then code generation [WIP] refactor(vol controller): refactored controller to use dynamic client rather then code generation Apr 22, 2022
@abhilashshetty04 abhilashshetty04 changed the title [WIP] refactor(vol controller): refactored controller to use dynamic client rather then code generation [WIP] refactor: refactor controller to use dynamic client rather then code generation Apr 28, 2022
@abhilashshetty04 abhilashshetty04 mentioned this pull request May 1, 2022
Closed
4 tasks
niladrih
niladrih requested changes May 11, 2022
View reviewed changes
Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

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

Requested some changes

pkg/mgmt/lvmnode/builder.go Outdated Show resolved Hide resolved
pkg/mgmt/lvmnode/builder.go Show resolved Hide resolved
pkg/mgmt/lvmnode/lvmnode.go Outdated Show resolved Hide resolved
pkg/mgmt/lvmnode/lvmnode.go Outdated Show resolved Hide resolved
pkg/mgmt/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/mgmt/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/mgmt/volume/volume.go Outdated Show resolved Hide resolved
@niladrih
Copy link
Member

@abhilashshetty04 -- All commits require a DCO signature. Commit e405d3c is not signed. Also, please add a relevant PR description

Abhilash Shetty and others added 7 commits May 24, 2022 22:23
@abhilashshetty04
Lvm volumes controller refactor:abhilashshetty04/lvm-driver-dev:4.5
16166c1 
Signed-off-by: Abhilash Shetty <abhilashshetty@abhilashshetty-mbp1.local>
@abhilashshetty04
Removed older implementation
5a8cbea
@abhilashshetty04
refactored code for lvmnode and lvmsnapshot controller in node plugin
29aabc2 
Signed-off-by: Abhilash Shetty <abhilashshetty@abhilashshetty-mbp1.local>
@abhilashshetty04
reverting changes on buildx file
8400575 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Fixed :invalid memory address or nil pointer dereference
7252e26 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Made changes to order of the import statements and changed fmt usage …
6819f61 
…to klog

Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Incorporated review comments
6444d2b 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Syntax fix
fa51b8b 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Added descriptions for newly added functions
ad75ec2 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
niladrih
niladrih requested changes May 28, 2022
View reviewed changes
Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

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

Few comments @abhilashshetty04.

pkg/mgmt/lvmnode/builder.go Show resolved Hide resolved
pkg/mgmt/lvmnode/lvmnode.go Outdated Show resolved Hide resolved
pkg/mgmt/lvmnode/lvmnode.go Show resolved Hide resolved
pkg/mgmt/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/mgmt/lvmnode/start.go Outdated Show resolved Hide resolved
pkg/mgmt/snapshot/snapshot.go Outdated Show resolved Hide resolved
pkg/mgmt/volume/volume.go Outdated Show resolved Hide resolved
pkg/mgmt/volume/volume.go Outdated Show resolved Hide resolved
@abhilashshetty04
Changes Incorporating review comments
946fb57 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Incorporating review comments
285c4fd 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@abhilashshetty04
Sorted imports
8e688eb 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
Ab-hishek
Ab-hishek approved these changes Jun 2, 2022
View reviewed changes
Copy link
Member

@Ab-hishek Ab-hishek left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @abhilashshetty04

niladrih
niladrih requested changes Jun 2, 2022
View reviewed changes
Copy link
Member

@niladrih niladrih left a comment

Choose a reason for hiding this comment

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

One little nitpick

pkg/mgmt/lvmnode/lvmnode.go Outdated Show resolved Hide resolved
@abhilashshetty04
Incorporating review comments
e8e9511 
Signed-off-by: Abhilash Shetty <abhilash.shetty@datacore.com>
@pawanpraka1 pawanpraka1 merged commit 433301a into openebs:develop Jun 8, 2022
@abhilashshetty04 abhilashshetty04 changed the title [WIP] refactor: refactor controller to use dynamic client rather then code generation [WIP] refactor: refactor controller to use dynamic client and use better ratelimiter for controller backoff Jun 14, 2022
@abhilashshetty04 abhilashshetty04 changed the title [WIP] refactor: refactor controller to use dynamic client and use better ratelimiter for controller backoff [WIP] refactor: refactor controller to use dynamic client and use better ratelimiter for controlled exponential backoff Jun 14, 2022
@abhilashshetty04 abhilashshetty04 changed the title [WIP] refactor: refactor controller to use dynamic client and use better ratelimiter for controlled exponential backoff refactor: refactor controller to use dynamic client and use better ratelimiter for controlled exponential backoff Jun 14, 2022
@abhilashshetty04 abhilashshetty04 mentioned this pull request Jun 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niladrih niladrih niladrih approved these changes

@Ab-hishek Ab-hishek Ab-hishek approved these changes

@pawanpraka1 pawanpraka1 Awaiting requested review from pawanpraka1

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

Successfully merging this pull request may close these issues.
4 participants
@abhilashshetty04 @niladrih @Ab-hishek @pawanpraka1