-
Notifications
You must be signed in to change notification settings - Fork 4.1k
update Az.AksArc tests and fix commands #28645
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this 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 adds comprehensive tests for the Az.AksArc PowerShell module and fixes various command parameter set configurations. The changes include creating extensive test coverage for cluster management operations, improving parameter organization, and correcting documentation to ensure proper command functionality.
- Extensive test suite implementation with live-only tests for cluster and nodepool operations
- Parameter set restructuring and validation fixes for Update/New/Remove commands
- Documentation corrections and help file updates for improved clarity
Reviewed Changes
Copilot reviewed 68 out of 73 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/AksArc/AksArc/help/*.md |
Updates command help documentation with corrected parameter sets and descriptions |
src/AksArc/AksArc/ChangeLog.md |
Documents fixes and test additions for version 0.2.4 |
src/AksArc/AksArc/Az.AksArc.psd1 |
Updates module dependencies and configuration settings |
src/AksArc/AksArc.Autorest/test/*.ps1 |
Adds comprehensive test coverage for all AksArc commands |
src/AksArc/AksArc.Autorest/test/utils.ps1 |
Implements test environment setup with cluster creation |
### -KubernetesVersion | ||
The version of Kubernetes in use by the provisioned cluster. | ||
Kubernetes version to upgrade to. | ||
If not provided, the cluster will be upgraded to the latest supported version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description provides clear information about the default behavior when no version is specified, which is helpful for users understanding the command's functionality.
If not provided, the cluster will be upgraded to the latest supported version. |
Copilot uses AI. Check for mistakes.
apt update | ||
apt install openssh-client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling for the apt commands to gracefully handle cases where the package manager is not available or the commands fail.
apt update | |
apt install openssh-client | |
if (-not (Get-Command apt -ErrorAction SilentlyContinue)) { | |
throw "apt package manager is not available. Please install openssh-client manually." | |
} | |
& apt update | |
if ($LASTEXITCODE -ne 0) { | |
throw "Failed to run 'apt update'. Please check your network connection and package sources." | |
} | |
& apt install -y openssh-client | |
if ($LASTEXITCODE -ne 0) { | |
throw "Failed to install openssh-client. Please check your package sources and try again." | |
} |
Copilot uses AI. Check for mistakes.
# May need to update this periodically to update to available versions. | ||
$k8sVersion = "1.32.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coding the Kubernetes version may cause test failures when this version becomes unavailable. Consider dynamically selecting an available version or making this configurable.
# May need to update this periodically to update to available versions. | |
$k8sVersion = "1.32.6" | |
# Dynamically select Kubernetes version: use env var if set, else get latest available, else fallback. | |
if ($env:K8S_VERSION) { | |
$k8sVersion = $env:K8S_VERSION | |
} else { | |
try { | |
# Attempt to get the latest available Kubernetes version for the location | |
Import-Module Az.AksArc -ErrorAction SilentlyContinue | |
$location = $env:LOCATION | |
if (-not $location) { | |
# Fallback to a default location if not set | |
$location = "eastus" | |
} | |
$versions = Get-AzAksArcKubernetesVersion -Location $location | Sort-Object -Property OrchestratorVersion -Descending | |
if ($versions -and $versions[0].OrchestratorVersion) { | |
$k8sVersion = $versions[0].OrchestratorVersion | |
} else { | |
$k8sVersion = "1.32.6" # fallback | |
} | |
} catch { | |
$k8sVersion = "1.32.6" # fallback | |
} | |
} |
Copilot uses AI. Check for mistakes.
4127fdd
to
108b404
Compare
108b404
to
617e476
Compare
Description
Added tests and fixed some commands. See
AksArc/AksArc/ChangeLog.md
.Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.md
and reviewed the following information:ChangeLog.md
file(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
.## Upcoming Release
header in the past tense.ChangeLog.md
if no new release is required, such as fixing test case only.Got this error: