search

Azure / avdaccelerator

AVD Accelerator deployment automation to simplify the setup of AVD (Azure Virtual Desktop) based on best practices
MIT License
330 stars 209 forks source link

update WAF compliant improvement #600

Closed yahanda closed 6 months ago

yahanda commented 6 months ago

Overview/Summary

Update default option and pre-requisites doc to be completely WAF compliant as per ADO Items below: User Story 32716: Default AVD LZA deployment to be completely WAF compliant - Boards (azure.com) Spike 34047: Is LZA deployment resilient by default? - Boards (azure.com)

This PR fixes/adds/changes/removes

AVD LZA Baseline deployment Changed the following Gaps/Scope of Improvement:

  1. The option to select "Zone redundant storage" for storage account is disabled by default. Zone redundant storage is supported for both Standard and premium storage.
  2. The option to deploy monitoring is not configured by default , we can make it to configure by default as configuring monitoring is an important aspects of resiliency.
  3. In the Pre-requisite document we should mention that the DC VMs if hosted in Azure should follow High Availability best practices as mentioned in https://learn.microsoft.com/en-us/azure/architecture/example-scenario/identity/adds-extend-domain#reliability and High availability for Entra Domain services can be setup using replica set as mentioned in https://learn.microsoft.com/en-us/entra/identity/domain-services/concepts-replica-sets.
  4. If customer selects "Compute gallery" as the image source then it is customer's responsibility to ensure the high availability of the image used as mentioned in https://learn.microsoft.com/en-us/azure/virtual-machines/azure-compute-gallery#high-availability. This can be mentioned in the pre-requisites document.

AVD LZA Custom Image Build Changed the following Gaps/Scope of Improvement:

  1. The Storage account type for image version storage is by default selected as Locally redundant. We can change it to Zone redundant storage.
  2. The "Enable replication to disaster recovery location" option is also unselected by default. We can enable it as a default option.

Breaking Changes

  1. Replace me
  2. Replace me

Testing Evidence

Confirmed that the default value has been changed from the link below,

AVD LZA Baseline deployment

1. image

2. image

AVD LZA Custom Image Build

5 and 6.

image

As part of this Pull Request I have

danycontre commented 6 months ago

@yahanda thanks for your feedback and contribution, some comments:

  1. Selecting ZRS by default can cause the deployment to fail if ZRS is not available in the region, by default the storage will use the same region selected for the session hosts and on this one we are only checking if AZs are available for compute. Would recommend updating the portal UI to run an API check and confirm if ZRS is available in the region before enabling it (similar to the API calls we are doing for VMs).
  2. Agree.
  3. Agree.
  4. Agree, though would also add the scaling configurations for replicas as this also may impact image availability.
  5. Agree.
  6. Agree.

@moisesjgomez @swathibhat1

yahanda commented 6 months ago

@danycontre thank you for your comments. I've fixed the UI and doc based on your comments. (cc @moisesjgomez @swathibhat1)

1. When choosing ZRS, only regions with AZ support are available for the “Session hosts region”, regardless of whether “Availability zones” in the session hosts section is selected or not. image

Conversely, when not choosing ZRS and not choosing “Availability zones”, regions that do not have AZ support will also be available in the region drop down.

image

4. I've made a small update to the documentation.

danycontre commented 6 months ago

@yahanda thanks for the updates, please ping me offlie to further discuss updates.

cc: @swathibhat1

danycontre commented 6 months ago

@swathibhat1 @yahanda merging and will revert ZRS changes for us to further discuss.

swathibhat1 commented 6 months ago

[like] Swathi Bhat reacted to your message:

From: Dany Contreras @.> Sent: Tuesday, March 26, 2024 10:05:01 PM To: Azure/avdaccelerator @.> Cc: Swathi Bhat @.>; Mention @.> Subject: Re: [Azure/avdaccelerator] update WAF compliant improvement (PR #600)

Merged #600https://github.com/Azure/avdaccelerator/pull/600 into main.

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/avdaccelerator/pull/600#event-12257186969, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A3ITHAI4BP74CDAXGEQ476DY2HWI3AVCNFSM6AAAAABFCUDVVKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGI2TOMJYGY4TMOI. You are receiving this because you were mentioned.Message ID: @.***>