-
Notifications
You must be signed in to change notification settings - Fork 420
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
feat: New module azure-stack-hci/virtual-hard-disk
module
#4482
base: main
Are you sure you want to change the base?
Conversation
Here is removal failed if not add |
.github/workflows/avm.res.azure-stack-hci.virtual-hard-disk.yml
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
module azlocal 'br/public:avm/res/azure-stack-hci/cluster:0.1.0' = { |
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.
Usually we don't use modules to deploy dependencies. One of the reasons for that is to minimize the risk of circular dependencies. Is there a reason why the cluster module was used instead?
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.
The module cluster
is required to create vhd. I have a question, in testing, I understand that the user experience is exactly the same. So under what circumstances would circular references occur?
customLocationResourceId: customLocation.id | ||
diskSizeGB: 4 | ||
dynamic: true | ||
tags: { |
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.
Please add a max test where to validate all optional parameters, including role assignments. tags can be removed from waf and validated in the other deployment test instead
avm/res/azure-stack-hci/virtual-hard-disk/tests/e2e/waf-aligned/main.test.bicep
Outdated
Show resolved
Hide resolved
avm/res/azure-stack-hci/virtual-hard-disk/tests/e2e/defaults/main.test.bicep
Outdated
Show resolved
Hide resolved
@@ -77,6 +77,7 @@ runs: | |||
'avm/res/azure-stack-hci/cluster' # Failing on resource deletion when trying to delete RBAC at subscription level | |||
'avm/res/azure-stack-hci/logical-network' # Failing on resource deletion when trying to delete RBAC at subscription level | |||
'avm/res/azure-stack-hci/network-interface' # Failing on resource deletion when trying to delete RBAC at subscription level | |||
'avm/res/azure-stack-hci/virtual-hard-disk' # Failing on resource deletion when trying to delete RBAC at subscription level |
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.
We'd need to update the ci separately from the PR, to avoid triggering all module workflows on this change. I'll have that done in a separate PR and let you know once ready to pull latest from upstream
@description('Optional. The password of the LCM deployment user and local administrator accounts.') | ||
@secure() | ||
param localAdminAndDeploymentUserPass string = newGuid() | ||
|
||
@description('Required. The app ID of the service principal used for the Azure Stack HCI Resource Bridge deployment.') | ||
@secure() | ||
#disable-next-line secure-parameter-default | ||
param arbDeploymentAppId string = '' | ||
|
||
@description('Required. The service principal ID of the service principal used for the Azure Stack HCI Resource Bridge deployment.') | ||
@secure() | ||
#disable-next-line secure-parameter-default | ||
param arbDeploymentSPObjectId string = '' | ||
|
||
@description('Required. The secret of the service principal used for the Azure Stack HCI Resource Bridge deployment.') | ||
@secure() | ||
#disable-next-line secure-parameter-default | ||
param arbDeploymentServicePrincipalSecret string = '' | ||
|
||
@description('Required. The service principal object ID of the Azure Stack HCI Resource Provider in this tenant. Can be fetched via `Get-AzADServicePrincipal -ApplicationId 1412d89f-b8a8-4111-b4fd-e82905cbd85d` after the \'Microsoft.AzureStackHCI\' provider was registered in the subscription.') | ||
@secure() | ||
#disable-next-line secure-parameter-default | ||
param hciResourceProviderObjectId string = '' |
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.
Why all these input parameters if not used? Same comment is valid for the waf test
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.
They are used in nested dependencies.
Description
Pipeline Reference
Type of Change
version.json
:version.json
.version.json
.Checklist
Set-AVMModule
locally to generate the supporting module files.