provider/vsphere: Add support for memory reservation

[tkak]

This PR adds memory reservation support for vsphere_virtual_machine resource. I'd like to submit again, #4138.

[stack72]

Hi @tkak

this PR looks good - please can you rebase it against master so we can get this merged?

Paul

[chrislovecnm]

@phinze amigo ... might we get a merge ... pretty please??

@stack72 you run the test cases locally???

[stack72]

Hi @chrislovecnm

This PR needs rebased so that we can merge it

I asked @tkak just yesterday

Paul

[chrislovecnm]

@stack72 thanks missed that :)

[jen20]

Hi @tkak and @chrislovecnm - I've just looked over this now - even if the conflicts are resolved during merge there is a problem with the vendored version of the SDK for vSphere that is preventing compilation.

[tkak]

Thank you @stack72, @chrislovecnm and @jen20. I fixed my PR.

[chrislovecnm]

@jen20 much thanks for the help! How are we looking on the merge now?

[stack72]

Do you really want to force a new resource when memory_reservation changes? Can it not be changed once created?

[tkak]

It can change on existing VMs with VMware vSphere. But it needs to be set ForceNew option because there is no support to update resources now.

[stack72]

@tkak (@chrislovecnm) - 2 small nit picks that could potentially be taken care of post-merge. Well 1 nit pick and 1 question I guess

[chrislovecnm]

@stack72 please nit pick away ... We want good code ... lol

[stack72]

Hi @tkak

Thanks for the updates here. I understand now why the Update func hasn't been added to the schema for the memory_reservation

@chrislovecnm it may be worth adding that to the overall tracking for the issues for vsphere provider :)

Thanks all for the work here - this looks good to merge :)

P.

[chrislovecnm]

@stack72 sorry please more info

[stack72]

@chrislovecnm currently, this vm resource only supports Create, Read and Delete. There are no update operations included, therefore if someone changes a value it will not get updated, it will create a new resource

So the large list of enhancements that were being tracked may want to have this included on it - to support the appropriate update operations

Make sense?

[chrislovecnm]

@stack72 now makes sense! We are missing a U...

@tkak you want to pipe in or shall I?

[chrislovecnm]

@stack72 thanks and appreciate the merge

[ghost]

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.