feat(lvm vol describe): Add feature to describe lvm volumes #74
Conversation
Signed-off-by: Abhinandan-Purkait <abhinandan.purkait@mayadata.io>
Signed-off-by: Abhinandan-Purkait <abhinandan.purkait@mayadata.io>
pkg/util/constant.go
Outdated
| // LVMLocalPV is the cas type name for LocalPV-LVM | ||
| LVMLocalPV = "lvmlocalpv" | ||
| // LVMCasType cas type name | ||
| LVMCasType = "lvmlocalpv" |
There was a problem hiding this comment.
We can change the flag name and avoid the normalize function, as this is an alpha product now, we can make such name changes.
There was a problem hiding this comment.
Changing the flag name surely helps. I did this way because the zfs is called as zfslocalpv thus to make things similar and avoid confusion.
There was a problem hiding this comment.
I'm not sure about the exact naming of it too, I created it as lvmlocalpv to avoid having a hyphen in the name.
pkg/util/util.go
Outdated
| @@ -171,3 +171,14 @@ func ColorStringOnStatus(stringToColor string) string { | |||
| return ColorText(stringToColor, Red) | |||
| } | |||
| } | |||
|
|
|||
| // NormalizeCas is used to maintain one version of cas name | |||
| func NormalizeCas(casType string) string { | |||
There was a problem hiding this comment.
Lets get rid of this function if the flag value strings won't have trouble getting a - in the flag value, i.e. if kubectl openebs get storage --cas-type=localpv-lvm passes the cas-type to programs correctly we can rename the lvmlocalpv to localpv-lvm.
There was a problem hiding this comment.
Yes, we can but to maintain uniformity this has been added. Also normalizing stuffs would give users to give any thing and still get an output at an overhead of this matching and all.
I was thinking of making the normalize recognize the cas-name from any string, viz localpv-lvm, lvmlocalpv, lvm whatever user enters by having some regex matching in here, does that sounds good to? 😅
There was a problem hiding this comment.
It sounds okay-ish, I don't know the exact name too 😓
@shubham14bajpai @prateekpandey14 some naming help please
There was a problem hiding this comment.
Okay, I had added a hypen seperated name in both the repos. So I thing we should keep these in our code as well and get rid of the normalize method. If this sounds good.
There was a problem hiding this comment.
Yes, the localpv-lvm & localpv-zfs names seem better for the flags & internally.
On a nitpick side, things are a bit different in different places, but it'd likely converge to a single set of names sooner or later, I don't like having a normalize function everywhere.
(node-disk-manager) $ git grep StorageEngine
blockdevice/blockdevice.go:413: UsedBy StorageEngine
blockdevice/blockdevice.go:416:// StorageEngine is a typed string for the storage engine
blockdevice/blockdevice.go:417:type StorageEngine string
blockdevice/blockdevice.go:421: CStor StorageEngine = "cstor"
blockdevice/blockdevice.go:424: ZFSLocalPV StorageEngine = "zfs-localpv"
blockdevice/blockdevice.go:427: Mayastor StorageEngine = "mayastor"
blockdevice/blockdevice.go:430: LocalPV StorageEngine = "localpv"
blockdevice/blockdevice.go:433: Jiva StorageEngine = "jiva"Signed-off-by: Abhinandan-Purkait <abhinandan.purkait@mayadata.io>
Signed-off-by: Abhinandan-Purkait <abhinandan.purkait@mayadata.io>
What does this PR do?
LVMvolume.Usage and Output