feat: add volume group API endpoint interaction #94
Conversation
|
Everyone contributing to this PR have now signed the CLA. Thanks! |
|
I'm trying to fill out to the CLA form, but not sure what to enter the field 'Please add the Canonical Project Manager or contact'. |
|
Hi @summerwind, Thank you for the contribution. You can put |
Signed-off-by: Moto Ishizawa <[email protected]>
summerwind
force-pushed
the
add-volume-group
branch
from
73c2312 to
a0824da
Compare
|
@skatsaounis Thank you for following up! |
|
Is there anything else I should do? |
|
Hi @summerwind. Thank you for signing the CLA. As you observed already, I re-triggered the actions and now everything looks green. Please give us some time to review your PR and if any change is needed we will come back to you. Thank you for your patience 🙂 |
There was a problem hiding this comment.
Hi @summerwind . I would like to apologize for the big delay in the review and also to thank you once again for your contribution. Overall the PR looks good but I was able to spot 1-2 things that need to change. I am happy to merge when they are addressed 🙂
entity/volume_group.go
Outdated
| SystemID string `json:"system_id,omitempty"` | ||
| HumanUsedSize string `json:"human_used_size,omitempty"` | ||
| Name string `json:"name,omitempty"` | ||
| LogicalVolumes []LogicalVolume `json:"logical_volumes,omitempty"` |
There was a problem hiding this comment.
I'm not sure if it is correct to use the BlockDevice type as is or to add VirtualBlockDevice. However using BlockDevice seems simpler and better at the moment, so I fixed it in 9464fad.
There was a problem hiding this comment.
Would it be more appropriate to define it as VirtualBlockDevice? I will make corrections if necessary.
There was a problem hiding this comment.
That would be ideal, yes. The only reason I proposed the BlockDevice was to not overwhelm you. You could give it a try and I would propose any missing piece afterwards.
There was a problem hiding this comment.
I've added VirtualBlockDevice as an extension to BlockDevice in 374a4df. Could you please confirm that this is what you intended?
|
@skatsaounis Thanks for the review! |
|
Everyone contributing to this PR have now signed the CLA. Thanks! |
|
CLA Check seems to have failed due to certificate expiration.
|
There was a problem hiding this comment.
LGTM +1 Thank you for your contribution and the patience @summerwind
|
Thank you for the review! |
This PR adds the support of following MAAS API endpoint.
DELETE /MAAS/api/2.0/nodes/{system_id}/volume-group/{id}/: Delete volume groupGET /MAAS/api/2.0/nodes/{system_id}/volume-group/{id}/: Read a volume groupPUT /MAAS/api/2.0/nodes/{system_id}/volume-group/{id}/: Update a volume groupPOST /MAAS/api/2.0/nodes/{system_id}/volume-group/{id}/op-create_logical_volume: Create a logical volumePOST /MAAS/api/2.0/nodes/{system_id}/volume-group/{id}/op-delete_logical_volume: Delete a logical volumeGET /MAAS/api/2.0/nodes/{system_id}/volume-groups/: List all volume groupsPOST /MAAS/api/2.0/nodes/{system_id}/volume-groups/: Create a volume groupI think this change would resolve #86.