Azure / bicep-registry-modules

Bicep registry modules
MIT License
471 stars 325 forks source link

[Bug Report]: Unable to attach existing disk to VM #2364

Open jpgutton opened 1 year ago

jpgutton commented 1 year ago

Describe the bug

It is not possible to attach an existing disk to a VM as it is missing the possibility to pass the resource id of a disk.

To reproduce

Try passing an existing disk resource id as a datadisk as shown below, note that I tried also with a full resource id in text instead of the output of a disk created earlier in the bicep file.

      dataDisks: [
        {
          createOption: 'Attach'
          lun: 0
          caching: 'None'
          deleteOption: 'Detach'
          managedDisk: {
            id: clustershareddisk.outputs.resourceId
          }
        }
      ]

Code snippet

// Existing code
      dataDisks: [for (dataDisk, index) in dataDisks: {
        lun: index
        name: '${name}-disk-data-${padLeft((index + 1), 2, '0')}'
        diskSizeGB: dataDisk.diskSizeGB
        createOption: contains(dataDisk, 'createOption') ? dataDisk.createOption : 'Empty'
        deleteOption: contains(dataDisk, 'deleteOption') ? dataDisk.deleteOption : 'Delete'
        caching: contains(dataDisk, 'caching') ? dataDisk.caching : 'ReadOnly'
        managedDisk: {
          storageAccountType: dataDisk.managedDisk.storageAccountType
          diskEncryptionSet: contains(dataDisk.managedDisk, 'diskEncryptionSet') ? {
            id: dataDisk.managedDisk.diskEncryptionSet.id
          } : null
        }
      }]

// Corrected code
      dataDisks: [for (dataDisk, index) in dataDisks: {
        lun: contains(dataDisk, 'lun') ? dataDisk.lun : index
        name: !contains(dataDisk, 'name') ? null : '${name}-disk-data-${padLeft((index + 1), 2, '0')}'
        diskSizeGB: contains(dataDisk, 'diskSizeGB') ? dataDisk.diskSizeGB : null
        createOption: contains(dataDisk, 'createOption') ? dataDisk.createOption : 'Empty'
        deleteOption: contains(dataDisk, 'deleteOption') ? dataDisk.deleteOption : 'Delete'
        caching: contains(dataDisk, 'caching') ? dataDisk.caching : 'ReadOnly'
        managedDisk: {
          storageAccountType: contains(dataDisk.managedDisk, 'storageAccountType') ? dataDisk.managedDisk.storageAccountType : null 
          diskEncryptionSet: contains(dataDisk.managedDisk, 'diskEncryptionSet') ? {
            id: dataDisk.managedDisk.diskEncryptionSet.id
          } : null
          id: contains(dataDisk.managedDisk, 'id') ? dataDisk.managedDisk.id : null
        }
      }]

Relevant log output

No response

microsoft-github-policy-service[bot] commented 3 months ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

AlexanderSehr commented 3 months ago

Hey @rahalan, just migrated this issue over from CARML. There is a fair chance this is implemented via your last few PRs, but it would be great if you could take a look and triage/close it.