From 4eb81f1081de8e5a4ecf5d7963796b7087a48f4b Mon Sep 17 00:00:00 2001 From: Simon Redman Date: Mon, 14 Dec 2020 15:09:47 -0800 Subject: [PATCH 1/5] Put the correct value into the StatusMessage field --- .../Model/AzureSqlDatabaseImportExportStatusModel.cs | 6 +++++- .../ImportExport/Service/ImportExportDatabaseAdapter.cs | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Sql/Sql/ImportExport/Model/AzureSqlDatabaseImportExportStatusModel.cs b/src/Sql/Sql/ImportExport/Model/AzureSqlDatabaseImportExportStatusModel.cs index 09a90563b7fa..b142f9ba98c9 100644 --- a/src/Sql/Sql/ImportExport/Model/AzureSqlDatabaseImportExportStatusModel.cs +++ b/src/Sql/Sql/ImportExport/Model/AzureSqlDatabaseImportExportStatusModel.cs @@ -67,6 +67,10 @@ public string RequestType /// /// Gets or sets the status message returned from the server. + /// + /// Ensure that this retains compatibility with the old Powershell versions since lots of customers use this for + /// their automation. + /// Compare to /// public string Status { @@ -84,7 +88,7 @@ public string StatusMessage } /// - /// Gets or sets the + /// Gets or sets the private endpoint status(es) /// public PrivateEndpointRequestStatus[] PrivateEndpointRequestStatus { diff --git a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs index 3b7b3e7e8005..4d185e830290 100644 --- a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs +++ b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs @@ -145,7 +145,7 @@ public AzureSqlDatabaseImportExportStatusModel GetStatus(string operationStatusL ErrorMessage = response.ErrorMessage, LastModifiedTime = response.LastModifiedTime, QueuedTime = response.QueuedTime, - Status = response.Status, + StatusMessage = response.Status, // in spite of the name, the field called "Status" is the correct one to put into the "StatusMessage" field RequestType = response.RequestType, PrivateEndpointRequestStatus = response.PrivateEndpointConnections?.Select(pec => new PrivateEndpointRequestStatus() { @@ -168,6 +168,12 @@ private AzureSqlDatabaseImportExportBaseModel CreateImportExportResponse(ImportE { AzureSqlDatabaseImportExportBaseModel model = originalModel == null ? new AzureSqlDatabaseImportExportBaseModel() : originalModel.Copy(); model.OperationStatusLink = statusLink?.ToString(); + + // It looks like the ExportDatabase SDK call is currently broken (and returns "null" instead of the response object). + // I need to check in a sev2 hotfix now. Once the SDK issue has been resolved, un-comment this and the asserts in + // the test code + // Also can probably remove the "LastLocationHeader" hack and just rely on the header in the returned results + // model.Status = response.Status; return model; } } From 13052aefc10fda9d91722014104cb6d8a76dbe35 Mon Sep 17 00:00:00 2001 From: Simon Redman Date: Mon, 14 Dec 2020 17:34:49 -0800 Subject: [PATCH 2/5] Add parsing of the http return value to determine the operation status --- .../Service/ImportExportDatabaseAdapter.cs | 34 ++++++++++++++++++- .../ImportExportDatabaseCommunicator.cs | 27 ++++++++------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs index 4d185e830290..af2323970764 100644 --- a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs +++ b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseAdapter.cs @@ -19,6 +19,8 @@ using Microsoft.Azure.Management.Sql.Models; using System; using System.Linq; +using System.Net; +using System.Net.Http; namespace Microsoft.Azure.Commands.Sql.ImportExport.Service { @@ -138,7 +140,10 @@ public AzureSqlDatabaseImportExportBaseModel ImportNewDatabase(AzureSqlDatabaseI /// Operation status response public AzureSqlDatabaseImportExportStatusModel GetStatus(string operationStatusLink) { - ImportExportOperationResult response = Communicator.GetOperationStatus(operationStatusLink); + HttpResponseMessage rawHttpResponse; + ImportExportOperationResult response = Communicator.GetOperationStatus(operationStatusLink, out rawHttpResponse); + + OperationStatus? operationStatus = GetOperationStatusFromHttpStatus(rawHttpResponse.StatusCode); AzureSqlDatabaseImportExportStatusModel status = new AzureSqlDatabaseImportExportStatusModel() { @@ -146,6 +151,7 @@ public AzureSqlDatabaseImportExportStatusModel GetStatus(string operationStatusL LastModifiedTime = response.LastModifiedTime, QueuedTime = response.QueuedTime, StatusMessage = response.Status, // in spite of the name, the field called "Status" is the correct one to put into the "StatusMessage" field + Status = operationStatus.HasValue ? operationStatus.Value.ToString() : "", RequestType = response.RequestType, PrivateEndpointRequestStatus = response.PrivateEndpointConnections?.Select(pec => new PrivateEndpointRequestStatus() { @@ -176,5 +182,31 @@ private AzureSqlDatabaseImportExportBaseModel CreateImportExportResponse(ImportE // model.Status = response.Status; return model; } + + /// + /// Get the "Status" value for the Import/Export request + /// + /// This logic is copied verbatim from the (generated) legacy SDK: + /// + /// + /// + /// + protected OperationStatus? GetOperationStatusFromHttpStatus(HttpStatusCode statusCode) + { + OperationStatus? status = null; + switch (statusCode) + { + // We expect this switch statement to cover all possible cases of return values from the service + case HttpStatusCode.OK: + case HttpStatusCode.Created: + status = OperationStatus.Succeeded; + break; + case HttpStatusCode.Accepted: + status = OperationStatus.InProgress; + break; + } + + return status; + } } } diff --git a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseCommunicator.cs b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseCommunicator.cs index 86a400da4a86..842f7acb2b31 100644 --- a/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseCommunicator.cs +++ b/src/Sql/Sql/ImportExport/Service/ImportExportDatabaseCommunicator.cs @@ -12,24 +12,20 @@ // limitations under the License. // ---------------------------------------------------------------------------------- -using Microsoft.Azure.Commands.Common.Authentication; -using Microsoft.Azure.Commands.Common.Authentication.Abstractions; -using Microsoft.Azure.Commands.Common.Authentication.Models; -using Microsoft.Azure.Commands.Sql.Common; -using Microsoft.Azure.Management.Sql; -using Microsoft.Azure.Management.Sql.Models; -using Microsoft.Rest.Azure; -using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; using System; using System.Collections.Generic; -using System.Linq; using System.Net.Http; using System.Net.Http.Headers; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using Microsoft.Azure.Commands.Common.Authentication; +using Microsoft.Azure.Commands.Common.Authentication.Abstractions; +using Microsoft.Azure.Management.Sql; +using Microsoft.Azure.Management.Sql.Models; + +using Newtonsoft.Json; + namespace Microsoft.Azure.Commands.Sql.Database.Services { /// @@ -114,7 +110,13 @@ public ImportExportOperationResult BeginImportNewDatabase(string resourceGroupNa return result; } - public ImportExportOperationResult GetOperationStatus(string operationStatusLink) + /// + /// Get the status of an operation given a raw Operation Status Link + /// + /// Status link as returned by the import or export commandlet + /// Out parameter for the raw HTTP response for further inspection + /// + public ImportExportOperationResult GetOperationStatus(string operationStatusLink, out HttpResponseMessage rawHttpResponse) { var client = GetCurrentSqlClient(); @@ -129,6 +131,7 @@ public ImportExportOperationResult GetOperationStatus(string operationStatusLink response.EnsureSuccessStatusCode(); + rawHttpResponse = response; string responseString = response.Content.ReadAsStringAsync().Result; ImportExportOperationResult operationResult = JsonConvert.DeserializeObject(responseString, new JsonSerializerSettings From 516c8795b033171740cf7b168f07e7a3c2b94e64 Mon Sep 17 00:00:00 2001 From: Simon Redman Date: Tue, 15 Dec 2020 15:57:53 -0800 Subject: [PATCH 3/5] Add ChangeLog.md entry --- src/Sql/Sql/ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Sql/Sql/ChangeLog.md b/src/Sql/Sql/ChangeLog.md index 71c6ad9ca37b..487facc90fff 100644 --- a/src/Sql/Sql/ChangeLog.md +++ b/src/Sql/Sql/ChangeLog.md @@ -19,6 +19,7 @@ --> ## Upcoming Release * Fixed parameter description for `InstanceFailoverGroup` command. +* Fix Status and StatusMessage fields in Get-AzSqlDatabaseImportExportStatus to conform to documentation ## Version 2.13.0 * Added SecondaryType to the following: From c16701ecfe4df34fa65fd6d4581bcb7ca1a7eb54 Mon Sep 17 00:00:00 2001 From: Simon Redman Date: Wed, 16 Dec 2020 16:27:34 -0800 Subject: [PATCH 4/5] Re-add as much testing as possible and add commenting for why the rest is broken --- .../ScenarioTests/ImportExportTests.ps1 | 106 ++++++++++++------ 1 file changed, 69 insertions(+), 37 deletions(-) diff --git a/src/Sql/Sql.Test/ScenarioTests/ImportExportTests.ps1 b/src/Sql/Sql.Test/ScenarioTests/ImportExportTests.ps1 index d5ef0b67489a..77d483ec3139 100644 --- a/src/Sql/Sql.Test/ScenarioTests/ImportExportTests.ps1 +++ b/src/Sql/Sql.Test/ScenarioTests/ImportExportTests.ps1 @@ -19,7 +19,7 @@ #> function Test-ExportDatabase { - # Setup + # Setup $testSuffix = 90070 $createServer = $true $createDatabase = $true @@ -27,7 +27,7 @@ function Test-ExportDatabase $operationName = "Export" $succeeded = $true $useNetworkIsolation = $false - + Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation } @@ -41,13 +41,13 @@ function Test-ExportDatabaseNetworkIsolation $operationName = "Export" $succeeded = $true $useNetworkIsolation = $true - + Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation } function Test-ImportNewDatabase { - # Setup + # Setup $testSuffix = 90071 $createServer = $true $createDatabase = $false @@ -56,7 +56,7 @@ function Test-ImportNewDatabase $succeeded = $true $useNetworkIsolation = $false - Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation + Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation } function Test-ImportNewDatabaseNetworkIsolation @@ -70,39 +70,39 @@ function Test-ImportNewDatabaseNetworkIsolation $succeeded = $true $useNetworkIsolation = $true - Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation + Verify-ImportExport $testSuffix $createServer $createDatabase $createFirewallRule $operationName $succeeded $useNetworkIsolation } function Verify-ImportExport($testSuffix, $createServer, $createDatabase, $createFirewallRule, $operationName, $succeeded, $useNetworkIsolation) { - # Setup + # Setup $params = Get-SqlDatabaseImportExportTestEnvironmentParameters $testSuffix $rg = New-AzResourceGroup -Name $params.rgname -Location $params.location $export = "Export" $importNew = "ImportNew" - try - { + try + { Assert-NotNull $params.storageKey Assert-NotNull $params.importBacpacUri Assert-NotNull $params.exportBacpacUri Assert-NotNull $params.storageResourceId $password = $params.password - - $secureString = ($password | ConvertTo-SecureString -asPlainText -Force) + + $secureString = ($password | ConvertTo-SecureString -asPlainText -Force) $credentials = new-object System.Management.Automation.PSCredential($params.userName, $secureString) $rgname = $params.rgname $serverName = $params.serverName if($createServer -eq $true){ - $server = New-AzSqlServer -ResourceGroupName $rgname -ServerName $serverName -ServerVersion $params.version -Location $params.location -SqlAdministratorCredentials $credentials + $server = New-AzSqlServer -ResourceGroupName $rgname -ServerName $serverName -ServerVersion $params.version -Location $params.location -SqlAdministratorCredentials $credentials } if($createDatabase -eq $true){ $standarddb = New-AzSqlDatabase -ResourceGroupName $rgname -ServerName $serverName -DatabaseName $params.databaseName } - + if($createFirewallRule -eq $true){ New-AzSqlServerFirewallRule -ResourceGroupName $rgname -ServerName $serverName -AllowAllAzureIPs } @@ -127,16 +127,16 @@ function Test-ImportNewDatabaseNetworkIsolation Write-Output "Assert-NotNull exportResponse" Assert-NotNull $exportResponse Write-Output (ConvertTo-Json $exportResponse) - #$operationStatusLink = $exportResponse.OperationStatusLink - #Assert-AreEqual $exportResponse.ResourceGroupName $params.rgname - #Assert-AreEqual $exportResponse.ServerName $params.serverName - #Assert-AreEqual $exportResponse.DatabaseName $params.databaseName - #Assert-AreEqual $exportResponse.StorageKeyType $params.storageKeyType - #Assert-Null $exportResponse.StorageKey - #Assert-AreEqual $exportResponse.StorageUri $params.exportBacpacUri - #Assert-AreEqual $exportResponse.AdministratorLogin $params.userName - #Assert-Null $exportResponse.AdministratorLoginPassword - #Assert-AreEqual $exportResponse.AuthenticationType $params.authType + $operationStatusLink = $exportResponse.OperationStatusLink + Assert-AreEqual $exportResponse.ResourceGroupName $params.rgname + Assert-AreEqual $exportResponse.ServerName $params.serverName + Assert-AreEqual $exportResponse.DatabaseName $params.databaseName + Assert-AreEqual $exportResponse.StorageKeyType $params.storageKeyType + Assert-Null $exportResponse.StorageKey + Assert-AreEqual $exportResponse.StorageUri $params.exportBacpacUri + Assert-AreEqual $exportResponse.AdministratorLogin $params.userName + Assert-Null $exportResponse.AdministratorLoginPassword + Assert-AreEqual $exportResponse.AuthenticationType $params.authType } if($operationName -eq $importNew){ @@ -146,26 +146,58 @@ function Test-ImportNewDatabaseNetworkIsolation } else { - $importResponse = New-AzSqlDatabaseImport -ResourceGroupName $params.rgname -ServerName $params.serverName -DatabaseName $params.databaseName -StorageKeyType $params.storageKeyType -StorageKey $params.storageKey -StorageUri $params.importBacpacUri -AdministratorLogin $params.userName -AdministratorLoginPassword $secureString -Edition $params.databaseEdition -ServiceObjectiveName $params.serviceObjectiveName -DatabaseMaxSizeBytes $params.databaseMaxSizeBytes -AuthenticationType $params.authType + $importResponse = New-AzSqlDatabaseImport -ResourceGroupName $params.rgname -ServerName $params.serverName -DatabaseName $params.databaseName -StorageKeyType $params.storageKeyType -StorageKey $params.storageKey -StorageUri $params.importBacpacUri -AdministratorLogin $params.userName -AdministratorLoginPassword $secureString -Edition $params.databaseEdition -ServiceObjectiveName $params.serviceObjectiveName -DatabaseMaxSizeBytes $params.databaseMaxSizeBytes -AuthenticationType $params.authType } Write-Output "Assert-NotNull importResponse" Assert-NotNull $importResponse Write-Output (ConvertTo-Json $importResponse) - #$operationStatusLink = $importResponse.OperationStatusLink - #Assert-AreEqual $importResponse.ResourceGroupName $params.rgname - #Assert-AreEqual $importResponse.ServerName $params.serverName - #Assert-AreEqual $importResponse.DatabaseName $params.databaseName - #Assert-AreEqual $importResponse.StorageKeyType $params.storageKeyType - #Assert-Null $importResponse.StorageKey - #Assert-AreEqual $importResponse.StorageUri $params.importBacpacUri - #Assert-AreEqual $importResponse.AdministratorLogin $params.userName - #Assert-Null $importResponse.AdministratorLoginPassword - #Assert-AreEqual $importResponse.AuthenticationType $params.authType - #Assert-AreEqual $importResponse.Edition $params.databaseEdition - #Assert-AreEqual $importResponse.ServiceObjectiveName $params.serviceObjectiveName - #Assert-AreEqual $importResponse.DatabaseMaxSizeBytes $params.databaseMaxSizeBytes + $operationStatusLink = $importResponse.OperationStatusLink + Assert-AreEqual $importResponse.ResourceGroupName $params.rgname + Assert-AreEqual $importResponse.ServerName $params.serverName + Assert-AreEqual $importResponse.DatabaseName $params.databaseName + Assert-AreEqual $importResponse.StorageKeyType $params.storageKeyType + Assert-Null $importResponse.StorageKey + Assert-AreEqual $importResponse.StorageUri $params.importBacpacUri + Assert-AreEqual $importResponse.AdministratorLogin $params.userName + Assert-Null $importResponse.AdministratorLoginPassword + Assert-AreEqual $importResponse.AuthenticationType $params.authType + Assert-AreEqual $importResponse.Edition $params.databaseEdition + Assert-AreEqual $importResponse.ServiceObjectiveName $params.serviceObjectiveName + Assert-AreEqual $importResponse.DatabaseMaxSizeBytes $params.databaseMaxSizeBytes } + + # The following part of the test is broken for now because $operationStatusLink is always null + # this does not reproduce when I run the commands manually, so I suspect a race condition in the + # way the v2020-02-02 New-Import and New-Export commandlets fetch the Location header + # That handling is necessary because the v2020-02-02 SDK is itself returning null instead of the + # promised AzureSqlDatabaseImportExportModel + # I am trying to solve an outage now, so leaving this test commented out, but I will create a work + # item to fix the SDK + <# TODO: Uncomment once the location header is returning correctly + Assert-NotNull $operationStatusLink + + #Get status + $statusInProgress = "InProgress" + $statusSucceeded = "Succeeded" + $status = "InProgress" + + if($succeeded -eq $true){ + Write-Output "Getting Status" + while($status -eq $statusInProgress){ + $statusResponse = Get-AzSqlDatabaseImportExportStatus -OperationStatusLink $operationStatusLink + Write-Output "Import Export Status Message:" + $statusResponse.StatusMessage + Assert-AreEqual $statusResponse.OperationStatusLink $operationStatusLink + $status = $statusResponse.Status + if($status -eq $statusInProgress){ + Assert-NotNull $statusResponse.LastModifiedTime + Assert-NotNull $statusResponse.QueuedTime + Assert-NotNull $statusResponse.StatusMessage + } + } + Assert-AreEqual $status $statusSucceeded + Write-Output "ImportExportStatus:" + $status + #> } finally { From b526ba3292407f56f9c17af19c5e40471a3d3d69 Mon Sep 17 00:00:00 2001 From: Dingmeng Xue Date: Thu, 17 Dec 2020 09:09:37 +0800 Subject: [PATCH 5/5] Update ChangeLog.md --- src/Sql/Sql/ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sql/Sql/ChangeLog.md b/src/Sql/Sql/ChangeLog.md index 487facc90fff..479e453a8c54 100644 --- a/src/Sql/Sql/ChangeLog.md +++ b/src/Sql/Sql/ChangeLog.md @@ -19,7 +19,7 @@ --> ## Upcoming Release * Fixed parameter description for `InstanceFailoverGroup` command. -* Fix Status and StatusMessage fields in Get-AzSqlDatabaseImportExportStatus to conform to documentation +* Fixed Status and StatusMessage fields in `Get-AzSqlDatabaseImportExportStatus` to conform to documentation ## Version 2.13.0 * Added SecondaryType to the following: