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

fix(localpv): ensure lvm volume creation & deletion idempotent #16

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

iyashu
Copy link
Contributor

@iyashu iyashu commented Feb 1, 2021

Pull Request template

Please, go through these steps before you submit a PR.

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

What this PR does?:

Does this PR require any upgrade changes?:
No

If the changes in this PR are manually verified, list down the scenarios covered::

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

Copy link
Contributor

@pawanpraka1 pawanpraka1 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. Changes are fine. Have one minor comment.

pkg/mgmt/volume/volume.go Outdated Show resolved Hide resolved
@iyashu iyashu force-pushed the lvmdel-idempotency branch 2 times, most recently from 9ad5f7e to 4b94e11 Compare February 1, 2021 17:34
@iyashu iyashu force-pushed the lvmdel-idempotency branch from 4b94e11 to a929ba6 Compare February 2, 2021 07:39
@iyashu iyashu changed the title fix(localpv): ensure lvm volume deletion idempotent fix(localpv): ensure lvm volume creation & deletion idempotent Feb 2, 2021
Copy link
Contributor

@pawanpraka1 pawanpraka1 left a comment

Choose a reason for hiding this comment

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

looks good.

@pawanpraka1
Copy link
Contributor

@iyashu can you share you email id. I want to add signoff for this commit.

@iyashu iyashu force-pushed the lvmdel-idempotency branch from a929ba6 to c44fc1e Compare February 2, 2021 12:23
@iyashu
Copy link
Contributor Author

iyashu commented Feb 2, 2021

@iyashu can you share you email id. I want to add signoff for this commit.

I've added the signed off to above commit, let me know if this works now. Thanks

@codecov-io
Copy link

Codecov Report

Merging #16 (c44fc1e) into master (3fe8d61) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #16      +/-   ##
=========================================
- Coverage    1.42%   1.41%   -0.02%     
=========================================
  Files          11      11              
  Lines         702     708       +6     
=========================================
  Hits           10      10              
- Misses        692     698       +6     
Impacted Files Coverage Δ
pkg/driver/agent.go 0.00% <0.00%> (ø)

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 3fe8d61...c44fc1e. Read the comment docs.

@pawanpraka1 pawanpraka1 merged commit b370f3c into openebs:master Feb 2, 2021
@pawanpraka1 pawanpraka1 added this to the v0.2 milestone Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lvm volume deletion is not idempotent leading to leak of lvmvolumes objects
3 participants