Repository navigation
fix(ci): make stable GitHub releases idempotent #976
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,11 @@ on: | |
| required: false | ||
| type: boolean | ||
| default: false | ||
| create-github-release: | ||
| description: "Create the Git tag and GitHub Release for a custom stable release. Stable all releases do this automatically." | ||
| required: false | ||
| type: boolean | ||
| default: false | ||
| helm-version: | ||
| description: "Optional exact Helm chart version override for stable Helm-only releases." | ||
| required: false | ||
|
|
@@ -159,6 +164,7 @@ jobs: | |
| helm_version_override: ${{ steps.plan.outputs.helm_version_override }} | ||
| update_ngc_metadata: ${{ steps.plan.outputs.update_ngc_metadata }} | ||
| send_notifications: ${{ steps.plan.outputs.send_notifications }} | ||
| create_github_release: ${{ steps.plan.outputs.create_github_release }} | ||
| dry_run: ${{ steps.plan.outputs.dry_run }} | ||
| steps: | ||
| # This job is intentionally the first gate. It resolves the user's | ||
|
|
@@ -189,6 +195,9 @@ jobs: | |
| const releaseScope = isManual ? (inputs["release-scope"] || "all") : "all"; | ||
| const updateNgcMetadata = isManual && inputs["update-ngc-metadata"] === "true"; | ||
| const sendNotifications = inputs["send-notifications"] !== "false"; | ||
| const createGitHubReleaseRequested = ( | ||
| isManual && inputs["create-github-release"] === "true" | ||
| ); | ||
| const dryRun = isManual && inputs["dry-run"] === "true"; | ||
| const helmVersionOverride = isManual ? (inputs["helm-version"] ?? "").trim() : ""; | ||
|
|
||
|
|
@@ -218,6 +227,14 @@ jobs: | |
| } | ||
| } | ||
|
|
||
| const createGitHubRelease = ( | ||
| releaseType === "stable" | ||
| && ( | ||
| releaseScope === "all" | ||
| || (releaseScope === "custom" && createGitHubReleaseRequested) | ||
| ) | ||
| ); | ||
|
|
||
| sourceSha = sourceSha.toLowerCase(); | ||
|
|
||
| const presets = { | ||
|
|
@@ -326,6 +343,7 @@ jobs: | |
| core.setOutput("helm_version_override", helmVersionOverride); | ||
| core.setOutput("update_ngc_metadata", String(updateNgcMetadata)); | ||
| core.setOutput("send_notifications", String(sendNotifications)); | ||
| core.setOutput("create_github_release", String(createGitHubRelease)); | ||
| core.setOutput("dry_run", String(dryRun)); | ||
|
|
||
| core.info( | ||
|
|
@@ -1100,7 +1118,7 @@ jobs: | |
| needs: [plan-release, poll-final-release] | ||
| if: >- | ||
| needs.plan-release.outputs.release_type == 'stable' && | ||
| needs.plan-release.outputs.release_scope == 'all' && | ||
| needs.plan-release.outputs.create_github_release == 'true' && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this at all? I don't see any reason not to just remove this and not introduce the new parameter. An improvement would be to also just output what it would have created with ACT="true", but I emphatically reject needing to explicitly ask for release when I said make a stabe release already. :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we can use stable custom runs to retry failed artifacts too, so we need a way to say whether that run should also cut the tag/release.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise it'll fail by default if the tag already exists
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about checking for the tag first and making it idempotent instead? This adds a random clickbox for what it should do by default.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the PR |
||
| needs.plan-release.outputs.dry_run != 'true' && | ||
| needs.poll-final-release.result == 'success' | ||
| runs-on: ubuntu-latest | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
each day, the parameter list grows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which is fine if the default is "do what we want most of the time"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I now believe this does NOT do that. This is the opposite of that, imo. Commented further below. I could be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dropped the input