Skip to content

Fix for https://github.com/Azure/azure-powershell/issues/5506 #5689

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 20, 2018
Merged

Fix for https://github.com/Azure/azure-powershell/issues/5506 #5689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
edd2a32
remove parameter sets
yolocs Mar 7, 2018
eadca31
add back azure file vol para set
yolocs Mar 9, 2018
920e5ba
add test
yolocs Mar 12, 2018
b68ce7d
Merge branch 'preview' into fix_reg
cormacpayne Mar 15, 2018
3961eba
Merge branch 'preview' into fix_reg
cormacpayne Mar 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ResourceManager/ContainerInstance/ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Additional information about change #1
-->
## Current Release
* Fix parameter sets issue for container registry and azure file volume mount
* Fix issue with Default Resource Group in CloudShell

## Version 0.2.3
Expand Down
8 changes: 6 additions & 2 deletions ...ontainerInstance/Commands.ContainerInstance.Test/ScenarioTests/ContainerInstanceTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,22 @@ function Test-AzureRmContainerGroupWithVolumeMount
$resourceGroupName = Get-RandomResourceGroupName
$containerGroupName = Get-RandomContainerGroupName
$location = Get-ProviderLocation "Microsoft.ContainerInstance/ContainerGroups"
$image = "alpine"
$image = "acc.azurecr.io/alpine"
$shareName = "acipstestshare"
$accountName = "acipstest"
$accountKey = "password"
$secureAccountKey = ConvertTo-SecureString $accountKey -AsPlainText -Force
$accountCredential = New-Object System.Management.Automation.PSCredential ($accountName, $secureAccountKey)
$registryUsername = "acc"
$registryPassword = "password"
$secureRegistryPassword = ConvertTo-SecureString $registryPassword -AsPlainText -Force
$registryCredential = New-Object System.Management.Automation.PSCredential ($registryUsername, $secureRegistryPassword)
$mountPath = "/mnt/azfile"

