azurerm_windows_web_app - fix health_check_eviction_time_in_min not being set issue

[xiaxyi]

This pr is fixing two issues:

1. The property WEBSITE_HEALTHCHECK_MAXPINGFAILURES in app_setting is suppose to be set if user assign a value to the property health_check_eviction_time_in_min.
2. The WEBSITE_HEALTHCHECK_MAXPINGFAILURES should be written back to the state file if user explicitly specify it in config.

fix issues:#17123
#17546
#16302

[jackofallops]

Hi @xiaxyi - Thanks for this PR. I've left a couple comments below for changes I believe to be needed. But otherwise I think it looks good 👍

Thanks!

[jackofallops]

This should be removed, users should set this value via health_check_eviction_time_in_min and not in the app_settings map. Leaving this in will lead to diffs in that map value. The second returned value from this function should be carried to be set in the site_config.0.health_check_eviction_time_in_min property.

[xiaxyi]

But azure provided such way to set this property and lots of users are aware of this usage as well. shall we just let them set it via app_setting?

[katbyte]

we have moved it out of the map into its own property, therefor it should only be set there as @jackofallops, and if the user does put it into the app settings map we should maybe toss up an error telling the user to set it there?

[jackofallops]

the metadata is not needed here, just the correction of the typo in the WEBSITE_HEALTHCHECK_MAXPINGFAILURES string.

[xiaxyi]

I think we need to move the WEBSITE_HEALTHCHECK_MAXPINGFAILURES from the unmanagedSettings list if user adds the key explicitly in the config?

[xiaxyi]

Thank @jackofallops for the comments, left some feedbacks inline

[katbyte]

also we have some test failureS:

[xiaxyi]

Thanks @jackofallops for the comments. I removed the app_setting KVP code change, also, the update check of app_setting should be removed as we are collecting data from site_config, which means, app_setting still needs to be updated even there is no change in the config.

For the test cases, I removed the property checks as we have the import step.

[katbyte]

we still have test failures

[xiaxyi]

we still have test failures

Thanks @katbyte for the comment, the acc test failure will be fixed in #18177

[pierluca]

Thanks for addressing these issues.
Is there any ETA on when this issue will be fixed/merged/released ?

[jackofallops]

Closing as superseded by 19685

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.