What kind of change is this?

Removal

What's the description of the change?

The database class item, AvdHostStatus, isn't being utilized anymore since the main portion of it is now included in the main AvdHost class. It should be removed entirely.

What are the proposed changes that need to be made?

In addition to what's been mentioned in issue #15, this class should be removed since it's not being used anywhere within the project.

Documents need to be created for explaining:

• How to deploy to Azure
• How to configure it to monitor hostpools

What kind of change is this?

Addition

What's the description of the change?

There's currently little to no error handling in the code. There needs to be a custom exception class (or classes) added to the project for handling/reporting errors that happen.

What are the proposed changes that need to be made?

No response

What kind of bug is this?

Unexpected behavior

If you selected "Other" in the previous section, what kind of bug is it?

N/A

Describe the bug.

During execution of EvaluateHostPoolsToMonitor_Timer or UpdateAvdHostStatuses_Timer, the Azure API client will throw an error that the HttpClient can't be accessed since it's disposed. This is a fault in how the API client handles token acquisition.

I'll need to rework the logic for how tokens are acquired.

Logs (Optional)

No response

What kind of change is this?

Addition

What's the description of the change?

Monitored hostpools are not removed from the database when the managed identity no longer has access to them or when the hostpool no longer exists in the Azure subscription.

What are the proposed changes that need to be made?

Add logic for removing the hostpool from the database when it no longer is returned by the Azure API. Possibly when the EvaluateHostPoolsToMonitor_Timer function is triggered.

What kind of change is this?

Change/Update

What's the description of the change?

The slowest portion of the Functions app is the initialization of the AzureApiService. I don't have any metrics on hand, but there's a noticeable performance issue with it. This can be especially painful when the Functions app is coming from a cold start.

What are the proposed changes that need to be made?

No response

What kind of change is this?

Addition

What's the description of the change?

Session hosts are currently configured to deallocate at a hard-set rate of 40 minutes of inactivity between evaluation cycles (2 cycles).

There needs to be a way to set a custom limit, so, for instance, you would want it to deallocate after a hour of inactivity. That would be set to 3 cycles instead.

What are the proposed changes that need to be made?

This one is bit of a tricky one.

The simple solution is to just add a configurable environment setting (Or, as it’s called in Azure, “App Settings”), but that doesn’t seem like the best option to go with.

A more complex solution would be to make it configurable in the database, but that would require some way to make it easily configurable for admins. Personally, I can handle interacting with the CosmosDB account since I’m very familiar with it; however, that’s not user friendly. Maybe start working on an API for configuring all of the bits and pieces of the solution? I’ll open another issue for this, because this would open up an opportunity to allow extra configuration on the backend for admins.

Then there’s the evaluation portion of it. Should it be based off of evaluation cycle counts or the amount of minutes the session host has been inactive? The timer is set to run every 20 minutes, but there’s a possibility I could drop that down to a lower interval to make the deallocations happen at a more granular level. There are two things I’ll have to look into:

1. The potential cost increase for this. I don’t think this will cause a massive increase as my current forecasted cost for running the Functions app, CosmosDB account, Log Analytics workspace, and Application Insights resources in my Azure subscription is a lot lower than it was originally forecasted to be (Now it’s only $0.74 USD this month, down from just shy of $2 USD).
2. Improving performance. I believe I’ve seen it take roughly 8 seconds per run. This might not be a huge issue, but it could cause some impact.

What kind of change is this?

Change/Update

What's the description of the change?

AppSettings.GetSetting() currently only returns the value back as a string. With this change, the type of the return should be generic and defined when executed.

What are the proposed changes that need to be made?

Change the return type from string to T.

What kind of change is this?

Addition

What's the description of the change?

In addition to issue #14, session hosts need to be removed from the database whenever the session host is no longer accessible by the managed identity or when the session host is no longer registered to the hostpool.

What are the proposed changes that need to be made?

Add logic for removing the session host from the database when it no longer is returned by the Azure API. Possibly when the EvaluateHostPoolsToMonitor_Timer function is triggered.

What kind of change is this?

Addition

What's the description of the change?

This change would add the ability to disable the deallocation of session host VMs and instead evaluate it as a dry run.

What are the proposed changes that need to be made?

The initial implementation will be focusing on a broad level by adding the ability to define an optional environment variable.

What kind of change is this?

Addition

What's the description of the change?

