fog / fog-vsphere

Fog for vSphere
MIT License
36 stars 63 forks source link

Add support for NVME Controllers #300

Closed girijaasoni closed 2 months ago

girijaasoni commented 4 months ago

This is the solution for the feature request. NVME (Non Volatile Memory Express) controllers are a class above the currently supported (SCSI Controllers) and has multiple advantages

Currently Foreman and fog-vsphere are designed to only support SCSI controllers and it's types. The goal is to extend it to adding NVME and in future, other types. Do give this a read SCSI, SATA, and NVMe Storage Controller Conditions, Limitations, and Compatibility

Front End PR : https://github.com/theforeman/foreman/pull/10168 Hammer doc PR : https://github.com/theforeman/hammer-cli-foreman/pull/628

More details: https://docs.google.com/document/d/1hs4JekeCQP6brbZGg8hS8kcHohselH1HTEEhThgw-AE/edit#heading=h.lrnpszeygrmo

Ready for testing :D

ShimShtein commented 3 months ago

Not tested, but code-wise looks good

chris1984 commented 3 months ago

Starting to test

chris1984 commented 3 months ago

Starting to test again, @ekohl have all your concerns been addressed? If so will merge and get a release out pending all testing works again with the latest pushes.

chris1984 commented 3 months ago

Looks fine to me @ekohl any other thoughts before I do a final round of testing.

chris1984 commented 3 months ago

@ekohl any other comments or is this good to go, tested it and it works with the latest commit and subhash did as well.

ekohl commented 3 months ago

@chris1984 I still see open conversations when I look on the changed files tab. I'd expect those to either be fixed or closed with an explanation why it won't be fixed.

girijaasoni commented 2 months ago

@chris1984 I still see open conversations when I look on the changed files tab. I'd expect those to either be fixed or closed with an explanation why it won't be fixed.

Thanks @ekohl , updated them.

chris1984 commented 2 months ago

Looks fine now, just one concern:

I tested this again and I am getting Failed to create a compute VMware8 (VMware) instance ron-doria.satellite.lab.eng.rdu2.redhat.com: InvalidController: The device '1' is referring to a nonexisting controller '1,000'.

Since it keeps happening between my env and qe's I wonder if customers will start to hit this too. Is there a way we can get that controller id for all envs since you make a fix and it works on mine, but errors's on qe's vmware then you fix and it works on theirs but not mine. I am worried customer will hit that same error