microsoft / azure-pipelines-extensions

Collection of all RM and deployment extensions
http://www.visualstudio.com/explore/release-management-vs
MIT License
275 stars 425 forks source link

Issue 1225 #1226

Open ErikEJ opened 3 months ago

ErikEJ commented 3 months ago

fixes #1225

Description: Look for SqlPackage.exe presence and version in the Visual Studio DAC folder introduced in later VS versions

Documentation changes required: N

Added unit tests: N

Attached related issue: #1225

Checklist:

Verified with

. "$PSScriptRoot/SqlPackageOnTargetMachines.ps1"
Get-SqlPackageOnTargetMachine
ErikEJ commented 3 months ago

@dzsquared FYI! Please have a look 😄

b3go commented 3 months ago

Tested and works for 2019 and 2022. Output:

PS C:\WINDOWS\system32> Get-SqlPackageOnTargetMachine
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\150\SqlPackage.exe
PS C:\WINDOWS\system32> Get-SqlPackageOnTargetMachine
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\SqlPackage.exe
ErikEJ commented 3 months ago

@b3go Thanks for confirming!

ErikEJ commented 3 months ago

@dzsquared wonder if this is enough to also affect the azuresqldeployment task?

ErikEJ commented 3 months ago

Looks like there is duplicate code in another repo: https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks%2FSqlAzureDacpacDeploymentV1%2FFindSqlPackagePath.ps1#L348-L352

dzsquared commented 3 months ago

Often the version from the dac.msi installer (C:\Program Files\Microsoft SQL Server\160\DAC\bin) will be newer than the VS version, causing it to be selected over the VS version - probably in part why we haven't seen this behavior crop up much.

Just fyi - the version in azure-pipelines-tasks has another PR open on it to enable more versatility - https://github.com/microsoft/azure-pipelines-tasks/pull/19648

Finally, the dotnet tool version of Sqlpackage was mentioned by @b3go and indeed we're hoping to integrate it with these tasks in the future - but this change doesn't need to be held up for that larger adjustment.

thank you @ErikEJ!

ErikEJ commented 3 months ago

probably in part why we haven't seen this behavior crop up much.

Ah, that's why there have been no issues from hosted agents.

PaulVrugt commented 3 months ago

Looks like there is duplicate code in another repo: https://github.com/microsoft/azure-pipelines-tasks/blob/master/Tasks%2FSqlAzureDacpacDeploymentV1%2FFindSqlPackagePath.ps1#L348-L352

I'm a bit confused. We are using the SqlAzureDacpacDeployment@1 task, and it seems to take the C:\Program Files\Microsoft SQL Server\160\DAC\bin sqlpackage version over the visual studio one, even if the visual studio one is newer. Which caused https://github.com/microsoft/DacFx/issues/427 for us. It seems that this PR doesn't fix this issue. But is there an active PR somewhere to fix the SqlAzureDacpacDeployment@1 as well?

ErikEJ commented 3 months ago

@PaulVrugt Are you still affected by this - I think the version in C:\Program Files\Microsoft SQL Server\160\DAC\bin has been updated on all the Hosted agents now. But I still plan to do a PR in the other repo (to at least align the logic)

ErikEJ commented 3 months ago

@dzsquared @PaulVrugt Gave up on the other task, too many moving parts

PaulVrugt commented 3 months ago

@PaulVrugt Are you still affected by this - I think the version in C:\Program Files\Microsoft SQL Server\160\DAC\bin has been updated on all the Hosted agents now. But I still plan to do a PR in the other repo (to at least align the logic)

Well yes, because we are not using hosted agents, we use private agents. Once I updated visual studio on the private agent image, the error started appearing. We now fixed it by updating sqlpackage on the private agent manually, but this would have been prevented if the task worked properly and found the sqlpackage of visual studio (which was updated with the visual studio update)

ErikEJ commented 3 months ago

@PaulVrugt Got it!

I encountered multiple issues trying to fix the other task:

PaulVrugt commented 3 months ago

@PaulVrugt Got it!

I encountered multiple issues trying to fix the other task:

Lol. Ok so in summary:

Well, thanks at least for trying. We'll create our own safeguards to keep sqlpackage.exe up to date at the location SqlAzureDacpacDeploymentV1 IS finding it