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(controller): add finalizer to claimed BD #416

Merged
merged 1 commit into from
May 6, 2020

Conversation

akhilerm
Copy link
Contributor

@akhilerm akhilerm commented May 5, 2020

Signed-off-by: Akhil Mohan akhil.mohan@mayadata.io

Pull Request template

Why is this PR required? What issue does it fix?:
When a user deletes a BD that is claimed, the BD will be automatically recreated by NDM daemon pod. But the recreated BD will be in Unclaimed state. Thus another claim can get bound to the same BD resulting in data corruption.

What this PR does?:

  • add openebs.io/bd-protection finalizer to BDs that are in Claimed state

Does this PR require any upgrade changes?:
No

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

  • claimed a BD, try to delete the BD, the delete should not happen. the BD should get deleted only after it is Unclaimed. (after cleanup)
  • old BDs already in claimed state should get this finalizer on the first reconciliation

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

Checklist:

  • Fixes #
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

@akhilerm akhilerm added bug bug to existing feature pr/release-note PR should be included in release notes pr/upgrade-automated Upgrade for the changes in this PR is automated labels May 5, 2020
@akhilerm akhilerm added this to Under development in NDM May 5, 2020
@akhilerm akhilerm added this to RC1 - Due: May 5 2020 in 1.10 Release Tracker - Due May 15th. May 5, 2020
@akhilerm akhilerm marked this pull request as ready for review May 5, 2020 18:05
@akhilerm akhilerm requested a review from kmova May 5, 2020 18:05
add openebs.io/bd-protection finalizer to BDs that are in claimed
state to prevent accidental deletion

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@codecov-io
Copy link

Codecov Report

Merging #416 into master will increase coverage by 0.54%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #416      +/-   ##
==========================================
+ Coverage   44.24%   44.78%   +0.54%     
==========================================
  Files          59       59              
  Lines        2839     2869      +30     
==========================================
+ Hits         1256     1285      +29     
- Misses       1479     1482       +3     
+ Partials      104      102       -2     
Impacted Files Coverage Δ
...g/controller/blockdevice/blockdevice_controller.go 15.38% <0.00%> (-2.80%) ⬇️
...er/blockdeviceclaim/blockdeviceclaim_controller.go 41.31% <50.00%> (+0.35%) ⬆️
pkg/features/util.go 100.00% <0.00%> (ø)
pkg/features/features.go 100.00% <0.00%> (ø)
cmd/ndm_daemonset/probe/eventhandler.go 12.56% <0.00%> (+1.57%) ⬆️
cmd/ndm_daemonset/probe/udevprobe.go 63.82% <0.00%> (+2.83%) ⬆️

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 2800e3b...823b93e. Read the comment docs.

@akhilerm akhilerm moved this from Under development to In Review in NDM May 5, 2020
err := r.updateBDStatus(openebsv1alpha1.BlockDeviceUnclaimed, instance)
if err != nil {
reqLogger.Error(err, "marking blockdevice "+instance.Name+" as Unclaimed failed")
}
}
case openebsv1alpha1.BlockDeviceClaimed:
if !util.Contains(instance.GetFinalizers(), controllerutil.BlockDeviceFinalizer) {
// finalizer is not present, may be a BlockDevice claimed from previous release
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bug to existing feature pr/release-note PR should be included in release notes pr/upgrade-automated Upgrade for the changes in this PR is automated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants