terraform-provider-vsphere icon indicating copy to clipboard operation
terraform-provider-vsphere copied to clipboard

feat: Add support for RDM disks

Open sumitAgrawal007 opened this issue 3 years ago • 15 comments

Description

This PR adds code to enable creating VM or adding into VM, RDM disks.

Acceptance tests

  • Have you added an acceptance test for the functionality being added? Yes
  • Have you run the acceptance tests on this branch? Yes

Output from acceptance testing:

run=TestAccResourceVSphereVirtualMachine_RDMDisk -timeout 10m
?   	github.com/hashicorp/terraform-provider-vsphere	[no test files]
=== RUN   TestAccResourceVSphereVirtualMachine_RDMDisk
--- PASS: TestAccResourceVSphereVirtualMachine_RDMDisk (356.00s)
=== RUN   TestAccResourceVSphereVirtualMachine_storageVMotionGlobalSetting
--- PASS: TestAccResourceVSphereVirtualMachine_storageVMotionGlobalSetting (427.26s)
=== RUN   TestAccResourceVSphereVirtualMachine_cloneFromTemplate
--- PASS: TestAccResourceVSphereVirtualMachine_cloneFromTemplate (213.51s)
PASS

Link to Binary to try out this feature.

Release Note

Release note for CHANGELOG:

Capability to Add RDM Disk to a VM.

References

https://github.com/hashicorp/terraform-provider-vsphere/issues/397

sumitAgrawal007 avatar Mar 17 '21 13:03 sumitAgrawal007

Hello @bill-rich @koikonom , Any update on this? Its been pending for a long time and requested by many customers. I have already rebased it twice.

sumitAgrawal007 avatar Jun 07 '21 04:06 sumitAgrawal007

Hi Sumit. Thank you for your contribution, as always. I've added your request to our backlog. We have a few items ahead of this right now, but we'll start looking at it in the near future.

redeux avatar Jun 08 '21 16:06 redeux

Any update on when this might get looked at?

clbx avatar Oct 12 '21 04:10 clbx

I have taken the liberty of rebasing and making necessary changes to this PR. The provided acceptance test seems acceptable, however it's not an environment/feature we have the ability to test at the moment. In general, testing homelab specific/hardware related features is not a scalable activity for us. So after making some final changes to this PR we will be asking the community to manually test the feature for us.

appilon avatar Dec 16 '21 23:12 appilon

After further inspection, one thing I am struggling with on the PR is the awkward polymorphic behavior between the 2 disk types. Some things are nil for one type and not the other (and vice versa if we were to completely expand out an expose all the attributes of RDM).

Are these the only 2 types of disk? (asking @tenthirtyam or any vsphere experts) If not, then it's a recipe for problems if there are eventually many "backings". If there were only ever 2, I could clean this up and live with it.

UPDATE: looking through govmomi indicates there are several possible backings. For this resource to scale something more granular is going to be needed I think.

appilon avatar Dec 17 '21 19:12 appilon

@appilon - agreed. Perhaps we could discuss this enhancement in detail early in New Year with the intended planning and collaboration. We can invite Sumit and also Michael/Doug from our vmware/govmomi team, if needed.

tenthirtyam avatar Dec 17 '21 19:12 tenthirtyam

@tenthirtyam Coming back to this, the best way forward I believe is introducing a new nested block disk_rdm, that way we can keep the attributes required by different disk types separate. The configuration would look like this when configuring disks

 disk {
   label = "disk0"
   size  = 20
 }
 disk_rdm {
   label       = "disk1"
   unit_number = 1
   size        = 2
   disk_mode = "independent_persistent"
   compatibility_mode = "physicalMode"
   rdm_lun_path = "path/to/lun"
 }

Subsequent disk types added later on will also have a separate block definition. We will also rename the existing disk block to disk_vmdk or disk_virtual to create a consistent pattern.

appilon avatar Jan 10 '22 22:01 appilon

I will go ahead and tweak this PR to that implementation, from there, as stated earlier, I'll ask the community to test the new disk type, as we do not have the capacity to test this type of disk.

appilon avatar Jan 10 '22 22:01 appilon

@sumitAgrawal007 @tenthirtyam After digging into this PR further, it's unclear to me how the RDM disk relevant data made its way to the backend (vsphere)? Specifically the compatibility mode, after looking into it further, I don't think this provider can support "physical" mode, logically it will be too difficult since the existing disk resource supports the features of vmdk (cloning specifically). I am not a vmware expert so I'm uncertain on if it's even safe in "virtual" mode (quick glance seems like it is?).

If we can't support physical mode at this time, is this feature still worth pursuing? Or should we wait until we can support physical mode?

appilon avatar Jan 12 '22 23:01 appilon

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Mar 12 '22 17:03 hashicorp-cla

Marking as draft due to merge conflicts that need to be resolved and open concerned regarding implementation.

Ryan Johnson Senior Staff Solutions Architect | Product Engineering @ VMware, Inc.

tenthirtyam avatar Sep 11 '22 22:09 tenthirtyam

I think this is needed... It would be nice to have it in the provider. RDM is quite useful, if not implemented pls. ignore it.

tkriviradev avatar Jul 31 '23 08:07 tkriviradev

Marking this pull request as stale due to inactivity in the past 180 days. This helps us focus on the active pull requests. If this pull request receives no comments in the next 30 days it will automatically be closed. If this pull request was automatically closed and you feel this pull request should be reopened, we encourage creating a new pull request linking back to this one for added context. Thank you!

github-actions[bot] avatar Feb 04 '24 00:02 github-actions[bot]

Any progress on this PR? This is indeed useful!

dijarvrella avatar Mar 19 '24 10:03 dijarvrella

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Apr 19 '24 02:04 github-actions[bot]