-
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
New Resource - azurerm_function_app_flex_consumption
#28199
Conversation
@xiaxyi - thank you for this PR. Only note from me is we don't want customers to worry about operating system, with no plans for a Windows version of Flex Consumption in the foreseeable future. Can we please simplify the name and code not to have Linux in it for Flex Consumption? Other than that I'm happy with separating it out into its own resource type, thank you. |
Patiently awaiting this feature! Thanks |
Ran into same issue, would love to see this merged! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the local location overrides for the tests, can you please address these last few comments.
|
||
* `app_settings` - (Optional) A map of key-value pairs for [App Settings](https://docs.microsoft.com/azure/azure-functions/functions-app-settings) and custom values. | ||
|
||
~> **NOTE:** For storage related settings, please use related properties that are available such as `storage_account_access_key`, terraform will assign the value to keys such as `WEBSITE_CONTENTAZUREFILECONNECTIONSTRING`, `AzureWebJobsStorage` in app_setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these reviews, we've introduced a guide on documentation standards. Please title case all of the notes
~> **NOTE:** For storage related settings, please use related properties that are available such as `storage_account_access_key`, terraform will assign the value to keys such as `WEBSITE_CONTENTAZUREFILECONNECTIONSTRING`, `AzureWebJobsStorage` in app_setting. | |
~> **Note:** For storage related settings, please use related properties that are available such as `storage_account_access_key`, terraform will assign the value to keys such as `WEBSITE_CONTENTAZUREFILECONNECTIONSTRING`, `AzureWebJobsStorage` in app_setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xiaxyi this hasn't been addressed, I'm asking for all instances of NOTE
(uppercase) in the documentation to be recased to Note
(titlecase).
@@ -47,6 +47,7 @@ func (r Registration) Resources() []sdk.Resource { | |||
LinuxFunctionAppSlotResource{}, | |||
LinuxWebAppResource{}, | |||
LinuxWebAppSlotResource{}, | |||
FunctionAppFlexConsumptionResource{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still not sorted alphabetically
@@ -67,6 +67,8 @@ The following arguments are supported: | |||
|
|||
* `site_config` - (Required) A `site_config` block as defined below. | |||
|
|||
* `flex_function_app_deployment` - (Optional) A `flex_consumption_app_deployment` block as defined below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These documentation updates on the linux function app should be removed since these properties haven't been added to the resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Hi @stephybun it appears that the requested changes have been made in the latest commit? |
Is there any help the community can provide to get this moving again? |
Thank you for the follow ups everyone. We are making progress. The testing automation subscription is running into a 'Flex Consumption pricing tier is not enabled for this subscription' message and we are actively working to unblock it so @xiaxyi's testing can resume. |
Hi @stephybun, I'm not sure about why all the test cases failed due to the |
os_type = "Linux" | ||
sku_name = "FC1" | ||
} | ||
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomInteger) | |
`, data.RandomInteger, "eastus2", data.RandomString, data.RandomInteger) // location needs to be hardcoded for the moment because flex isn't available in all regions yet and appservice already has location overrides in TC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephybun : If we are suggesting to hardcode it to eastus2
, does that mean customers can only create the resource in eastus2
region, even if other regions are supported from Azure? If I'm not wrong, there are many regions which support flex consumption plans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcsrijith this hardcodes the region only for our acceptance tests and has no bearing on what regions users will be able to provision this in.
@stephybun it looks like Not a huge deal but would |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests looks good, thanks @xiaxyi LGTM 👍
Thank you @xiaxyi , @nzthiago and @stephybun for MR. Will create the resources as soon as 4.21.0 goes live. |
* Update CHANGELOG.md for #28840 * Update CHANGELOG.md #28808 * Update CHANGELOG.md #27962 * Update CHANGELOG.md for #28859 * Update for #28825 * Update CHANGELOG.md for #28864 * Update CHANGELOG.md #28539 * Update CHANGELOG.md #28685 * Update CHANGELOG.md for #28818 * Update for #28857 #28799 #28856 * Update for #28122 * Update for #28248 #27805 * Update for #28853 * Update for #28316 #28494 #28696 * Update for #28754 * Update CHANGELOG.md #28771 * Update CHANGELOG.md #28842 * Update for #28879 * Update for #28199 * Update CHANGELOG.md #28862 * prep for release v4.21.0 --------- Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Wodans Son <[email protected]> Co-authored-by: stephybun <[email protected]> Co-authored-by: catriona-m <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: Wyatt Fry <[email protected]>
azurerm_linux_function_app_flex_consumption
azurerm_function_app_flex_consumption
this is awesome!! |
Please try using |
Thanks, everyone, for this! Does it include a parameter for always-ready instances? |
Community Note
Description
To make a separate pr for this new feature, referring to the #27531
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource
- support for thething1
property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.