Adding support for OS disk size specification on TRE config file.#2546
Conversation
Co-authored-by: Martin O'Reilly <moreilly@turing.ac.uk>
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||
Co-authored-by: Martin O'Reilly <moreilly@turing.ac.uk>
There was a problem hiding this comment.
Code changes LGTM. Note that I was at the desk with Carlos while these changes were made so we applied the second pair of eyes at code change time.
@cptanalatriste Please could you re-run the end to end VM deployment using an OS disk size that is different to the 64GB default specified in the code, to validate that this does override the defaults and propagate the user-specified OS disk size to the deployed VM.
I added a couple of inline notes where the defaults are set asking if we can easily add unit tests that validate the defaults are overridden when a different value is provided as input.
If the end to end test shows the user-specified OS disk value propagating to the deployed VM, I'm happy to merge this PR. If its easy to incorporate unit tests baking this validation into the test suite, it would be great to see these added but I would not consider the lack of these a blocker to merging.
Hi @martintoreilly , I did the requested end-to-end test. Given the new TRE configuration (see the first screenshot), we can see that the deployed OS disk has the size specified (see second screenshot).
|
a95fcb5
into
alan-turing-institute:develop


✅ Checklist
Enable foobar integrationrather than515 foobar).develop.🚦 Depends on
N/A
As part of this PR:
IMPORTANT!: It is not possible to update the OS Disk size using the DSH CLI, due to Pulumi limitations. Attempts of doing it will fail with the following error:
We expect users to manually update OS Disk size (using the Portal or Azure CLI), and normalise Pulumi state later.
🌂 Related issues
Closes: #2545
🔬 Tests
The workspaces are created with the right disk size: