-
Notifications
You must be signed in to change notification settings - Fork 27
Task7 azurestoragescript #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
79a1f9f
53f8801
76ae1cf
4949fbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
*.log |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
Start-Transcript -Path "$PSScriptRoot\create_Azure_StorageAccountlog.log" | ||
Write-Host "Logging to $PSScriptRoot\create_Azure_StorageAccountlog.log" | ||
Write-Host "Create Azure Storage Account" | ||
|
||
# Name of resource group | ||
$resourceGroup = "nsc-ad440-winter2021-Th-StorageRG" | ||
# Location for Azure resources | ||
$location = "westus2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @theanticrumpet Same comment as above CC:// @1jc |
||
#Name of storage account | ||
$storageAccountName = "nscad440thsa" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @theanticrumpet Same comment as above CC:// @1jc |
||
|
||
#Check if the resource group exists | ||
Get-AzResourceGroup -Name $resourceGroup -ErrorVariable noRG -ErrorAction SilentlyContinue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @theanticrumpet I do not see a logic to sign into the Azure account. This line will fail if I try to run the script. There should be logic to sign into Azure and select the right tenant and subscription. CC:// @1jc |
||
if($noRG){ | ||
#Resource group does not exist | ||
Write-Host "Creating Resource Group $resourceGroup" | ||
New-AzResourceGroup -Name $resourceGroup -Location $location | ||
} | ||
|
||
#Check if storage account exists | ||
#Returns a CheckNameAvailabilityResult object, use property NameAvailable to get boolean value of if the name is available | ||
$doesntExist = Get-AzStorageAccountNameAvailability -Name $storageAccountName | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @theanticrumpet I would prefer that you use ARM template to create the storage account instead doing this manually in the script. ARM templates provide additional benefits like for example checking if the resource exist, can be run asynchronously etc. I would suggest you investigate that option. CC:// @1jc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely looks like I've got more reading to do on general Azure + Powershell scripting instead of specific functions |
||
|
||
if($doesntExist.NameAvailable){ | ||
#If the storage account does not exist | ||
Write-Host "Creating Storage Account $storageAccountName" | ||
New-AzStorageAccount -ResourceGroupName $resourceGroup ` | ||
-Name $storageAccountName ` | ||
-Location $location ` | ||
-SkuName Standard_RAGRS ` | ||
-Kind StorageV2 | ||
|
||
if(Get-AzStorageAccount -ResourceGroupName $resourceGroup -Name $storageAccountName){ | ||
Write-Host "Successfully created storage account $storageAccountName" | ||
} else { | ||
throw "Failed to create storage account. Failure in create_Azure_StorageAccount.ps1" | ||
} | ||
} else { | ||
#If the storage account already exists | ||
Write-Host "Storage Account $storageAccountName already exists" | ||
} | ||
|
||
Read-Host -Prompt "Press enter to continue" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line will block the script for executing without manual interaction. Not a good practice for automation scripts. CI/CD and other automation systems will not be able to use your script. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, do I want to take user input or not? Wouldn't the previous examples of requiring user input (EG: for resource group) also halt? Should I just make the whole thing take command line arguments as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be wrong but for those dynamic variables you can use the Key Vault and share them within your team. |
||
Stop-Transcript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theanticrumpet Hard coding the name of the resource group is not a group practice. This makes the script inflexible - i.e. once you use it you cannot reuse it again if the resource groups. It is better if you make those at least input parameters for the script.
CC:// @1jc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a little hesitant to do what you suggested in slack where I take multiple inputs and combine them into the RG name since that would just lead to a bunch of similarly named but different RGs. Would prompting for the RG name be sufficient?