Add support for OS disk performance tier configuration#1
Merged
sgreene570 merged 13 commits intomainfrom Nov 10, 2025
Merged
Conversation
This commit adds a new configuration option 'os_disk_performance_tier' to the Azure ARM builder, allowing users to specify a performance tier for the OS disk during VM creation. This addresses issue hashicorp#548. The performance tier can be set to values like P1, P2, P3, P4, P6, P10, P15, P20, P30, P40, P50, P60, P70, P80, which allows users to improve disk performance for disk-intensive operations during the build process without having to increase the final image artifact size. Changes: - Added OSDiskPerformanceTier field to Config struct in config.go - Added Tier field to ManagedDisk struct in template.go - Added SetOSDiskPerformanceTier method in template_builder.go - Updated template_factory.go to call SetOSDiskPerformanceTier when configured - Updated config.hcl2spec.go to include the new field for HCL2 support Fixes hashicorp#548 Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Added assertAllowedOSDiskPerformanceTier function to validate that the os_disk_performance_tier configuration value is one of the allowed Azure performance tiers: P1, P2, P3, P4, P6, P10, P15, P20, P30, P40, P50, P60, P70, P80. The validation is called during the config Prepare phase and will return a clear error message if an invalid tier is specified. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Used packer-sdc to properly regenerate the autogenerated files: - config.hcl2spec.go: Added HCL2Spec mapping for os_disk_performance_tier - Config-not-required.mdx: Added complete documentation for the new field Also improved the documentation comment in config.go to include: - Full description of the performance tier feature - List of valid tier values (P1-P80) - Link to Azure documentation The manual edit from the previous commit has been replaced with properly generated code that includes all necessary HCL2 mappings. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Added two test suites to verify the OS disk performance tier functionality:
1. Template generation test (TestSetOSDiskPerformanceTier):
- Verifies that SetOSDiskPerformanceTier correctly sets the tier property
- Checks that the generated ARM template JSON contains the tier value
2. Validation tests (config_test.go):
- TestConfigShouldRejectInvalidOSDiskPerformanceTier: Verifies that
invalid tier values (e.g., P99) are rejected with appropriate error messages
- TestConfigShouldAcceptValidOSDiskPerformanceTier: Verifies that all
valid tier values (P1-P80) are accepted during configuration validation
All tests pass successfully.
Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Updated TestSetOSDiskPerformanceTier to follow the same approval test pattern used by all other tests in template_builder_test.go: - Removed custom string matching logic - Now uses approvaltests.VerifyJSONBytes() to verify the entire JSON structure - Added approved JSON file that shows the tier property is correctly set at storageProfile.osDisk.managedDisk.tier with value "P30" This provides more comprehensive verification of the template generation compared to simple string matching. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
sgreene570
reviewed
Nov 7, 2025
This commit fixes the implementation of os_disk_performance_tier to work correctly with Azure's ARM template API: 1. **Architecture Change**: Azure doesn't support the 'tier' property on VM managed disks. The implementation now creates a standalone Microsoft.Compute/disks resource with the tier property, then attaches it to the VM. 2. **Template Generation**: - Added DiskCreationData struct to support disk resource creation - Extended Properties struct with disk-specific fields (CreationData, DiskSizeGB, Tier) - Rewrote SetOSDiskPerformanceTier to create standalone disk resource with creationData.FromImage 3. **Validation**: Added validation to reject os_disk_performance_tier with: - Marketplace images (image_publisher/image_offer/image_sku) - only SIG/Managed Images supported - disk_encryption_set_id (not yet supported) 4. **Tests**: - Updated approval test to use SIG instead of marketplace images - Fixed config test to use managed image source instead of marketplace - All tests passing This implementation now correctly generates ARM templates that Azure accepts for SIG-based builds with performance tier configuration. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
This commit addresses architectural issues identified in code review: 1. **Removed invalid Tier field from ManagedDisk struct**: Azure doesn't support tier on VM managed disks (only on standalone disk resources). Removed the field to prevent future misuse and accurately reflect Azure's schema. 2. **Simplified imageReference logic**: Changed from setting both imageReference and galleryImageReference to setting only one field per case: - SIG with full resource ID: use galleryImageReference.id only - SharedGalleryImageId: use imageReference.sharedGalleryImageId only - CommunityGalleryImageId: use imageReference.communityGalleryImageId only This aligns with Azure API expectations and avoids ambiguous field combinations. 3. **Updated approval test**: Regenerated approved JSON to reflect the simplified structure with only galleryImageReference (no redundant imageReference field). All tests pass with these changes. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Azure disks don't support API version 2023-03-01 (which is used for VMs).
This commit fixes the deployment error by:
1. **Added diskApiVersion variable**: Created a new template variable 'diskApiVersion' set to '2023-04-02', which is a supported API version for Microsoft.Compute/disks resources.
2. **Updated disk resource**: Changed the OS disk resource to use [variables('diskApiVersion')] instead of [variables('computeApiVersion')].
3. **Updated all approval tests**: Regenerated all approval test JSON files to include the new diskApiVersion variable in the variables section.
This resolves the Azure deployment error:
'No registered resource provider found for location 'eastus2' and API version '2023-03-01' for type 'disks''
The fix ensures that disk resources use a compatible API version (2023-04-02) that supports both galleryImageReference and tier properties, while VM resources continue to use 2023-03-01.
Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Azure doesn't allow osProfile to be set when using createOption: 'Attach' for OS disks. This commit fixes the deployment error by: 1. **Added ClearOsProfile call**: At the end of SetOSDiskPerformanceTier, call ClearOsProfile() to remove the osProfile section from the VM template when using attached disks. 2. **Updated approval test**: Regenerated the TestSetOSDiskPerformanceTier approval test JSON to reflect the removed osProfile section. This resolves the Azure deployment error: 'Parameter OSProfile is not allowed because osDisk.createOption is specified as Attach.' **Important note**: When using os_disk_performance_tier, the source SIG image must already contain accessible credentials (admin user and SSH key) since Azure won't inject them via osProfile. The attached disk deployment assumes the image is pre-configured with the necessary access. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Implements Option B: inject SSH credentials via VMAccessForLinux extension
when using attached OS disks with performance tier.
This allows the disk tier to be effective immediately (via attached disk)
while still providing credential access (via VM extension), similar to how
the Azure portal handles this scenario.
Changes:
1. **Extract credentials from osProfile**: Before clearing osProfile, extract
the SSH public key and admin username to pass to the extension.
2. **Add VMAccessForLinux extension**: Create a VM extension resource that
injects the SSH credentials after the VM boots with the attached disk.
- Publisher: Microsoft.OSTCExtensions
- Type: VMAccessForLinux
- TypeHandlerVersion: 1.5
- Settings: username and ssh_key
3. **Change Settings type to interface{}**: Modified the Properties.Settings
field from *CustomScriptSettings to interface{} to support both
CustomScriptExtension settings and VMAccess extension settings.
4. **Updated approval test**: Regenerated the TestSetOSDiskPerformanceTier
approval test JSON to include the VMAccessForLinux extension resource.
This approach ensures:
- Disk tier is effective immediately at boot (attached disk)
- SSH credentials are available for Packer provisioning (VM extension)
- Works like the Azure portal's credential injection mechanism
Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
The VMAccessForLinux extension was failing with error:
'Enable failed: NoneType object has no attribute get'
This was caused by the extension expecting protectedSettings but receiving None.
Changes:
1. **Added ProtectedSettings field**: Added ProtectedSettings interface{} to
the Properties struct to support extension protected settings.
2. **Fixed extension configuration**: Updated VMAccessForLinux extension to use
the correct settings format:
- settings: { username, reset_ssh: true }
- protectedSettings: { ssh_key }
3. **Updated approval test**: Regenerated the TestSetOSDiskPerformanceTier
approval test JSON to reflect the correct extension configuration.
This follows the VMAccessForLinux 1.5 schema where:
- username and reset_ssh go in public settings
- ssh_key goes in protectedSettings (both for security and to avoid the
NoneType error)
The extension should now successfully inject SSH credentials when using
attached OS disks with performance tier.
Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
Per the Azure VMAccessForLinux extension documentation at: https://learn.microsoft.com/en-us/azure/virtual-machines/extensions/vmaccess-linux The correct schema shows that username, ssh_key, and reset_ssh all belong in protectedSettings, not in public settings. Changes: 1. **Moved username to protectedSettings**: The username field was incorrectly placed in public settings. Per the Azure docs, it belongs in protectedSettings along with ssh_key. 2. **Removed reset_ssh**: We don't need reset_ssh for our use case (injecting SSH credentials). The extension will work without it. 3. **Removed Settings field entirely**: Since we're not using any of the public settings (check_disk, repair_disk, disk_name), we can omit the Settings field. 4. **Updated approval test**: Regenerated the TestSetOSDiskPerformanceTier approval test JSON to reflect the correct extension configuration. The extension configuration now matches the Azure documentation schema: - protectedSettings: { username, ssh_key } - settings: (omitted - not needed for our use case) This should resolve the VMAccessForLinux extension deployment error. Co-Authored-By: Jonathan Mortensen <jmomort@gmail.com>
sgreene570
reviewed
Nov 8, 2025
sgreene570
reviewed
Nov 8, 2025
d3bcb25 to
0278313
Compare
|
tweaked and tested, working on my end! Going to merge to our fork so we can important into our other repos seamlessly - can always follow up with additional PRs as neede. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements support for setting OS disk performance tier during VM builds to improve disk I/O performance for disk-intensive operations.
Link to Devin run: https://app.devin.ai/sessions/7d2a23a5f78e405bb802cd69d6169718
Requested by: Jonathan Mortensen (jmomort@gmail.com) / @jmomort
What Changed
Adds a new
os_disk_performance_tierconfiguration option that allows users to specify a higher performance tier (P1-P80) for the OS disk during builds. This improves build performance without affecting the final image size.Key Implementation Details:
Due to Azure ARM template limitations (tier property only supported on standalone disk resources, not on VM managedDisk objects), this implementation:
Microsoft.Compute/disksresource with the specified tier and the VM's configured storage account typecreateOption: "FromImage"createOption: "Attach"osProfilewith attached disks)Recent fixes:
Limitations:
managed_image_storage_account_typeto be set toPremium_LRS(validated at config time)disk_encryption_set_id(validation prevents this combination)Files Changed
Core Implementation:
builder/azure/arm/config.go: Added field, validation for 14 valid tiers (P1-P80), restrictions for marketplace/encryption, Premium_LRS requirementbuilder/azure/arm/template_factory.go: Integrated into VM template creation flowbuilder/azure/common/template/template_builder.go: Implemented disk creation + extension logic with storage account type from VM configbuilder/azure/common/template/template.go: Added disk-specific structs and fieldsTests:
builder/azure/arm/config_test.go: Validation tests for valid/invalid tiersbuilder/azure/common/template/template_builder_test.go: Template generation approval testdiskApiVersionvariableGenerated Files:
builder/azure/arm/config.hcl2spec.go: HCL2 support (auto-generated)docs-partials/builder/azure/arm/Config-not-required.mdx: Documentation (auto-generated)Resolved Issues
Closes hashicorp#548
Rollback Plan
If a change needs to be reverted, we will roll out an update to the code within 7 days.
Changes to Security Controls
Credential Injection Change: This PR changes how SSH credentials are provided to the VM when using OS disk performance tier:
osProfileduring VM creationThe extension uses Azure's
protectedSettingsmechanism to securely transmit credentials (username and SSH key are encrypted in transit and not exposed in ARM template logs).Human Review Checklist
VMAccessForLinux Extension Config: The extension settings format was corrected in the final commits based on Azure documentation but hasn't been tested with a real deployment yet. Previous versions failed with various Azure API errors. Current config uses:
With no public
settingsfield. This needs to be verified with an actual Packer build against Azure.Storage Account Type Requirement: The implementation now correctly uses the VM's configured storage account type for the disk SKU and validates that it must be Premium_LRS. However, users might not realize they need to explicitly set
managed_image_storage_account_type: "Premium_LRS"in their config. Is the error message clear enough?Windows Compatibility: This implementation uses VMAccessForLinux extension. Does this break Windows builds? Should this feature be:
Error Handling: What happens if the VMAccessForLinux extension fails to provision? Will Packer detect this and fail the build gracefully, or will it hang trying to SSH?
API Version Split: Verify that using separate API versions for disks (
2023-04-02) and VMs (2023-03-01) is acceptable and won't cause future maintenance issues.Disk Creation from SIG: The disk is created with
createOption: "FromImage"andgalleryImageReference. Verify this is the correct way to create a disk from a Shared Image Gallery image (not just marketplace images).Security Review: Validate that using VMAccessForLinux extension for credential injection (instead of osProfile) doesn't introduce any security concerns or edge cases.
Example Usage