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

azure: Allow selecting disk controller for the VM #517

Merged
merged 5 commits into from
Apr 5, 2024

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Apr 3, 2024

Not even tested yet.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)

Copy link
Member

@jepio jepio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this is exactly what I was hoping for!

@krnowak
Copy link
Member Author

krnowak commented Apr 4, 2024

Awesome, this is exactly what I was hoping for!

@jepio: Cool. It doesn't address the issue of setting NVMe support for our images though, but I couldn't see how it can be done - the DiskControllers stuff is not exposed in GalleryImageStorageProfile. Anyway, any hints how to test it, or were you able to see if it works?

@jepio
Copy link
Member

jepio commented Apr 4, 2024

Awesome, this is exactly what I was hoping for!

@jepio: Cool. It doesn't address the issue of setting NVMe support for our images though, but I couldn't see how it can be done - the DiskControllers stuff is not exposed in GalleryImageStorageProfile. Anyway, any hints how to test it, or were you able to see if it works?

This stuff is underdocumented, it's through an under specified "features" field in the gallery specification.

az sig image-definition create (...) --feature DiskControllerTypes=SCSI,NVMe

Source: https://learn.microsoft.com/en-us/azure/virtual-machines/enable-nvme-faqs#how-do-i-use-a-base-image-that-supports-nvme-and-create-a-custom-image-for-my-remote-disk-.

This should translate to this change to the gallery template:

       "properties": {
         "hyperVGeneration": "[parameters('hyperVGeneration')]",
         "architecture": "[parameters('architecture')]",
+        "features": [
+          { "name": "DiskControllerTypes", "value": "NVMe,SCSI" },
+        ],
         "identifier": {
           "offer": "Flatcar",
           "publisher": "kola",

@krnowak
Copy link
Member Author

krnowak commented Apr 4, 2024

Awesome, this is exactly what I was hoping for!

@jepio: Cool. It doesn't address the issue of setting NVMe support for our images though, but I couldn't see how it can be done - the DiskControllers stuff is not exposed in GalleryImageStorageProfile. Anyway, any hints how to test it, or were you able to see if it works?

This stuff is underdocumented, it's through an under specified "features" field in the gallery specification.

az sig image-definition create (...) --feature DiskControllerTypes=SCSI,NVMe

Source: https://learn.microsoft.com/en-us/azure/virtual-machines/enable-nvme-faqs#how-do-i-use-a-base-image-that-supports-nvme-and-create-a-custom-image-for-my-remote-disk-.

This should translate to this change to the gallery template:

       "properties": {
         "hyperVGeneration": "[parameters('hyperVGeneration')]",
         "architecture": "[parameters('architecture')]",
+        "features": [
+          { "name": "DiskControllerTypes", "value": "NVMe,SCSI" },
+        ],
         "identifier": {
           "offer": "Flatcar",
           "publisher": "kola",

Oh yeah, I saw the Features field, but as you said, it's undocumented and my assumption was that it was about stuff like "GPU for AI" support.

I'll update the PR.

@krnowak
Copy link
Member Author

krnowak commented Apr 4, 2024

Updated, I have bumped the api version in the template too, so they match the api version used in code. I visually checked that the bump didn't require any other changes in the template.

@krnowak krnowak marked this pull request as ready for review April 5, 2024 11:21
This bumps the Azure SDK version to v68.0.0 and the autorest to
0.9.23. The latter was done to avoid a warning about retracted version
due to some token refresh issues.

The compute API we updated to is still deprecated, but at least it
provides settings for NVMe disk controllers to use when creating a VM.
We bumped API version of compute in Go code to 2022-08-01, and do the
same for the template, since it's using compute resources.
@krnowak krnowak merged commit c46820f into flatcar-master Apr 5, 2024
2 checks passed
@krnowak krnowak deleted the krnowak/nvme-azure branch April 5, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants