-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AKS: Incorrect vm_size default for version >= v2.14.0 #8127
Comments
hi @pst Thanks for opening this issue. The Since this should be fixed by updating the Terraform Configuration being used here - I'm going to close this issue for the moment - but please let us know if updating the Thanks! cc @JeffreyRichter here's another breaking behavioural change in an existing API |
I understand the schema defines it as required. But it does so for both If the attribute is required, Terraform should error during validate or plan already. It does not, it starts to apply and then fails mid-way. Do you see my point now @tombuildsstuff? |
@pst unfortunately this is a breaking behavioural change in the AKS API - not something specific to Terraform. The difference in Provider behaviour here is likely due to the version of the AKS API being used changing between versions 2.13 and 2.14 of the Azure Provider. Where the AKS API has a history of shipping breaking behavioural changes to existing API versions - we're only able to apply best-effort validation for this resource rather than more granular as we'd like. The API itself doesn't expose this information via an API endpoint - and so from our side we're forced to lean on the AKS API's validation here - rather than validating against a dictionary of all of the valid SKU's for each VM type at plan time (which could change at any time behind the scenes, as is the case in this breaking behavioural change to the Azure API). As such whilst this breaking behavioural change is unfortunate - by the nature of this being an API change there's not much that Terraform could do ahead of time to prevent this. Thanks! |
I've deleted my last comment. I made a mistake, I did provide a value through a But that a breaking change like this between two minor versions is just immediately accepted like this in one of the first class Terraform providers is still disappointing at best. |
@pst this isn't a breaking change in Terraform - this is a breaking change by Microsoft in the Azure/AKS API - I'd suggest filing an issue with Microsoft on the AKS repository - but unfortunately we're at the mercy of the API here. |
This TF provider changed the API version it is using under the hood between the two releases. If the API you migrated to has breaking changes, you have breaking changes too now. This should have been reflected in the semver. Mistakes happen and this is nothing personal, but I strongly disagree with your view on this. If that Frank Turner concert will ever happen I owe you a beer. I'll let this rest now. |
👋 I came here from the linked issue and was a bit concerned AKS would be making behaviorally breaking changes as described. Looking at the commits, @pst seems correct -- changing to 2020-03-01 means you started using system and user pools, which enforced extra validation. I don't see that in the AKS changelog anywhere, although it's doc'd on the relevant site: https://docs.microsoft.com/en-us/azure/aks/use-system-pools#system-and-user-node-pools Could probably call that out as a behavior change in the release notes, though.. |
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Starting with release
v2.14.0
creating an AKS cluster without specifying avm_size
for thedefault_node_pool
defaults to a sku that does not meet the min. requirements of more than 2 cores and 4GB memory.Either the
vm_size
attribute should be mandatory and not have a default. Or the default should work. Weirdly enough, it seems no change has been made to this part of the schema.The text was updated successfully, but these errors were encountered: