[HDInsight] Add three parameters to support relay outbound and private link and custom Ambari DB #13413

aim-for-better · 2020-11-04T03:51:22Z

Description

For the cmdlet New-AzHDInsightCluster

Add two parameters ResourceProviderConnection and PrivateLink to support Relay and Private Link
Add one parameters AmbariDatabase to support custom Ambari DB

For the cmdlet Add-AzHDInsightMetastore:

Add the accept value "AmbariDatabase" for the paramter -MetastoreType

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate ChangeLog.md file(s) has been updated:
- For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
- A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
- the markdown help files have been regenerated using the commands listed here

aim-for-better · 2020-11-04T05:50:50Z

The cmdlet Restart-AzHDInsightHost doesn't use the property NetworkSettings, but I still got the static analysis error,
I think this is false alarm, because our code doesn't use this property. So I have to suppress this.

"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
"Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll","Microsoft.Azure.Commands.HDInsight.RestartAzureHDInsightHostCommand","Restart-AzHDInsightHost","0","3010","The property 'NetworkSettings' of type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties' has been removed.","Add the property 'NetworkSettings' back to type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties'."

aim-for-better · 2020-11-05T08:46:47Z

Hi @isra-fel Could you please help review the PR? Thank you very much~

src/HDInsight/HDInsight/help/New-AzHDInsightCluster.md

isra-fel · 2020-11-06T01:56:52Z

The cmdlet Restart-AzHDInsightHost doesn't use the property NetworkSettings, but I still got the static analysis error,
I think this is false alarm, because our code doesn't use this property. So I have to suppress this.

"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
"Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll","Microsoft.Azure.Commands.HDInsight.RestartAzureHDInsightHostCommand","Restart-AzHDInsightHost","0","3010","The property 'NetworkSettings' of type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties' has been removed.","Add the property 'NetworkSettings' back to type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties'."

