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

don't skip the "build platforms" job even when there are no platforms to build #283

Closed
wants to merge 1 commit into from

Conversation

spoonincode
Copy link
Member

When there are no platforms to build for the platform cache, it conceptually makes sense to skip the platform building job matrix. But skipping a job in the middle of the graph like this causes some grief downstream: we needed to start doing things like if: always() && needs.Build.result == 'success' for dependent jobs because the default success() rule didn't like to see an ancestor skipped.

But this if: always() hack comes with a nasty side effect: it's impossible to cancel the workflow. Apparently a job with if: always() is not cancellable. This is particularly annoying when something like NP Tests fail and you have to wait until LR Tests finish before you can retry the workflow.

So instead, the Build Platforms job will now inject a dummy entry in to its matrix when there are no platforms to build. This allows the job to still pass when it has no work to do and all dependent jobs don't need the if: always() hack. Thus making the workflow cancellable.

At some point, the discover & build jobs should be bundled in to a reusable workflow to encapsulate away some of the gory details.

@spoonincode spoonincode added the CICD Anything dealing with the CI workflow behavior label Oct 4, 2022
Copy link
Contributor

@kj4ezj kj4ezj left a comment

Choose a reason for hiding this comment

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

🍕

@spoonincode spoonincode marked this pull request as draft October 18, 2022 21:19
@spoonincode
Copy link
Member Author

I'm holding up this change until runners can be more robust in handling cancelled jobs.

@spoonincode
Copy link
Member Author

changes effectively incorporated in #1703

@spoonincode spoonincode closed this Oct 3, 2023
@oschwaldp-oci
Copy link
Contributor

I'm holding up this change until runners can be more robust in handling cancelled jobs.
changes effectively incorporated in #1703

Out of curiosity, does this mean that the runners now more robustly handle cancelled jobs?

@spoonincode spoonincode deleted the no_skip_platform_build branch October 3, 2023 16:36
@spoonincode
Copy link
Member Author

No, they still have problems with that 😞 But this PR is from.. oh man almost 1 year ago.. I don't want to hold up progress forever. Maybe the problems will be benign in practice, or maybe this will force me to come up with a solution quicker.

@oschwaldp-oci
Copy link
Contributor

No, they still have problems with that 😞 But this PR is from.. oh man almost 1 year ago.. I don't want to hold up progress forever. Maybe the problems will be benign in practice, or maybe this will force me to come up with a solution quicker.

You got my hopes up. Although it doesn't come up too frequently. Thanks for the reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD Anything dealing with the CI workflow behavior
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants