search

Azure / RDS-Templates

ARM Templates for Remote Desktop Services deployments
MIT License
482 stars 609 forks source link

For every action in the DSC we validate the environment #447

Open JimMoyle opened 4 years ago

JimMoyle commented 4 years ago

The WVD configuration.ps1 script runs these actions multiple times

  1. Validate the Service Principle
  2. Import the RDS Module
  3. Authenticate to the control plane
  4. Validate Tenant

By running ValidateServicePrincipal, ImportRDPSMod, AuthenticateRdsAccount and SetTenantGroupContextAndValidate from the functions.ps1 file.

To deploy 2 hosts it looks like we run this validation ~8 times, it's even in a DSC test script.

Can we run once and set a flag to save time?

nakranimohit0 commented 4 years ago

@JimMoyle Thank you for your feedback and suggestions.

  1. Validating service principal: sure, it might be a better idea to perform that in just TestScript of the Script Resource of DSC, but its not too concerning because its just a simple if statement check.
  2. Import the RD Module: Not too concerning because it doesn't get downloaded/imported every time the function is called except the very first time, rest is just a simple check. (Imported modules stay imported for the entire DSC)
  3. WVD Authentication: WVD Auth context stays alive only for current and any nested scopes in PowerShell. So, this has to be done for each of TestScript, SetScript of the Script Resource of DSC as the context will be lost outside that.
  4. Validate Tenant: Sure, this can also be done in just TestScript of the Script Resource of DSC

As far saving time goes, it doesn't save a whole lot of time by doing these operations just once because DSC simply takes longer to start and stop which is something we don't control. The only time consuming operation is importing the RD module which as I explained happens the very first time.

@lintFan FYI & in case you want to add anything here.

JimMoyle commented 4 years ago

I'd say it's not just about time savings (though that was the only justification I put in the original issue) , but also API calls to the control plane.

It might be worth combining all these calls to one function if you think they are needed:

Test-WvdIsValidForTest returns bool Test- WvdIsValidForSet resturns bool

Might make things easier