Add SIG to Lab Account #749
Conversation
| @@ -338,29 +338,55 @@ function Publish-Labs { | |||
| $ConfigObject | |||
| ) | |||
|
|
|||
| $lacs = $ConfigObject | Select-Object -Property ResourceGroupName, LabAccountName -Unique | |||
| $lacs = $ConfigObject | Select-Object -Property ResourceGroupName, LabAccountName, SharedGalleryResourceGroupName, SharedGalleryName -Unique | |||
There was a problem hiding this comment.
I'm not sure we should do it this way... This means that the bulk lab creation scripts can only use a shared image gallery from the current subscription. We should either add a "SharedGallerySubscriptionId" or we should consider just having a single field with the full shared image gallery resource ID. (I like the single field idea, because it could just be an empty column or missing entirely if the user doesn't need it - vs needing to do validation across 3 columns to make sure there isn't a missing field...
|
|
||
| $gallery = $labAccount | Get-AzLabAccountSharedGallery | ||
| if ($gallery -ne $null) { | ||
| Write-Host "$LabAccountName lab account already has attached gallery $gallery." |
There was a problem hiding this comment.
If it's the wrong gallery that's attached to the lab account, should we change it to what's in the CSV file? I think currently we don't 'fix' settings in other places - but I'm wondering what the user's expectation would be. Perhaps we should consider this as a "Upsert" (update/insert) operation when running the scripts for any changes? The downside is that changing the SIG if there are labs & student VMs means that users couldn't reset the student VMs from SIG anymore...
There was a problem hiding this comment.
Yes, I was hesitant changing it if it's already set for the reasons you mentioned. I was thinking that if there are failures of some sort (for example, 503 which i've hit occassionally), which we could also include when there is an attached SIG already, that we could output this in a .csv...similar to what you're planning. When you're done with your code, we can likely reuse it here too.
|
|
||
| if ($gallery -ne $null) { | ||
| Write-Host "$SharedGalleryName shared gallery found." | ||
| New-AzLabAccountSharedGallery -LabAccount $labAccount -SharedGallery $gallery |
There was a problem hiding this comment.
It might be a good idea to do this by Resource ID instead of by object... The reason is because if the SIG is in a different subscription- we don't want to manually swap subscriptions back and forth to get the object if we don't have to (and I think the API just needs the SIG ID)
There was a problem hiding this comment.
Good catch - I've changed it to look by resource id...but, i am still having issues with it working to find a SIG successfully across subs. I've tested it in my subs, but I have different tentants involved. Also, tried it with Roger where we used our Microsoft subs, and still couldn't get it to work. I'd like to talk to you more to see what I'm missing.
| $lacs = $ConfigObject | Select-Object -Property ResourceGroupName, LabAccountName, SharedGalleryResourceId, EnableSharedGalleryImages | Sort-Object -Property ResourceGroupName, LabAccountName | ||
| $lacNames = $lacs | Select-Object -Property ResourceGroupName, LabAccountName -Unique | ||
|
|
||
| # Get the first unique resource group\lab account and attempt to attach a shared gallery if one is specified in the csv. If the resource group\lab account exists |
There was a problem hiding this comment.
I wonder if at this point we should require that the lab account, location, resource group name and shared image gallery id all match exactly... I think this might be a good validation item to force people to do the right thing rather than have the smarts here...
| $images = $labAccount | Get-AzLabAccountSharedImage -EnableState All | ||
| foreach ($imageName in $imageNames) | ||
| { | ||
| $image = $images | Where-Object { $_.name -like $imageName } | Set-AzLabAccountSharedImage -EnableState Enabled |
There was a problem hiding this comment.
Should we explicitly set the ones not in the CSV file as disabled, if the field is included? I'm not sure but that might be the default anyway right after attaching the SIG...
There was a problem hiding this comment.
Yes I can do that...i think that makes sense to do.
This is the first part of adding the ability to attach SIG to a lab account.
In my next PR, I will add support for enabling a custom image in SIG.