Skip to content

Conversation

@hvedati
Copy link

@hvedati hvedati commented Jul 21, 2025

Description

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copilot AI review requested due to automatic review settings July 21, 2025 20:37
@azure-client-tools-bot-prd
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates Azure Stack HCI VM test configuration and dependencies. It migrates test environment configurations to new Azure subscriptions/resource groups and updates API specifications to use a newer stable release.

  • Updated test environment configurations with new subscription IDs, resource groups, and resource paths
  • Enhanced test coverage by adding new VM state management tests (Save and Pause operations)
  • Updated API specifications from preview version to stable release (2025-01-13)

Reviewed Changes

Copilot reviewed 8 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
env.json / envVariables.json Updated test environment with new Azure subscription, tenant, and resource identifiers
AzStackHCIVMVirtualMachine.Tests.ps1 Added new test cases for VM save and pause operations with proper error handling
AzStackHCIVMStoragePath.Tests.ps1 Updated to use environment variable for storage path instead of hardcoded value
AzStackHCIVMNetworkInterface.Tests.ps1 Updated to use environment variable for subnet ID instead of hardcoded value
AzStackHCIVMImageNew.Tests.ps1 Fixed test formatting and updated to use dynamic resource IDs from environment
New-AzStackHCIVMImage.ps1 Added test playback detection, secure string handling for ImagePath, and minor documentation fixes
README.md Updated commit hash and API specification to stable version
Comments suppressed due to low confidence (1)

@hvedati
Copy link
Author

hvedati commented Jul 21, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Commenter does not have sufficient privileges for PR 28237 in repo Azure/azure-powershell

@hvedati hvedati changed the title Dev/blairurish/azlocalvms hcrp update [StackHCIVM] hcrp api version update Jul 21, 2025
@hvedati
Copy link
Author

hvedati commented Jul 22, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Commenter does not have sufficient privileges for PR 28237 in repo Azure/azure-powershell

@YanaXu
Copy link
Contributor

YanaXu commented Jul 22, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 22, 2025

Hi @hvedati, the ChangeLog file path is src/StackHCIVM/StackHCIVM/ChangeLog.md. Please add a line right below Upcoming Release using the past tense to describe the purpose of this PR. Thanks.

Please check the placeholders:

  • src/StackHCIVM/StackHCIVM.Autorest/docs/Get-AzStackHCIVMAttestationStatuses.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Get-AzStackHCIVMNetworkSecurityGroup.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Get-AzStackHCIVMSecurityRule.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/New-AzStackHCIVMNetworkSecurityGroup.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/New-AzStackHCIVMSecurityRule.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Remove-AzStackHCIVMNetworkSecurityGroup.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Remove-AzStackHCIVMSecurityRule.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Save-AzStackHCIVMVirtualMachine.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Suspend-AzStackHCIVMVirtualMachine.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Update-AzStackHCIVMNetworkSecurityGroupTag.md
  • src/StackHCIVM/StackHCIVM.Autorest/docs/Update-AzStackHCIVMSecurityRule.md

After you run autorest, build-module.ps1, please submit all changed files, including generate-info.json.

@YanaXu
Copy link
Contributor

YanaXu commented Jul 24, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 24, 2025

Hi @hvedati, please notice the next code complete date is 07/29/2025. If the PR can't be ready by then, it'll be left over to the next next release.

@YanaXu
Copy link
Contributor

YanaXu commented Jul 25, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 25, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 28, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 28, 2025

Please fix CI errors.

@YanaXu
Copy link
Contributor

YanaXu commented Jul 28, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@YanaXu
Copy link
Contributor

YanaXu commented Jul 29, 2025

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@hvedati hvedati requested a review from NoriZC August 19, 2025 23:19
@hvedati
Copy link
Author

hvedati commented Aug 19, 2025

@NoriZC I have gone through the breaking changes and there are two types of breaking changes present. The number corresponds to the row in the BreakingChanges.csv which is added to the PR.
4 , 9 , 18, 25 , 30, 25, 40, 45 , 50, 57 - The type of property 'OSProfileAdminPassword' of type 'Microsoft.Azure.PowerShell.Cmdlets.StackHCIVM.Models.IVirtualMachineInstance' has changed from 'System.String' to 'System.Security.SecureString'. This is a valid breaking change.

12, 13, 21, 22, 53, 54, - The cmdlet 'Update-AzStackHCIVMLogicalNetwork' no longer has output type 'Microsoft.Azure.PowerShell.Cmdlets.StackHCIVM.Models.ILogicalNetworks'. and The cmdlet 'Update-AzStackHCIVMImage' no longer has output type 'Microsoft.Azure.PowerShell.Cmdlets.StackHCIVM.Models.IGalleryImages'. This is due to an update in the swagger from the old model to typespec. This change makes sense and was a result of the autogeneration pointing at the new swagger. Since this powershell module is not used by customer it should be safe to update this type.

Both of these types of breaking changes are needed in our module. It should be safe to include these breaking changes since this module is not used by customers. What is the process for adding these breaking changes?

@VeryEarly
Copy link
Collaborator

Hi @hvedati
This module is GAed, please involve PMs to send email to our team to provide data and justification why these breaking changes are safe and necessary. And discuss whether we should keep this module as GAed going forward.

@github-actions
Copy link

github-actions bot commented Sep 1, 2025

‼️ DO NOT MERGE THIS PR ‼️
This PR was labeled "Do Not Merge" because it contains code change that cannot be merged. Please contact the reviewer for more information.

@isra-fel
Copy link
Member

isra-fel commented Oct 2, 2025

Hi @hvedati the securestring breaking changes make sense but can you elaborate on "Since this powershell module is not used by customer it should be safe to update this type."? If not by cusomters, who would use it and what would the impact of the breaking change be to them?

@isra-fel isra-fel assigned hvedati and unassigned VeryEarly Oct 2, 2025
@isra-fel isra-fel added needs-revision Contains Breaking Change This PR contains breaking change labels Oct 2, 2025
@github-actions
Copy link

github-actions bot commented Oct 2, 2025

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

To the author of the pull request,
This PR was labeled "Breaking Change Release" because it contains breaking changes.

  • According to our policy, breaking changes can only take place during major release and they must be preannounced.
  • Please follow our guide on the detailed steps.
  • Required: Please fill in the task below to facilitate our contact,you will receive notifications related to breaking changes.

@isra-fel
Copy link
Member

Kindly ping @hvedati

@isra-fel
Copy link
Member

Closing the PR for lack of activity. Feel free to reopen to continue working on it. Thanks 😀

@isra-fel isra-fel closed this Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants