Skip to content
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

Bug fix for pipeline downloading incorrect package ver #20294

Merged
merged 10 commits into from
Aug 20, 2021
Merged

Bug fix for pipeline downloading incorrect package ver #20294

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
102522e
Bug fix: pipeline to download correct package ver
ckairen Aug 12, 2021
e13583d
Update clean version
ckairen Aug 17, 2021
578c84a
Update clean version
ckairen Aug 17, 2021
d7d0a63
Cleanup
ckairen Aug 18, 2021
2cdb687
Cleanup
ckairen Aug 18, 2021
4f11a9f
Cleanup
ckairen Aug 18, 2021
7fcd66b
Cleanup
ckairen Aug 18, 2021
c8d7d4b
Comments added
ckairen Aug 19, 2021
9755ae1
Testing
ckairen Aug 20, 2021
45df900
Final-Version
ckairen Aug 20, 2021
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
36 changes: 22 additions & 14 deletions eng/pipelines/templates/jobs/smoke.tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,15 @@ jobs:
timeoutInMinutes: 5

- pwsh: |
ckairen marked this conversation as resolved.
Show resolved Hide resolved
$packages = Get-ChildItem "$(Pipeline.Workspace)/${{ parameters.ArtifactName }}/*.zip"
$packages = Get-ChildItem "$(Pipeline.Workspace)/${{ parameters.ArtifactName }}/${{ parameters.Artifact.name }}/*.zip"
Write-Host "Artifacts found:"
Copy link
Member

Choose a reason for hiding this comment

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

We should still try to add some sort of validation to make sure we found the packages we were expecting.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this essentially just testing that our release build scripts work and build the right thing? I think that might be an environment assumption we can make and let the build scripts be responsible for validating that? Or am I misunderstanding.

Copy link
Member

Choose a reason for hiding this comment

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

I just want to make sure we don't accidently slip through this step and not update any package references in the requirements and then everything works as expected but we didn't actually test anything.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Do you happen to know of a clear reference to the current version being released? It seems like we may have to export a variable as part of our version update scripts the way we did for java?

Copy link
Member

Choose a reason for hiding this comment

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

I think for our purposes here the goal is to test the artifact we have just built so we can continue to parse the version from the artifact if we want (or we can switch to getting that from the PackageInfo.json) and then we just need to check to make sure what we passed in via the parameter.Artifact matches what we found and are updating.

Copy link
Member

Choose a reason for hiding this comment

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

@ckairen @scbedd and I are still a little confused as to what's being proposed here. Within the release pipeline context, based on previous steps and the actions of the release engineer, the build artifact and version in it is a source of truth for the version. So if we wanted to check that we were pulling in the right versioned artifact, wouldn't we be essentially be checking the same source of version data against itself?

Since you're out until next week, I think we should merge what's here to fix the immediate issue, but keep #19928 open so we can discuss/clarify when you get back.

Copy link
Member

Choose a reason for hiding this comment

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

I reopened #19928 and added some more context there.

$artifacts = $packages | ForEach-Object {
if ($_.Name -match "([a-zA-Z\-]+)\-(.*).zip") {
Write-Host "$($matches[1]): $($matches[2])"
return @{ "name" = $matches[1]; "version" = $matches[2] }
}
}
$dependencies = Get-Content $env:REQUIREMENTS | ForEach-Object {
$dependencies = Get-Content $(requirements) | ForEach-Object {
$line = $_
if ($line -match "([a-zA-Z\-]+)(\W+)(.*)") {
$override = ($artifacts | Where-Object { $_.Name -eq $matches[1] }).Version
Expand All @@ -179,36 +179,44 @@ jobs:
return $line
}

$dependencies | Out-File $env:REQUIREMENTS
$dependencies | Out-File $(requirements)

displayName: Override requirements with pipeline build artifact versions
env:
REQUIREMENTS: $(requirements)

- pwsh: pip install -r "$env:REQUIREMENTS" --no-deps --upgrade
# Retry for pip install due to delay in package availability after publish
# The package is expected to be available for download/installation within 10 minutes
- pwsh: |
$ErrorActionPreference = "Continue"
ckairen marked this conversation as resolved.
Show resolved Hide resolved
while ($retries++ -lt 15) {
Write-Host "pip install -r $(requirements) --no-deps --upgrade --no-cache-dir"
pip install -r "$(requirements)" --no-deps --upgrade --no-cache-dir
if ($LASTEXITCODE) {
if ($retries -ge 15) {
exit $LASTEXITCODE
}
Write-Host "Installation failed, retrying in 1 minute..."
sleep 60
} else {
break
}
}
displayName: Install requirements without dependencies
env:
REQUIREMENTS: $(requirements)

- ${{ if eq(parameters.Daily, true) }}:
- pwsh: |
pip install -r "$env:REQUIREMENTS" --pre --no-deps --upgrade `
pip install -r "$(requirements)" --pre --no-deps --upgrade `
--index-url https://pkgs.dev.azure.com/azure-sdk/public/_packaging/azure-sdk-for-python/pypi/simple

displayName: Install requirements from dev feed without dependencies
env:
REQUIREMENTS: $(requirements)

- pwsh: pip install -r $(Build.SourcesDirectory)/common/smoketest/requirements_async.txt
displayName: "Install requirements_async.txt"
condition: and(succeeded(), ne(variables['SkipAsyncInstall'], true))

- pwsh: |
python $(Build.SourcesDirectory)/common/smoketest/dependencies.py -r "$env:REQUIREMENTS" `
python $(Build.SourcesDirectory)/common/smoketest/dependencies.py -r "$(requirements)" `
| Out-File $(Build.SourcesDirectory)/common/smoketest/requirements_dependencies.txt
displayName: Create dependency list from installed packages
env:
REQUIREMENTS: $(requirements)

- script: pip install -r $(Build.SourcesDirectory)/common/smoketest/requirements_dependencies.txt
displayName: Install package dependencies from PyPI
Expand Down