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.

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

feat: add check release to auto trigger internal release job #16207

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

tadelesh
Copy link
Member

@tadelesh tadelesh commented Nov 17, 2021

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to CHANGELOG.md are included.
  • MIT license headers are included in each file.

As there are too many manual trigger works for SDK release, I added a release check stage before the release pipeline. The stage will check whether there is a new release and to trigger or skip release stage then.

@weshaggard
Copy link
Member

@tadelesh can you give me more context on what we are trying to fix here? If we don't want to release a given package we should opt it out of release as opposed to ignoring what looks like could be real errors in other packages we do want to release.

@tadelesh
Copy link
Member Author

tadelesh commented Nov 18, 2021

@tadelesh can you give me more context on what we are trying to fix here? If we don't want to release a given package we should opt it out of release as opposed to ignoring what looks like could be real errors in other packages we do want to release.

I need to trigger an internal release pipeline manually if I need to do the SDK release for now. Then after the build stage in pipeline pass, I need to do another manual approval to continue the release stage. As we release SDK with separate RP packages, there are so many manual clicks. So I want to setup a new stage to trigger the internal release pipeline automatically. The trigger condition is: if a package is under development with the changelog in UNRELEASED status, then there is no need to trigger; if a package's latest version is ready with proper changelog, and such version has not released, then trigger. So the new added check release stage and script will do some checks with the changelog and tag, then next release stage can continue or skip automatically according to the check result. This stage is just a helper before the release stage and will not influence any existed release validations.

@tadelesh tadelesh requested a review from weshaggard November 18, 2021 02:31
@weshaggard
Copy link
Member

If the goal is to cut down on manual approvals, I don't think this is the correct approach. With the current changes it will actually potentially skip out on packages we want to ship but have errors that weren't discovered in other ways. I'm actually unsure how your current changes even eliminate the manual steps of clicking the approve button on the packages you want to release.

Just in case you aren't aware the thing that causes the manual approval check is the environment: github part of the deployment stage. If we want to reduce the number of approvals we would need to make some adjustments to that logic. We might be able to potentially setup a special pipeline or maybe a pipeline variable/parameter that someone could set to automatically do the release after build. However we need to be careful with doing that as only some folks have access to approve releases so whatever we setup will need to use the same SG for protecting the auto-release logic. If you want to setup some time with me next week I'd be happy to talk through some options.

@tadelesh
Copy link
Member Author

If the goal is to cut down on manual approvals, I don't think this is the correct approach. With the current changes it will actually potentially skip out on packages we want to ship but have errors that weren't discovered in other ways. I'm actually unsure how your current changes even eliminate the manual steps of clicking the approve button on the packages you want to release.

Just in case you aren't aware the thing that causes the manual approval check is the environment: github part of the deployment stage. If we want to reduce the number of approvals we would need to make some adjustments to that logic. We might be able to potentially setup a special pipeline or maybe a pipeline variable/parameter that someone could set to automatically do the release after build. However we need to be careful with doing that as only some folks have access to approve releases so whatever we setup will need to use the same SG for protecting the auto-release logic. If you want to setup some time with me next week I'd be happy to talk through some options.

I think you misunderstand what I've done. When I want to do a release for some SDK. I need to find the SDK's specific internal pipeline in the ado and trigger it manually. Then the pipeline will do build and test first. After that, I need to do the approval to continue the pipeline. So there are TWO manual work here (trigger and approval). What I've done is to cut the first manual trigger work. Any following release step including the approval step will not change.

@weshaggard
Copy link
Member

OK I think I see. You removed ${{if and(eq(variables['Build.Reason'], 'Manual'), eq(variables['System.TeamProject'], 'internal'))}}: That means any CI triggered build you can go in an approve the release for it. There are a couple issues with this approach:

  • You will need to keep the internal check here as the public builds will not have the necessary permissions to run the follow-up steps.
  • With this approach if no one decides to release the package then the pipeline will be in a state where the approval will timeout and then fail so the builds will all look like they have failed.

