Add longhorn v2 provisioner #82
Conversation
- we add the provisioner to provision different type storage backend Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
|
Hi @tserong, Thanks for the quick review. So this change will still draft here until v1.3.0 release. |
| @@ -212,12 +213,16 @@ func (c *Controller) OnBlockDeviceChange(_ string, device *diskv1.BlockDevice) ( | |||
| if devPath == "" { | |||
| return nil, fmt.Errorf("failed to resolve persistent dev path for block device %s", device.Name) | |||
| } | |||
| if device.Spec.Provisioner == "" { | |||
There was a problem hiding this comment.
nit: Do we have a mutator mechanism to add the default value? If yes, we can prevent this sort of non-biz code in controller.
| Tags: device.Spec.Tags, | ||
|
|
||
| diskSpec := generateDiskSpec(device.Spec.Provisioner) | ||
| if device.Spec.Provisioner == defaultProvisioner { |
There was a problem hiding this comment.
nit: suggest handling each case explicitly to prevent using else for v2. Can use switch.
| @@ -175,3 +176,14 @@ func CallerWithCondLock[T any](cond *sync.Cond, f func() T) T { | |||
| defer cond.L.Unlock() | |||
| return f() | |||
| } | |||
|
|
|||
| // DoCommand executes a command and returns the stdout, stderr and error | |||
| func DoCommand(cmdString string) (string, string, error) { | |||
There was a problem hiding this comment.
Nit: This function is quite general. Not sure if there is an existing lib able to use or even we can add this to common lib for other repos to reuse later if appropriate.
Can extend it to pass io.Writer out and err instead of hard-coded bytes buffer here only.
If for testing only, why not just add this to a related test package folder?
|
we will have a new v2 engine provisioner later. |
Problem:
Add capability to provision Longhorn v2 engine backend disk
Solution:
Add capability to provision Longhorn v2 engine backend disk
Related Issue:
harvester/harvester#5274
Test plan:
TBD
Tasks: