Skip to content
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

Allow SKU capacity 0 #6733

Closed
wants to merge 1 commit into from
Closed

Allow SKU capacity 0 #6733

wants to merge 1 commit into from

Conversation

omgapuppy
Copy link

@omgapuppy omgapuppy commented May 1, 2020

Addresses #6730

Fixes #6730

@ghost ghost added the size/XS label May 1, 2020
@omgapuppy
Copy link
Author

Might be sensible to only pass allowed capacity of 0 if sku_name = "Consumption_0", but this is a quick workaround 🤷

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @omgapuppy, this change seems sensible but do you mind writing a test to confirm that Consumption_0 works?

@katbyte
Copy link
Collaborator

katbyte commented May 3, 2020

and update the docs if needed!

@omgapuppy
Copy link
Author

Didn't get around to it over the weekend, but looks like further changes to the provider are needed:

Error: creating/updating API Management Service "REDACTED" (Resource Group "staging"): apimanagement.ServiceClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="NotSupported" Message="'Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TripleDes168,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Protocols.Ssl30' customProperties are not supported in SkuType Consumption."

So while the 0 value satisfies the API constraint for Consumption tier capacity, the provider passes attributes with the API call that are not supported in that tier - annoying, as I had them left as defaults in given environment (disabled).

Will have a look at adding conditionals specific to Consumption tier in resource_arm_api_management.go later & writing some tests.

I can only commit time to this after work at present, but it is something that'd save us a fair chunk on low traffic endpoints vs using Basic_1.

@evertonmc
Copy link
Contributor

You better add this to the docs as well, because Basic_0 won't work, as well as Consumption_1

@omgapuppy
Copy link
Author

Closing as I don't, unfortunately, have the capacity to commit to fixing this at the moment :(
🤞 #6730 will be picked up by someone closer to the project.

@omgapuppy omgapuppy closed this May 14, 2020
@ghost ghost removed the waiting-response label May 14, 2020
@ghost
Copy link

ghost commented Jun 13, 2020

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!

@ghost ghost locked and limited conversation to collaborators Jun 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TF apply/plan have different constraints for "Consumption" tier for API Management
6 participants