I understand what you mean. I think the root cause is the OutputType of Restart-AzHDInsightHost was set to Cluster (see https://github.com/Azure/azure-powershell/blob/master/src/HDInsight/HDInsight/ManagementCommands/RestartAzureHDInsightHost.cs#L28 ) but it actually returns a bool. Our static analysis tool was based on OutputType so it thought that type has changed.

So I think the right thing to do is to fix the output type. Could you help fix it in this PR? Thanks

aim-for-better · 2020-11-06T03:10:50Z

has

The cmdlet Restart-AzHDInsightHost doesn't use the property NetworkSettings, but I still got the static analysis error,
I think this is false alarm, because our code doesn't use this property. So I have to suppress this.
"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
"Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll","Microsoft.Azure.Commands.HDInsight.RestartAzureHDInsightHostCommand","Restart-AzHDInsightHost","0","3010","The property 'NetworkSettings' of type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties' has been removed.","Add the property 'NetworkSettings' back to type 'Microsoft.Azure.Management.HDInsight.Models.ClusterGetProperties'."

I understand what you mean. I think the root cause is the OutputType of Restart-AzHDInsightHost was set to Cluster (see https://github.com/Azure/azure-powershell/blob/master/src/HDInsight/HDInsight/ManagementCommands/RestartAzureHDInsightHost.cs#L28 ) but it actually returns a bool. Our static analysis tool was based on OutputType so it thought that type has changed.

So I think the right thing to do is to fix the output type. Could you help fix it in this PR? Thanks

Hi @isra-fel , Got it, Thanks, I will fix the output type.

isra-fel · 2020-11-06T03:25:52Z

A reminder: now that you will get a static analysis warning about changing the output type of Restart-AzHDInsightHost which needs to be suppressed.

… exception

aim-for-better · 2020-11-06T06:40:16Z

A reminder: now that you will get a static analysis warning about changing the output type of Restart-AzHDInsightHost which needs to be suppressed.

Hi @isra-fel , Thanks for reminder, in fact I got two static analysis error, besides the error you mentioned
there is a new NetworkSettings property has been removed issue for Set-AzHDInsightClusterDiskEncryptionKey 😂,
I suppress both of them.
Please see the previous static analysis error
https://dev.azure.com/azure-sdk/public/_build/results?buildId=608528&view=logs&jobId=99d77eec-fdb1-5e11-3259-110c26331ac6&j=99d77eec-fdb1-5e11-3259-110c26331ac6&t=d24a834f-31ee-5ffa-ba54-b348410dd76a

isra-fel

LGTM! Thanks

isra-fel · 2020-11-09T01:51:20Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2020-11-09T01:51:30Z

Azure Pipelines successfully started running 1 pipeline(s).

aim-for-better · 2020-11-09T03:02:38Z

/azp run azure-powershell - powershell-core

Hi @isra-fel Thanks

isra-fel · 2020-11-09T03:22:59Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2020-11-09T03:23:09Z

Azure Pipelines successfully started running 1 pipeline(s).

aim-for-better · 2020-11-09T05:15:13Z

Hi @isra-fel , The CI still failed. The error is not related with specific service, I think maybe it is common issue. Please see https://dev.azure.com/azure-sdk/public/_build/results?buildId=610796&view=logs&j=ad07b744-5a1a-5d54-7693-fcb02e666633&t=b0841cae-a6b1-5a5c-4ebd-3cc6b3d3163d

aim-for-better · 2020-11-09T05:15:25Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2020-11-09T05:15:35Z

Azure Pipelines successfully started running 1 pipeline(s).

aim-for-better · 2020-11-09T05:16:24Z

/azp run azure-powershell - powershell-core (Test linux)

azure-pipelines · 2020-11-09T05:16:29Z

No pipelines are associated with this pull request.

aim-for-better · 2020-11-09T06:27:43Z

PowerShell core phase still failed

aim-for-better · 2020-11-09T07:40:44Z

The CI failed for ##[error]Artifact test-linux already exists for build 610796 more than one time. So I just generate a new commit id to rerun CI totally.

isra-fel · 2020-11-09T09:10:56Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2020-11-09T09:11:07Z

Azure Pipelines successfully started running 1 pipeline(s).

aim-for-better changed the title ~~[HDInsight] Support Relay Outbound and Private Link~~ [HDInsight] Add two parameters to support relay outbound and private link Nov 4, 2020

azuresdkci added the needs-review label Nov 4, 2020

Support Relay Outbound and Private Link

d08ff19

aim-for-better force-pushed the SupportRelayOutboundAndPrivateLink branch from 741e7a7 to d08ff19 Compare November 4, 2020 05:41

aim-for-better added this to the S178 (2020-11-17) milestone Nov 4, 2020

aim-for-better requested review from dingmeng-xue, isra-fel and wyunchi-ms and removed request for dingmeng-xue November 4, 2020 05:57

isra-fel self-assigned this Nov 4, 2020

aim-for-better requested review from dingmeng-xue and MiYanni and removed request for dingmeng-xue and MiYanni November 4, 2020 11:36

aim-for-better changed the title ~~[HDInsight] Add two parameters to support relay outbound and private link~~ [HDInsight] Add three parameters to support relay outbound and private link and custom Ambari DB Nov 5, 2020

Support Custom Ambari DB as metastore

d20e5ac

aim-for-better force-pushed the SupportRelayOutboundAndPrivateLink branch from 0396b9d to d20e5ac Compare November 5, 2020 06:30

isra-fel reviewed Nov 5, 2020

View reviewed changes

src/HDInsight/HDInsight/help/New-AzHDInsightCluster.md Show resolved Hide resolved

isra-fel added needs-revision and removed needs-review labels Nov 6, 2020

isra-fel assigned aim-for-better Nov 6, 2020

aim-for-better requested a review from isra-fel November 6, 2020 03:13

Fix output type of Restar-AzHDInsightHosts and remove breaking change…

82034ab

… exception

aim-for-better force-pushed the SupportRelayOutboundAndPrivateLink branch from f2b1e14 to 82034ab Compare November 6, 2020 05:27

isra-fel previously approved these changes Nov 6, 2020

View reviewed changes

isra-fel added Waiting for CI and removed needs-revision labels Nov 6, 2020

isra-fel unassigned aim-for-better Nov 6, 2020

Fix output issue of Restart-AzHDInsightHosts and update help doc

77e4232

aim-for-better dismissed isra-fel’s stale review via 77e4232 November 9, 2020 07:38

aim-for-better force-pushed the SupportRelayOutboundAndPrivateLink branch from a5c3828 to 77e4232 Compare November 9, 2020 07:38

aim-for-better requested a review from isra-fel November 9, 2020 07:39

isra-fel approved these changes Nov 10, 2020

View reviewed changes

isra-fel merged commit 6cf9281 into Azure:master Nov 10, 2020

[HDInsight] Add three parameters to support relay outbound and private link and custom Ambari DB #13413

[HDInsight] Add three parameters to support relay outbound and private link and custom Ambari DB #13413

Uh oh!

Conversation

aim-for-better commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

aim-for-better commented Nov 4, 2020

Uh oh!

aim-for-better commented Nov 5, 2020

Uh oh!

Uh oh!

isra-fel commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aim-for-better commented Nov 6, 2020

Uh oh!

isra-fel commented Nov 6, 2020

Uh oh!

aim-for-better commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isra-fel left a comment

Choose a reason for hiding this comment

Uh oh!

isra-fel commented Nov 9, 2020

Uh oh!

azure-pipelines bot commented Nov 9, 2020

Uh oh!

aim-for-better commented Nov 9, 2020

Uh oh!

isra-fel commented Nov 9, 2020

Uh oh!

azure-pipelines bot commented Nov 9, 2020

Uh oh!

aim-for-better commented Nov 9, 2020

Uh oh!

aim-for-better commented Nov 9, 2020

Uh oh!

azure-pipelines bot commented Nov 9, 2020

Uh oh!

aim-for-better commented Nov 9, 2020

Uh oh!

azure-pipelines bot commented Nov 9, 2020

Uh oh!

aim-for-better commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aim-for-better commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isra-fel commented Nov 9, 2020

Uh oh!

azure-pipelines bot commented Nov 9, 2020

Uh oh!

Uh oh!

aim-for-better commented Nov 4, 2020 •

edited

Loading

isra-fel commented Nov 6, 2020 •

edited

Loading

aim-for-better commented Nov 6, 2020 •

edited

Loading

aim-for-better commented Nov 9, 2020 •

edited

Loading

aim-for-better commented Nov 9, 2020 •

edited

Loading