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

Adding support for Additional Hooks Paths #3124

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

CerealBoy
Copy link
Contributor

@CerealBoy CerealBoy commented Dec 12, 2024

Description

We would like to provide the ability to place hooks in multiple locations, meaning there can be multiple hooks of the same type, and they can be provided under different circumstances. These additional hooks are comma separated list, and ordered as they are provided in terms of how multiple hooks are executed. Those in hooks-path are always first, and then sequentially through additional-hooks-paths.

There are 2 places where we pull in these hooks:

  1. During the agent startup phase
  2. Through the job execution phases

We don't currently support the pre-bootstrap hook with additional paths.

Note

There is no change to any default behaviour included, nor is there any change in existing functionality when configuration is not changed. The only way to have any impact from this change is to set the new option with some value (which should be a comma separated list of paths).

A test is included here to validate the ordering of the hooks is predictable and preserved, however there is a weird timing issue during the test that prevents the additional hooks from being executed successfully. All manual usage of the changes has yielded expected results, so the test is left with t.SkipNow() to highlight that it should be possible to test this, and perhaps there's something I've overlooked here.

Screenshots

Before any additional hook paths are defined:
Screenshot 2024-12-30 at 12 32 11

Example trace showing additional hooks being run for a job:
image

Build output of the above within Buildkite
image

Changes

  • Adding BUILDKITE_ADDITIONAL_HOOKS_PATHS configuration option
  • Find and run any hooks found after hooks-path
    • For the agent-startup hook
    • For hooks within job execution

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go fmt ./...)
  • Additional tests covering new functionality (although t.SkipNow() is used)

@DrJosh9000 DrJosh9000 self-requested a review December 16, 2024 02:32
@CerealBoy CerealBoy force-pushed the feature/additional-hooks-paths branch from a61e84b to 5fc588d Compare December 27, 2024 01:58
@CerealBoy CerealBoy force-pushed the feature/additional-hooks-paths branch from 5fc588d to 752e2c3 Compare December 29, 2024 22:03
@CerealBoy CerealBoy marked this pull request as ready for review December 30, 2024 01:37
Copy link
Contributor

@wolfeidau wolfeidau left a comment

Choose a reason for hiding this comment

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

Looks great 🚀 ready for some testing.

@CerealBoy CerealBoy merged commit 841f492 into main Jan 2, 2025
1 check passed
@CerealBoy CerealBoy deleted the feature/additional-hooks-paths branch January 2, 2025 04:39
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.

2 participants