Skip to content
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

Linux VM OS Disk #4317

Open
tamirkamara opened this issue Feb 5, 2025 · 7 comments
Open

Linux VM OS Disk #4317

tamirkamara opened this issue Feb 5, 2025 · 7 comments
Labels
story Stories are the smallest unit of work to be done for a project.

Comments

@tamirkamara
Copy link
Collaborator

tamirkamara commented Feb 5, 2025

Description

As a Researcher
I want to have access to more disk space in Linux VM
So that I could store data in the VM for faster processing

@tamirkamara tamirkamara added the story Stories are the smallest unit of work to be done for a project. label Feb 5, 2025
@TonyWildish-BH
Copy link
Contributor

@tamirkamara, a few questions if you don't mind:

  • Is the motivation purely to have something faster than the shared storage? If so, is there any option to specify a performance tier for the shared storage instead, which might be simpler?
  • Is this intended to be a shared disk, among any/all Linux VMs, or just per VM?
  • If it's per VM, is there any reason not to simply increase the size of the system disk, letting the user specify the size, and perhaps the performance tier?
  • Is there any requirement for Windows VMs to have the same performant disks?

@marrobi
Copy link
Member

marrobi commented Feb 6, 2025

Maybe initially we do OS disk? Think that is a quick win, and I've hit this before. @tamirkamara should we create a new issue for this, or I can reword this one.

variable "os_disk_account_type" {
  description = "The type of storage account to use for the OS disk."
  type        = string
  default     = "Standard_LRS"

  validation {
    condition = contains(["Standard_LRS", "Premium_LRS", "StandardSSD_LRS", "UltraSSD_LRS"], var.os_disk_account_type)
  }
}

variable "os_disk_size" {
  description = "The size of the OS disk in GB."
  type        = number
  default     = 64

  validation {
    condition = contains([64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32767], var.os_disk_size)
  }
}

Need to prevent downsizing, not sure how that can be done.

@tamirkamara
Copy link
Collaborator Author

@TonyWildish-BH the motivation is mainly to allow more software to be installed on the VM. Not need to share with other VMs.
Indeed OS disk size is easier for this, however as pointed out earlier by Vijay, every image has a different default OS disk size so setting something below that might cause a problem.

@TonyWildish-BH
Copy link
Contributor

thanks for the clarification, @tamirkamara. In that case, maybe this ticket should be renamed to 'System Disk', not 'Data Disk'?

I'd simply set a minimum value, say 50GB for Linux, 150GB for Windows, which is above the default OS sizes I've seen so far, and allow any value above that. I don't see the point in restricting to an enumerated set of values.

@marrobi
Copy link
Member

marrobi commented Feb 6, 2025

There needs to be a minimum size, also maximum, also get billed at set size increments - https://learn.microsoft.com/en-us/azure/virtual-machines/disks-types#ultra-disk-size hence a drop down of choices might be the best option.

@TonyWildish-BH
Copy link
Contributor

There needs to be a minimum size, also maximum, also get billed at set size increments - https://learn.microsoft.com/en-us/azure/virtual-machines/disks-types#ultra-disk-size hence a drop down of choices might be the best option.

fair enough, thanks!

@marrobi marrobi changed the title Linux VM Data Disk Linux VM OS Disk Feb 6, 2025
marrobi added a commit that referenced this issue Feb 6, 2025
Fixes #4317

Add support for configurable OS disk account type and size for Linux and Windows VMs.

* **Linux VM Changes:**
  - Add new parameters `os_disk_account_type` and `os_disk_size` in `parameters.json`.
  - Update `porter.yaml` to include new parameters and update `install`, `upgrade`, and `uninstall` steps.
  - Add new properties `os_disk_account_type` and `os_disk_size` in `template_schema.json` and update the `required` array.
  - Add new variables `os_disk_account_type` and `os_disk_size` in `variables.tf`.
  - Update `os_disk` block in `linuxvm.tf` to use new variables `os_disk_account_type` and `os_disk_size`.

* **Windows VM Changes:**
  - Add new parameters `os_disk_account_type` and `os_disk_size` in `parameters.json`.
  - Update `porter.yaml` to include new parameters and update `install`, `upgrade`, and `uninstall` steps.
  - Add new properties `os_disk_account_type` and `os_disk_size` in `template_schema.json` and update the `required` array.
  - Add new variables `os_disk_account_type` and `os_disk_size` in `variables.tf`.
  - Update `os_disk` block in `windowsvm.tf` to use new variables `os_disk_account_type` and `os_disk_size`.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/microsoft/AzureTRE/issues/4317?shareId=XXXX-XXXX-XXXX-XXXX).
@marrobi
Copy link
Member

marrobi commented Feb 6, 2025

Not tested, but let copilot do some work - #4334

If someone is able to give it a test, or even pick up the PR and finish off, that would be great.

Might need to look at how to prevent reduction in disk size, or what happens if try to reduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
story Stories are the smallest unit of work to be done for a project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants