Improve maintainability by merging Standard.json & LowPri.json

AmineI commented 4 years ago

Adds Spot VM as a param in Standard.json. This gets rid of the need to use a different template for LowPri,

I was wondering why LowPri was using VM Scale Sets, and I think it was an older limitation of LowPri before it became Spot. If there are advantages I don't know of, I'd be glad to be enlightened :).

However, now Azure Spot VM can be deployed without Scale Sets, so merging both templates would really help with maintainability down the road, and eliminate the need to test the Spot deployments in the future as they would have no difference in configuration.

[x] Standard with the Spot VM parameter disabled works properly.
[x] Standard with the Spot VM parameter enabled works properly. Thanks to @andiradulescu for his help testing this.
[x] Readme updates

Deployment link for this branch

Fixes #36 (the second issue mentioned in it, due to changing the evictionpolicy to deallocate) Fixes #26 Fixes #21 (LowPri now have the same RDP config of Standard, as a result of the merge)

andiradulescu commented 4 years ago

@AmineI - testing this now, with Standard_NV6.

Running the template with Standard_NV6_Promo gives this error:

{
    "error": {
        "code": "OperationNotAllowed",
        "message": "The requested VM Size Standard_NV6_Promo does not support creation of Low Priority instances."
    }
}

AmineI commented 4 years ago

Yes, Promo is not supported in LowPri indeed. You fixed that in #39 but that was when there were 2 deployment files.. I'll see if I can hide Promo when Spot is selected

andiradulescu commented 4 years ago

With Standard_NV6 everything seems to work fine! Steam and the NVIDIA drivers are installed.

Yes, indeed, not sure if you can hide the Promo one when there is only one template.

AmineI commented 4 years ago

With Standard_NV6 everything seems to work fine! Steam and the NVIDIA drivers are installed.

Great, thanks ! Steam and Nvidia drivers matter less here as I'm merging against master, which does not have the changes from #41 - only the VM deployment matters for this PR.

Yes, indeed, not sure if you can hide the Promo one when there is only one template. Yup. The error message seems explicit enough so it should be fine this way, with a small note in the readme.

I made a last set of minor updates to the "Spot Disabled" behavior, and reconfirmed the non-spot deployment. The template should be ready to go, and I'll tackle the readme update once ecalder6 validates the template changes.

ecalder6 / azure-gaming

Improve maintainability by merging Standard.json & LowPri.json #42