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

Is it possible to exclude a specific check from being required for deployment? #321

Open
redoz opened this issue Nov 15, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@redoz
Copy link

redoz commented Nov 15, 2024

Details

We use branch-deploy to deploy to our dev environments and part of that deployment is to run our integration test suite. This test suite creates a check in github to indicate whether it succeeded or not, and this check is required to succeed for us to merge this branch into main.

Now, branch-deploy also requires all checks to pass before running the deployment, so we have a bit of a catch 22 if the integration tests fail, we cannot re-run the deployment without touching the branch to reset the failed check, which could trigger lengthy builds due to the content in the pull request.

In theory this isn't something that should be required, the tests fail, you should fix them before trying to re-run them, but unfortunately it can happen due to the complexity of the system and the tests that a test can fail due to an issue with an external dependency.

I think in effect what I'm asking for is an inverse to required_contexts, a way to opt out of a check being required for the deployment (even though it's required in the branch protection settings of the repository).

@redoz redoz added the question Further information is requested or the issue is a question label Nov 15, 2024
@GrantBirki
Copy link
Member

👋 Hey @redoz thank you for opening this issue and sorry for the delay, I was traveling. I think your suggested approach makes a lot of sense. You are probably looking for a new feature where we have something like ignored_contexts in addition to the existing required_contexts so that certain CI checks can be disabled.

That could potentially take some time to implement. In the meantime, have you looked into the checks: "required" field? Perhaps you could make your acceptance test (for your dev env) not required and then use this branch-deploy setting.

@idasilva
Copy link

i am looking forward for this feature, some checks is required to deploy o specific environment but not all of them.

@redoz
Copy link
Author

redoz commented Dec 4, 2024

Hi @GrantBirki no worries, I think the inclusion of an ignored_contexts property would indeed fix the issue. But unless I've misunderstood something (quite possible) I don't think the checks: required property would help us, we use a monorepo setup where only changed projects are rebuilt. The only required check in the main branch protection is the integration test suite.

So branch-deploy wont run unless the builds pass (that are triggered by watching for path changes), and by default you can't merge the branch even if the builds pass, because the integration test suite hasn't run yet.

Once we run the branch-deploy onto one of our dev environments it executes the integration tests and will add the pass/fail check to the PR/Commit.

If it fails, I cannot re-run the branch-deploy because no one of the checks are failing (which is slightly annoying, because in order to release that check, we have to push to the branch, which triggers all changed components to rebuild again). But also, I cannot merge the PR because the required branch protection rule has failed, which is exactly what we want.

I don't think I can list all component builds as required for branch-deploy, because if they didn't change, they wont rebuild, so they by definition didn't pass then, correct? And if that assumption is true I'd then be forced to rebuild every component on any change in the PR which would be quite inefficient.

@GrantBirki
Copy link
Member

@redoz Thank you for the clarity here! So it does indeed sound like a ignored_contexts input would be the solution here. This would be a great feature to have then so I'll add this to the backlog. Thank you!

@GrantBirki GrantBirki added enhancement New feature or request and removed question Further information is requested or the issue is a question labels Dec 4, 2024
@GrantBirki GrantBirki self-assigned this Dec 10, 2024
@GrantBirki
Copy link
Member

Upon further research into this feature, I have decided against making an ignore_contexts option as this would actually be quite confusing to the required_contexts option that already exists. Instead, I will be making adjustments to the existing checks input and adding a new ignored_checks input. The reason for this is that the required_contexts input adjusts the payload that is sent to the GitHub API upon deployment. This feature is baked into this Action for those that really need it but in the vast majority of cases you shouldn't have to touch this option.

So in practice, the features that people are really going to want is being able to control which CI checks (not contexts) control whether their deployment can proceed or not.

Currently, the checks input option only supports the two following values:

  • all - All CI checks must pass for a deployment to start
  • required - Only CI checks marked as "required" via your branch protection settings are required

I am going to make it so that the checks option now also accepts CI job names (in addition to all / required) so that those names can be used as a "filter" as to what CI jobs should truly block deployments.

To complement these changes, I will also make a new input option called ignored_checks which does the inverse. Rather than list out all the checks you want to pass, with ignored_checks you can simply provide the ones that you don't care about. Is your lint job failing, and you don't want it to block you from shipping a critical bug fix? No problem... ignored_checks: lint

@redoz
Copy link
Author

redoz commented Dec 11, 2024

@GrantBirki that sounds like it would fit my use case perfectly, thank you!

@GrantBirki
Copy link
Member

@redoz These changes are merged into main if you want to check them out. Otherwise, a new major release (v10) will be coming out shortly 🎉

@GrantBirki
Copy link
Member

Here is the latest RC containing the changes if you want to test it out!

v10.0.0-rc.1

@redoz
Copy link
Author

redoz commented Dec 12, 2024

Here is the latest RC containing the changes if you want to test it out!

v10.0.0-rc.1

Just changed our workflow to use the RC and the new ignore_checks parameter and it does exactly what we want, thank you so much for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants