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

(feat): add support for custom disk #192

Merged
merged 3 commits into from Jan 29, 2019

Conversation

Projects
None yet
5 participants
@imazik
Copy link
Member

imazik commented Jan 5, 2019

If I create a custom disk using sample yaml ndm will make it's state inactive in every restart or if there is any error in ndm. This pr add one more label in disk cr to detect if the resource will be managed by
ndm or not.

NDM will manage those resources which contains ndm.io/unmanage=false in label.

To create a sable disk using spaese files or partition of a disk You need to modify (update disk/device attributes) and apply this yaml file.

Signed-off-by: Shovan Maity shovan.cse91@gmail.com

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 5, 2019

Codecov Report

Merging #192 into master will increase coverage by 0.17%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
+ Coverage   51.04%   51.21%   +0.17%     
==========================================
  Files          43       43              
  Lines        1720     1722       +2     
==========================================
+ Hits          878      882       +4     
+ Misses        779      777       -2     
  Partials       63       63
Impacted Files Coverage Δ
cmd/controller/controller.go 18.66% <ø> (ø) ⬆️
cmd/controller/disk.go 94.87% <100%> (+0.06%) ⬆️
cmd/controller/diskstore.go 86.41% <100%> (+0.16%) ⬆️
cmd/probe/udevprobe.go 62.38% <0%> (+1.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 c063519...a86d013. Read the comment docs.

@imazik imazik force-pushed the imazik:support-custom-disk branch from 4bc1015 to ad0257e Jan 5, 2019

@imazik imazik changed the title [wip] (feat): add support for custom disk (feat): add support for custom disk Jan 5, 2019

@imazik imazik self-assigned this Jan 6, 2019

@satbirchhikara

This comment has been minimized.

Copy link
Collaborator

satbirchhikara commented Jan 7, 2019

Hi Shovan,

Great work. I have written controllers (device claim,device and disk) using operator-sdk which is little involved change. I folding exisiting code into new controllers. This PR might have lots of conflicts to resolve manually thats why request you to please hold this PR till I converge code to operator-sdk way.

Thanks & Regards,
Satbir Singh

@imazik

This comment has been minimized.

Copy link
Member Author

imazik commented Jan 7, 2019

Hi Shovan,

Great work. I have written controllers (device claim,device and disk) using operator-sdk which is little involved change. I folding exisiting code into new controllers. This PR might have lots of conflicts to resolve manually thats why request you to please hold this PR till I converge code to operator-sdk way.

Thanks & Regards,
Satbir Singh

Love to see your controller handling disk cr's ❤️ .
Thanks.

@satbirchhikara
Copy link
Collaborator

satbirchhikara left a comment

I have one comment(minor), see if you can take it otherwise looks good to me.

Thanks & Regards,
Satbir Singh

@@ -46,6 +50,8 @@ const (
NDMUnknown = "Unknown"
// NDMDiskTypeKey specifies the type of disk.
NDMDiskTypeKey = "ndm.io/disk-type"
// NDMUnmanagedDiskKey specifies disk cr should be managed by ndm or not.
NDMUnmanagedKey = "ndm.io/unmanaged"

This comment has been minimized.

Copy link
@satbirchhikara

satbirchhikara Jan 15, 2019

Collaborator

"ndm.io/managed?" would improve readability(IMO). Need to see '?' allowed in CR or not.

This comment has been minimized.

Copy link
@imazik

imazik Jan 15, 2019

Author Member

Same I was discussed with @kmova.
IMO ndm.io/managed is good.

add support for custom disk
If I create a custom disk using sample yaml ndm will make it's state
inactive in every restart or if there is any error in ndm. This pr add
one more label in disk cr to detect if the resource will be managed by
ndm or not.

NDM will manage those resources which does not contain ndm.io/unmanage=true in label.

To create a sable disk using spaese files or partition of a disk You
need to modify (update disk/device attributes) and apply samples/disk.yaml

Signed-off-by: Shovan Maity <shovan.cse91@gmail.com>

@imazik imazik force-pushed the imazik:support-custom-disk branch from ad0257e to c60509d Jan 22, 2019

change key from unmanaged to managed
Signed-off-by: Shovan Maity <shovan.cse91@gmail.com>

@imazik imazik force-pushed the imazik:support-custom-disk branch from c60509d to 7f1573d Jan 22, 2019

disk.yaml Outdated
@@ -0,0 +1,15 @@
apiVersion: openebs.io/v1alpha1

This comment has been minimized.

Copy link
@kmova

kmova Jan 23, 2019

Member

@shovanmaity - can this be moved to samples/custom-disk.yaml?

This comment has been minimized.

Copy link
@imazik

imazik Jan 24, 2019

Author Member

done

resolve review comments.
Signed-off-by: Shovan Maity <shovan.cse91@gmail.com>

@imazik imazik force-pushed the imazik:support-custom-disk branch from 4883427 to a86d013 Jan 24, 2019

@@ -103,6 +103,7 @@ func (c *Controller) DeleteDisk(name string) {
// and returns list of disk resources.
func (c *Controller) ListDiskResource() (*apis.DiskList, error) {
label := NDMHostKey + "=" + c.HostName
label = label + "," + NDMManagedKey + "!=" + FalseString

This comment has been minimized.

Copy link
@pawanpraka1

pawanpraka1 Jan 24, 2019

Contributor

@Shovan, test the upgrade scenario also. As When we will upgrade NDM, old disk CR won't have "ndm.io/managed" label and we are passing it in label selector.

This comment has been minimized.

Copy link
@imazik

imazik Jan 29, 2019

Author Member

done.

@@ -1,27 +1,29 @@
apiVersion: openebs.io/v1alpha1
kind: Disk
metadata:
name: disk-3d8b1efad969208d6bf5971f2c34dd0c
name: <name of disk> #should be unique like disk-random-unique-name

This comment has been minimized.

Copy link
@pawanpraka1

pawanpraka1 Jan 24, 2019

Contributor

lets not make it a template, as the idea was we can quickly add one sample disk which has some random values. We can have comment explaining each field. So lets have a sample yaml with some default/random values and put comments explaining each field.

This comment has been minimized.

Copy link
@imazik

imazik Jan 29, 2019

Author Member

I added one sample disk.

@kmova

kmova approved these changes Jan 29, 2019

@kmova kmova merged commit 1e11657 into openebs:master Jan 29, 2019

4 checks passed

Better Code Hub ✅ Better Code Hub approves this code
Details
codecov/patch 100% of diff hit (target 51.04%)
Details
codecov/project 51.21% (+0.17%) compared to c063519
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@imazik imazik deleted the imazik:support-custom-disk branch Jan 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.