microsoft / oe-engine

ACC template generation engine
MIT License
11 stars 14 forks source link

Add support for DC8_v2 #67

Closed achamayou closed 4 years ago

achamayou commented 4 years ago

Adding supporting for the newly available VMs in uksouth, Standard_DC8_v2.

It's worth noting that they can only be provisioned with osDiskType: "StandardSSD_LRS", since they do not support premium storage, and with the "18_04-lts-gen2" image, since "18.04-LTS" is gen1 and those VMs only support gen2.

I've tested this against our subscription.

shruti25ratnam commented 4 years ago

LGTM

paulcallen commented 4 years ago

so you talk about a bunch of limitations with this VM type. Do you address those issues as part of this change, or does the command line options give configurability of this?

yakman2020 commented 4 years ago

Any reason why the machines don't support premium storage? When you say can only support 18.04 you really mean gen2 (for example windows 2019, ubu 16.04)?

I don't see such limitations in the code per se. (and thats good).

paulcallen commented 4 years ago

we specifically need to use gen2 only as these new VMs are limiting to gen2 where the old DC machines could support gen1 also (even though they should not have)

paulcallen commented 4 years ago

also we need to add all the new v2 SKUs. 1, 2, 4 and 8 core I believe. Do we support 6 as well? Not sure about that one

achamayou commented 4 years ago

@yakman2020, I haven’t got any idea why the ACC folks have restricted storage on those SKUs, nor why they support only gen2 images. If you try to use other options, the az CLI gives fairly accurate errors though.

@paulcallen the CLI does not prevent users from picking working combinations of options, but it also lets through some invalid ones (as it did before by the way, it does some validation but you could always easily come up with a combination of settings that could not deploy successfully and would produce an error).

All this change does is it makes the validation permissive enough that you can deploy the new VMs. It could be improved further by validating more precisely and for example checking that DC8 are configured with a gen2 image before generating the template. I’m not sure how to do that though, I would imagine it’s not as easy as checking it ends in “-gen2”.

I have not tried Windows on the new VMs, it’s not a target for our project, nor 16.04, for the same reasons.

paulcallen commented 4 years ago

@achamayou we need gen2 to properly enable sgx. the old VMs are a strange world which are completely non-standard as we had to make everything work before gen2 was fully implemented within azure. I can check on the storage questions though

achamayou commented 4 years ago

@paulcallen that’s fine, this PR neither forces nor enables any particular image, it’s just a comment in passing that new VMs only work with gen2 images. It seems like a reasonable thing to document for users who just want a DC8 and aren’t aware of this subtlety?

I’m happy to update the documentation and samples, and take any other steps you suggest.

paulcallen commented 4 years ago

@achamayou you make a good point here. Not only should we add support for all new hardware skus, we need to update the documentation to give examples of these new skus only as we are deprecating the old ones. We may not remove them just yet, but we need to encourage everyone to use the new _v2 SKUs and gen2 images. If this pull request can get this it would be really helpful. thanks!

achamayou commented 4 years ago

Ok, I will update the doc!

achamayou commented 4 years ago

@paulcallen I believe I have addressed all your feedback, but if there's anything missing please let me know.

achamayou commented 4 years ago

@paulcallen @shruti25ratnam if this ok, can it be merged? I do not have permission to merge PRs.