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

Only push the production container when the release was cut #1077

Merged
merged 3 commits into from
Apr 9, 2024

Conversation

haoming29
Copy link
Contributor

This should fix the critical bug described in #1075 but leave the part untouched where "automatically tagging the latest release with the largest semantic version as the latest"

@haoming29 haoming29 added critical High priority for next release container labels Apr 8, 2024
@haoming29 haoming29 added this to the v7.7.0 milestone Apr 8, 2024
@haoming29 haoming29 requested a review from brianhlin April 8, 2024 21:39
Copy link
Contributor

@brianhlin brianhlin left a comment

Choose a reason for hiding this comment

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

Is there a way to be even more specific here? I imagine that /any/ workflow call (even those that aren't for release) would trigger this if.

Also, is there a world in which you'd want workflow_dispatch to trigger a push?

@haoming29
Copy link
Contributor Author

Is there a way to be even more specific here? I imagine that /any/ workflow call (even those that aren't for release) would trigger this if.

Also, is there a world in which you'd want workflow_dispatch to trigger a push?

Good call. I pass a variable from the prelease action to the build yaml to be sure it's called from the release

@@ -192,7 +194,7 @@ jobs:
with:
context: .
file: ./images/Dockerfile
push: ${{ github.repository == 'PelicanPlatform/pelican' && github.event_name != 'pull_request' }}
push: ${{ github.repository == 'PelicanPlatform/pelican' && (inputs.release_workflow || false) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's with the || false here? Kinda seems superfluous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does look superfluous but otherwise the yaml check will fail when the workflow was not triggered by the workflow call (in which case release_workflow is undefined) and gives the error:

Error: Input does not meet YAML 1.2 "Core Schema" specification: push
Support boolean input list: `true | True | TRUE | false | False | FALSE`

Copy link
Contributor

@brianhlin brianhlin left a comment

Choose a reason for hiding this comment

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

One question but otherwise LGTM. Pre-approving.

@haoming29 haoming29 force-pushed the only-push-when-release branch from 8fbbeff to b1d2e45 Compare April 9, 2024 15:25
@haoming29 haoming29 merged commit 7f38798 into PelicanPlatform:main Apr 9, 2024
19 checks passed
@haoming29 haoming29 deleted the only-push-when-release branch April 9, 2024 15:54
@haoming29 haoming29 linked an issue Apr 10, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container critical High priority for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overhaul production docker container push and tagging
2 participants