try
{
New-AzureRmResourceGroup -Name $resourceGroupName -Location $location
$containerGroupCreated = New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RestartPolicy "Never" -Command "ls $mountPath" -AzureFileVolumeShareName $shareName -AzureFileVolumeAccountCredential $accountCredential -AzureFileVolumeMountPath $mountPath
$containerGroupCreated = New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryCredential $registryCredential -RestartPolicy "Never" -Command "ls $mountPath" -AzureFileVolumeShareName $shareName -AzureFileVolumeAccountCredential $accountCredential -AzureFileVolumeMountPath $mountPath

Assert-NotNull $containerGroupCreated.Volumes
Assert-NotNull $containerGroupCreated.Volumes[0].AzureFile
Expand Down
6 changes: 3 additions & 3 deletions ...nstance.Test.ScenarioTests.ContainerInstanceTests/TestCreateContainerGroupWithVolume.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@
"RequestUri": "/subscriptions/ae43b1e3-c35d-4c8c-bc0d-f148b4c52b78/resourceGroups/ps9107/providers/Microsoft.ContainerInstance/containerGroups/ps2492?api-version=2018-02-01-preview",
"EncodedRequestUri": "L3N1YnNjcmlwdGlvbnMvYWU0M2IxZTMtYzM1ZC00YzhjLWJjMGQtZjE0OGI0YzUyYjc4L3Jlc291cmNlR3JvdXBzL3BzOTEwNy9wcm92aWRlcnMvTWljcm9zb2Z0LkNvbnRhaW5lckluc3RhbmNlL2NvbnRhaW5lckdyb3Vwcy9wczI0OTI/YXBpLXZlcnNpb249MjAxOC0wMi0wMS1wcmV2aWV3",
"RequestMethod": "PUT",
"RequestBody": "{\r\n \"properties\": {\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\",\r\n \"storageAccountKey\": \"ELFwG1AfwHJbr2kC0N/k8P+kz+t/UHoYMKtQnhr5B/M2sZ+65PqSyQP+aiY9A7j0VSa6e2ZId/3r2zOL+Cb8rw==\"\r\n }\r\n }\r\n ]\r\n },\r\n \"location\": \"westus\"\r\n}",
"RequestBody": "{\r\n \"properties\": {\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"acc.azurecr.io/alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"imageRegistryCredentials\": [\r\n {\r\n \"server\": \"acc.azurecr.io\",\r\n \"username\": \"acc\",\r\n \"password\": \"password\",\r\n }\r\n ],\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\",\r\n \"storageAccountKey\": \"ELFwG1AfwHJbr2kC0N/k8P+kz+t/UHoYMKtQnhr5B/M2sZ+65PqSyQP+aiY9A7j0VSa6e2ZId/3r2zOL+Cb8rw==\"\r\n }\r\n }\r\n ]\r\n },\r\n \"location\": \"westus\"\r\n}",
"RequestHeaders": {
"Content-Type": [
"application/json; charset=utf-8"
Expand All @@ -373,7 +373,7 @@
"Microsoft.Azure.Management.ContainerInstance.ContainerInstanceManagementClient/0.0.0.0"
]
},
"ResponseBody": "{\r\n \"properties\": {\r\n \"provisioningState\": \"Creating\",\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"ports\": [],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\"\r\n }\r\n }\r\n ],\r\n \"instanceView\": {\r\n \"events\": [],\r\n \"state\": \"Pending\"\r\n }\r\n },\r\n \"id\": \"/subscriptions/ae43b1e3-c35d-4c8c-bc0d-f148b4c52b78/resourceGroups/ps9107/providers/Microsoft.ContainerInstance/containerGroups/ps2492\",\r\n \"name\": \"ps2492\",\r\n \"type\": \"Microsoft.ContainerInstance/containerGroups\",\r\n \"location\": \"westus\"\r\n}",
"ResponseBody": "{\r\n \"properties\": {\r\n \"provisioningState\": \"Creating\",\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"acc.azurecr.io/alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"ports\": [],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"imageRegistryCredentials\": [\r\n {\r\n \"server\": \"acc.azurecr.io\",\r\n \"username\": \"acc\",\r\n \"password\": \"\",\r\n }\r\n ],\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\"\r\n }\r\n }\r\n ],\r\n \"instanceView\": {\r\n \"events\": [],\r\n \"state\": \"Pending\"\r\n }\r\n },\r\n \"id\": \"/subscriptions/ae43b1e3-c35d-4c8c-bc0d-f148b4c52b78/resourceGroups/ps9107/providers/Microsoft.ContainerInstance/containerGroups/ps2492\",\r\n \"name\": \"ps2492\",\r\n \"type\": \"Microsoft.ContainerInstance/containerGroups\",\r\n \"location\": \"westus\"\r\n}",
"ResponseHeaders": {
"Content-Length": [
"726"
Expand Down Expand Up @@ -436,7 +436,7 @@
"Microsoft.Azure.Management.ContainerInstance.ContainerInstanceManagementClient/0.0.0.0"
]
},
"ResponseBody": "{\r\n \"properties\": {\r\n \"provisioningState\": \"Creating\",\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"ports\": [],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\"\r\n }\r\n }\r\n ],\r\n \"instanceView\": {\r\n \"events\": [],\r\n \"state\": \"Pending\"\r\n }\r\n },\r\n \"id\": \"/subscriptions/ae43b1e3-c35d-4c8c-bc0d-f148b4c52b78/resourceGroups/ps9107/providers/Microsoft.ContainerInstance/containerGroups/ps2492\",\r\n \"name\": \"ps2492\",\r\n \"type\": \"Microsoft.ContainerInstance/containerGroups\",\r\n \"location\": \"westus\"\r\n}",
"ResponseBody": "{\r\n \"properties\": {\r\n \"provisioningState\": \"Creating\",\r\n \"containers\": [\r\n {\r\n \"name\": \"ps2492\",\r\n \"properties\": {\r\n \"image\": \"acc.azurecr.io/alpine\",\r\n \"command\": [\r\n \"ls\",\r\n \"/mnt/azfile\"\r\n ],\r\n \"ports\": [],\r\n \"environmentVariables\": [],\r\n \"resources\": {\r\n \"requests\": {\r\n \"memoryInGB\": 1.5,\r\n \"cpu\": 1.0\r\n }\r\n },\r\n \"volumeMounts\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"mountPath\": \"/mnt/azfile\"\r\n }\r\n ]\r\n }\r\n }\r\n ],\r\n \"restartPolicy\": \"Never\",\r\n \"osType\": \"Linux\",\r\n \"imageRegistryCredentials\": [\r\n {\r\n \"server\": \"acc.azurecr.io\",\r\n \"username\": \"acc\",\r\n \"password\": \"\",\r\n }\r\n ],\r\n \"volumes\": [\r\n {\r\n \"name\": \"azurefile\",\r\n \"azureFile\": {\r\n \"shareName\": \"acipstestshare\",\r\n \"storageAccountName\": \"acipstest\"\r\n }\r\n }\r\n ],\r\n \"instanceView\": {\r\n \"events\": [],\r\n \"state\": \"Pending\"\r\n }\r\n },\r\n \"id\": \"/subscriptions/ae43b1e3-c35d-4c8c-bc0d-f148b4c52b78/resourceGroups/ps9107/providers/Microsoft.ContainerInstance/containerGroups/ps2492\",\r\n \"name\": \"ps2492\",\r\n \"type\": \"Microsoft.ContainerInstance/containerGroups\",\r\n \"location\": \"westus\"\r\n}",
"ResponseHeaders": {
"Content-Length": [
"726"
Expand Down
5 changes: 1 addition & 4 deletions ...er/ContainerInstance/Commands.ContainerInstance/Commands/NewAzureContainerGroupCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ namespace Microsoft.Azure.Commands.ContainerInstance
public class NewAzureContainerGroupCommand : ContainerInstanceCmdletBase
{
protected const string CreateContainerGroupBaseParamSet = "CreateContainerGroupBaseParamSet";
protected const string CreateContainerGroupWithRegistryParamSet = "CreateContainerGroupWithRegistryParamSet";
protected const string CreateContainerGroupWithAzureFileVolumeParamSet = "CreateContainerGroupWithAzureFileMountParamSet";

[Parameter(
Expand Down Expand Up @@ -60,8 +59,7 @@ public class NewAzureContainerGroupCommand : ContainerInstanceCmdletBase
public string Image { get; set; }

[Parameter(
Mandatory = true,
ParameterSetName = CreateContainerGroupWithRegistryParamSet,
Copy link
Contributor

@maddieclayton maddieclayton Mar 12, 2018

Choose a reason for hiding this comment

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

Can you add the third parameter set we discussed in preview comments? Looks like you only have two at the moment.

Copy link
Contributor Author

@yolocs yolocs Mar 12, 2018

Choose a reason for hiding this comment

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

I tried to but I don't think it can work. The problem is there is no unique mandatory parameter for Registry. You might think RegistryServer should be the unique mandatory one. But we actually support omitting it if the Image is from azure container registry, which means RegistryServer is not mandatory. Am I right?

Copy link
Contributor Author

@yolocs yolocs Mar 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You might also argue I can use RegistryCredential as the unique mandatory parameter. However, if that's present, there is nothing else to validate because RegistryServer is optional as described above.

Copy link
Contributor

@maddieclayton maddieclayton Mar 12, 2018

Choose a reason for hiding this comment

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

Your new parameter set would have both RegistryServer and RegistryCredential mandatory. Any case where RegistryServer is given, RegistryCredential must be also given, and this parameter set covers this case. Any other case where RegistryServer is not specified will be covered by the first parameter set (which will not contain RegistryServer and RegistryCredential will be optional).

Copy link
Contributor Author

@yolocs yolocs Mar 14, 2018

Choose a reason for hiding this comment

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

In my test, it doesn't validate Registry parameters at all. Here are the parameter sets:

SYNTAX
    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> [-RegistryCredential <pscredential>] [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB
    <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-RegistryServerDomain <string>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]
    [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -RegistryCredential <pscredential> -RegistryServerDomain <string> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}]
    [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]
    [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -AzureFileVolumeShareName <string> -AzureFileVolumeAccountCredential <pscredential> -AzureFileVolumeMountPath <string> [-RegistryCredential <pscredential>]
    [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>]
    [-RegistryServerDomain <string>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

With above parameter sets, if I have:

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryCredential $registryCredential

It won't fail while I expect it asks me to give -RegistryServer. On the other hand, if I have:

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryServer "registry.io"

It gives me runtime error while I expect a parameter set check.

Any idea?

Copy link
Contributor Author

@yolocs yolocs Mar 14, 2018

Choose a reason for hiding this comment

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

It is supported. Registry parameters can be used along with FileVolume ones.
Back to my first comment, in our case, almost every parameter can be used with other parameters. The validation needs happen in a way like: when parameter A is present, parameter B must be present too. But at the same time A & B can be used with every other parameter.

Copy link
Contributor

@maddieclayton maddieclayton Mar 14, 2018

Choose a reason for hiding this comment

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

Okay, these should be your parameter sets then:

PS1: All parameters that don't need validation (don't include FileVolume or Registry) - all parameters are optional and this is the default
PS2: All parameters that don't need validation, and the two registry parameters as required.
PS3: All parameters that don't need validation, and the FileVolume parameters as required.
PS4: All parameters that don't need validation, and both the registry and FileVolume parameters as required.

Copy link
Contributor Author

@yolocs yolocs Mar 16, 2018

Choose a reason for hiding this comment

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

OK. Confirming the following is what you suggested:

SYNTAX
    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType
    {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -RegistryServerDomain <string> -RegistryCredential <pscredential> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never |
    OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>]
    [-WhatIf] [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -RegistryServerDomain <string> -RegistryCredential <pscredential> -AzureFileVolumeMountPath <string> -AzureFileVolumeShareName <string>
    -AzureFileVolumeAccountCredential <pscredential> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port
    <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -AzureFileVolumeMountPath <string> -AzureFileVolumeShareName <string> -AzureFileVolumeAccountCredential <pscredential> [-Location <string>] [-OsType
    {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag
    <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

With these parameter sets, the following valid commands won't work as expected:

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryCredential $registryCredential 
# Parameter set cannot be resolved using the specified named parameters.

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryServerDomain "abc.io"
# Parameter set cannot be resolved using the specified named parameters.

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryCredential $registryCredential -AzureFileVolumeShareName $shareName -AzureFileVolumeAccountCredential $accountCredential -AzureFileVolumeMountPath $mountPath
# Cannot process command because of one or more missing mandatory parameters: RegistryServerDomain.

The problem is -RegistryServer is not always required. And -RegistryCredential is mandatory if and only if -RegistryServer is present. When -RegistryServer is not present, -RegistryCredential is optional. So I also tried the following parameter sets:

SYNTAX
    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> [-RegistryCredential <pscredential>] [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu
    <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf]
    [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -RegistryServerDomain <string> -RegistryCredential <pscredential> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy
    {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile
    <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -RegistryServerDomain <string> -RegistryCredential <pscredential> -AzureFileVolumeMountPath <string> -AzureFileVolumeShareName <string>
    -AzureFileVolumeAccountCredential <pscredential> [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel
    <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

    New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -AzureFileVolumeMountPath <string> -AzureFileVolumeShareName <string> -AzureFileVolumeAccountCredential <pscredential>
    [-RegistryCredential <pscredential>] [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] [-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port
    <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] [-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

It can make the above 2 commands work as expected. But there is still one command doesn't work as expected:

New-AzureRmContainerGroup -ResourceGroupName $resourceGroupName -Name $containerGroupName -Image $image -RegistryServerDomain "abc.io"
# Parameter set cannot be resolved using the specified named parameters.
# This one doesn't give the right error message, it should complain -RegistryCredential can't be null

Thoughts?

Copy link
Contributor

@maddieclayton maddieclayton Mar 19, 2018

Choose a reason for hiding this comment

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

Okay, as PowerShell can't resolve the parameter set, you should keep the validation for registryserverdomain/registrycredential in the executecmdlet statement. Please keep the individual sets for the Volume share cmdlets:

New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> [-RegistryServerDomain <string>] `
[-RegistryCredential <pscredential>] [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] `
[-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] `
[-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

New-AzureRmContainerGroup [-ResourceGroupName] <string> [-Name] <string> [-Image] <string> -AzureFileVolumeMountPath <string> `
-AzureFileVolumeShareName <string> -AzureFileVolumeAccountCredential <pscredential> [-RegistryServerDomain <string>] `
[-RegistryCredential <pscredential>] [-Location <string>] [-OsType {Linux | Windows}] [-RestartPolicy {Always | Never | OnFailure}] [-Cpu <int>] `
[-MemoryInGB <double>] [-IpAddressType {Public}] [-DnsNameLabel <string>] [-Port <int[]>] [-Command <string>] [-EnvironmentVariable <hashtable>] `
[-Tag <hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm]  [<CommonParameters>]

Copy link
Contributor Author

@yolocs yolocs Mar 19, 2018

Choose a reason for hiding this comment

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

I think that's what's already in the PR.

Mandatory = false,
HelpMessage = "The custom container registry credential.")]
[ValidateNotNullOrEmpty]
public PSCredential RegistryCredential { get; set; }
Expand Down Expand Up @@ -165,7 +163,6 @@ public class NewAzureContainerGroupCommand : ContainerInstanceCmdletBase

[Parameter(
Mandatory = false,
ParameterSetName = CreateContainerGroupWithRegistryParamSet,
HelpMessage = "The custom container registry login server.")]
[ValidateNotNullOrEmpty]
[Alias("RegistryServer")]
Expand Down
12 changes: 8 additions & 4 deletions ...r/ContainerInstance/Commands.ContainerInstance/Models/ContainerGroupCreationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,12 @@ public class ContainerGroupCreationParameters
/// </summary>
public void Validate()
{
if (string.IsNullOrEmpty(this.Location))
{
throw new ArgumentException("Please specify Location");
}
ValidateRegistryParameters();
ValidateAzureFileVolumeParameters();
}

private void ValidateRegistryParameters()
{
if (!string.IsNullOrEmpty(this.RegistryServer))
{
if (string.IsNullOrEmpty(this.RegistryUsername) || string.IsNullOrEmpty(this.RegistryPassword))
Expand All @@ -189,7 +190,10 @@ public void Validate()

this.RegistryServer = acrServer;
}
}

private void ValidateAzureFileVolumeParameters()
{
if (!string.IsNullOrWhiteSpace(this.AzureFileVolumeMountPath) && this.AzureFileVolumeMountPath.Contains(":"))
{
throw new ArgumentException("Azure File volume mount path must not contain ':'");
Expand Down
2 changes: 1 addition & 1 deletion .../ContainerInstance/Commands.ContainerInstance/help/AzureRM.ContainerInstance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
---
---
Module Name: AzureRM.ContainerInstance
Module Guid:
Download Help Link:
Expand Down
27 changes: 10 additions & 17 deletions .../ContainerInstance/Commands.ContainerInstance/help/New-AzureRmContainerGroup.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,8 @@ Creates a container group.

### CreateContainerGroupBaseParamSet (Default)
```
New-AzureRmContainerGroup [-ResourceGroupName] <String> [-Name] <String> [-Image] <String> [-Location <String>]
[-OsType <String>] [-RestartPolicy <String>] [-Cpu <Int32>] [-MemoryInGB <Double>] [-IpAddressType <String>]
[-DnsNameLabel <String>] [-Port <Int32[]>] [-Command <String>] [-EnvironmentVariable <Hashtable>]
[-Tag <Hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
```

### CreateContainerGroupWithRegistryParamSet
```
New-AzureRmContainerGroup [-ResourceGroupName] <String> [-Name] <String> [-Image] <String>
-RegistryCredential <PSCredential> [-Location <String>] [-OsType <String>] [-RestartPolicy <String>]
[-RegistryCredential <PSCredential>] [-Location <String>] [-OsType <String>] [-RestartPolicy <String>]
[-Cpu <Int32>] [-MemoryInGB <Double>] [-IpAddressType <String>] [-DnsNameLabel <String>] [-Port <Int32[]>]
[-Command <String>] [-EnvironmentVariable <Hashtable>] [-RegistryServerDomain <String>] [-Tag <Hashtable>]
[-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
Expand All @@ -32,11 +24,12 @@ New-AzureRmContainerGroup [-ResourceGroupName] <String> [-Name] <String> [-Image
### CreateContainerGroupWithAzureFileMountParamSet
```
New-AzureRmContainerGroup [-ResourceGroupName] <String> [-Name] <String> [-Image] <String>
-AzureFileVolumeShareName <String> -AzureFileVolumeAccountCredential <PSCredential>
-AzureFileVolumeMountPath <String> [-Location <String>] [-OsType <String>] [-RestartPolicy <String>]
[-Cpu <Int32>] [-MemoryInGB <Double>] [-IpAddressType <String>] [-DnsNameLabel <String>] [-Port <Int32[]>]
[-Command <String>] [-EnvironmentVariable <Hashtable>] [-Tag <Hashtable>]
[-DefaultProfile <IAzureContextContainer>] [-WhatIf] [-Confirm] [<CommonParameters>]
[-RegistryCredential <PSCredential>] -AzureFileVolumeShareName <String>
-AzureFileVolumeAccountCredential <PSCredential> -AzureFileVolumeMountPath <String> [-Location <String>]
[-OsType <String>] [-RestartPolicy <String>] [-Cpu <Int32>] [-MemoryInGB <Double>] [-IpAddressType <String>]
[-DnsNameLabel <String>] [-Port <Int32[]>] [-Command <String>] [-EnvironmentVariable <Hashtable>]
[-RegistryServerDomain <String>] [-Tag <Hashtable>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf]
[-Confirm] [<CommonParameters>]
```

## DESCRIPTION
Expand Down Expand Up @@ -432,10 +425,10 @@ The custom container registry credential.

```yaml
Type: PSCredential
Parameter Sets: CreateContainerGroupWithRegistryParamSet
Parameter Sets: (All)
Aliases:

Required: True
Required: False
Position: Named
Default value: None
Accept pipeline input: False
Expand All @@ -447,7 +440,7 @@ The custom container registry login server.

```yaml
Type: String
Parameter Sets: CreateContainerGroupWithRegistryParamSet
Parameter Sets: (All)
Aliases: RegistryServer

Required: False
Expand Down