As it stands right now, 99% of the configuration is automated based off of the permissions that the managed identity has access to. This was done to make it as simple as possible, for myself if I’m being honest, to configure. Almost like a “turn-key solution” sort of setup, but that means that every hostpool, that is configured to be monitored, all get the same settings.

As I mentioned in #27, one possible solution would be to add an API for making these sorts of configurations. This would allow for more granular control, but this would take some time to implement.

As for accessing the API, which also means securely accessing it with authentication, I have two potential ideas:

1. A PowerShell module
2. A Blazor WebAssembly app

Both of those would allow me to reuse the same models/classes I use inside this Function app itself. It would require some refactoring to be done by moving all of the shared models/classes to their own class library. As for authentication, right now I’m thinking of focusing on Azure AD. It’s what I’m using and the likelihood that others, who would use this, do too.

As Emperor Palpatine once said:

Implementing an API is a pathway to many administrative and management abilities some consider to be unnatural.

Or something like that.

What are the proposed changes that need to be made?

No response

What kind of bug is this?

Crash

If you selected "Other" in the previous section, what kind of bug is it?

N/A

Describe the bug.

During the UpdateAvdHostStatuses_Timer function's execution, initializing a SessionHostDbEntry class throws an error to occur because a method could not b found.

Logs (Optional)

An exception of type 'System.MissingMethodException' occurred in SmallsOnline.AVD.ResourceManager.dll but was not handled in user code: 'Method not found: 'Azure.Core.ResourceIdentifier Azure.ResourceManager.Models.Resource.get_Id()'.'

   at SmallsOnline.AVD.ResourceManager.Lib.Models.Database.SessionHostDbEntry..ctor(VirtualMachineData virtualMachineData, HostPoolDbEntry avdHostPool, SessionHost avdSessionHostData, SessionHostDbEntry previousHostData) in src\SmallsOnline.AVD.ResourceManager.Lib\models\database\SessionHostDbEntry.cs:line 31
   at SmallsOnline.AVD.ResourceManager.Functions.UpdateAvdHostStatuses_Timer.Run(TimerInfo timer) in src\SmallsOnline.AVD.ResourceManager\functions\UpdateAvdHostStatuses_Timer.cs:line 100
   at Microsoft.Azure.Functions.Worker.Invocation.VoidMethodInvoker`2.InvokeAsync(TReflected instance, Object[] arguments) in D:\a\1\s\src\DotNetWorker.Core\Invocation\VoidMethodInvoker.cs:line 23
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionInvoker`2.InvokeAsync(Object instance, Object[] arguments) in D:\a\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionInvoker.cs:line 31
   at Microsoft.Azure.Functions.Worker.Invocation.DefaultFunctionExecutor.<ExecuteAsync>d__4.MoveNext() in D:\a\1\s\src\DotNetWorker.Core\Invocation\DefaultFunctionExecutor.cs:line 37

There are currently 5 warnings being generated on build due to not handling possible null values correctly.

Affected lines in code:

What kind of change is this?

Change/Update

What's the description of the change?

The current implementation for reading and writing data to the database is by using the primary read/write key assigned to the Azure CosmosDB account. This should be potentially changed to another method.

What are the proposed changes that need to be made?

A few options worth looking into:

• Automatic key rotation with Azure KeyVault so that the key isn’t static 100% of the time.
• Configure RBAC permissions for reading/writing data on the CosmosDB account and assign permissions to the managed identity created for the Functions app.

What kind of change is this?

Change/Update

What's the description of the change?

Instead of focusing on just personal hostpools, pooled hostpools, which are used for multi-session hosts, should be monitored as well.

What are the proposed changes that need to be made?

• Change the logic for adding hostpools that should be monitored. Specifically removing the check if a hostpool is personal or not.
• Implement a new way for ResourceManager to add hostpools for monitoring.

With the last point, I'm currently using an Azure resource tag defined on the hostpool resources to enable ResourceManager to monitor it. This would be a breaking change.

What kind of change is this?

Change/Update

What's the description of the change?

Currently the classes for the database items is in a namespace called SmallsOnline.AVD.ResourceManager.Models.AVD. This can cause confusion with the SmallsOnline.AVD.ResourceManager.Models.Azure.DesktopVirtualization namespace and the classes inside of it.

What are the proposed changes that need to be made?

For the namespace, I'm considering changing it to SmallsOnline.AVD.ResourceManager.Models.Database since they are specifically designated for database items. As for the class names themselves, I'm not entirely too sure what I should change them to.