We will definitely have to fix the first issue but we might be able to live with the second issue but we would need to test to see what that looks like.

@benbp @seankane-msft what do you think about this release process for go packages?


if (!$changeLogEntry)
{
Write-Host "Changelog not existed for package: $PackageName, version: $($PackageProp.Version)."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Write-Host "Changelog not existed for package: $PackageName, version: $($PackageProp.Version)."
Write-Host "Changelog does not exist for package: $PackageName, version: $($PackageProp.Version)."


if ([System.String]::IsNullOrEmpty($changeLogEntry.ReleaseStatus) -or $changeLogEntry.ReleaseStatus -eq $CHANGELOG_UNRELEASED_STATUS)
{
Write-Host "Changelog not in release status for package: $PackageName, version: $($PackageProp.Version)."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Write-Host "Changelog not in release status for package: $PackageName, version: $($PackageProp.Version)."
Write-Host "Changelog is not in release status for package: $PackageName, version: $($PackageProp.Version)."

@tadelesh
Copy link
Member Author

OK I think I see. You removed ${{if and(eq(variables['Build.Reason'], 'Manual'), eq(variables['System.TeamProject'], 'internal'))}}: That means any CI triggered build you can go in an approve the release for it. There are a couple issues with this approach:

  • You will need to keep the internal check here as the public builds will not have the necessary permissions to run the follow-up steps.
  • With this approach if no one decides to release the package then the pipeline will be in a state where the approval will timeout and then fail so the builds will all look like they have failed.

We will definitely have to fix the first issue but we might be able to live with the second issue but we would need to test to see what that looks like.

@benbp @seankane-msft what do you think about this release process for go packages?

For the first issue, there is an internal check in the outside template (archetype-sdk-client.yml: ${{if and(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['System.TeamProject'], 'internal'))}}). I removed the inner check (archetype-go-release.yml: ${{if and(eq(variables['Build.Reason'], 'Manual'), eq(variables['System.TeamProject'], 'internal'))}}) for deduplication. Btw, I've fixed the log grammar. AFAIK, mgmt. plane of JS SDK has the same requirement.


if (!$changeLogEntry)
{
Write-Host "Changelog does not existed for package: $PackageName, version: $($PackageProp.Version)."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Write-Host "Changelog does not existed for package: $PackageName, version: $($PackageProp.Version)."
Write-Host "Changelog does not exist for package: $PackageName, version: $($PackageProp.Version)."

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

I'm good with these changes I would like to get someone like @seankane-msft to also examine the changes.

@tadelesh
Copy link
Member Author

I'm good with these changes I would like to get someone like @seankane-msft to also examine the changes.

Thanks @weshaggard . @benbp @seankane-msft Could you help to have a review?

@tadelesh tadelesh merged commit 251da19 into Azure:main Nov 24, 2021
@seankane-msft
Copy link
Member

@tadelesh I'm not sure this does what we want it to do. I tried to release internal with this internal pipeline, but the "Create release tag" step was never run. Can you take a look at why this is happening?

jobs:
- deployment: TagRepository
displayName: "Create release tag"
condition: and(succeeded(), eq(variables['NeedToRelease'], 'true'), ne(variables['Skip.TagRepository'], 'true'))
Copy link
Member

Choose a reason for hiding this comment

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

Based on https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables?view=azure-devops&tabs=yaml%2Cbatch#use-outputs-in-a-different-stage

I suspect you need to get this value from the outputs similar to the other usage. Actually, do we need the condition here as well as on the stage? Perhaps this one can be removed.

I think this is likely the reason for the issue that @seankane-msft is seeing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this job condition is wrong and also redundant. I've removed it by this PR. Thanks @weshaggard @seankane-msft for your validation and correction.

@tadelesh tadelesh deleted the release_pipeline branch December 20, 2021 05:47
jhendrixMSFT pushed a commit to jhendrixMSFT/azure-sdk-for-go that referenced this pull request Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants