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.

Sign up for GitHub

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

Jump to bottom

feat: Natively support runner pre/post job hooks #4263

Merged
merged 5 commits into from
Dec 20, 2024
Merged

feat: Natively support runner pre/post job hooks #4263

merged 5 commits into from
Dec 20, 2024

Conversation

winwinashwin
Copy link
Contributor

Adds two optional string arguments to the module

  • runner_hook_job_started
  • runner_hook_job_completed

If not empty, registers the hook in user-data.sh

Ref: https://github.com/actions/runner/blob/main/docs/adrs/1751-runner-job-hooks.md

Closes: https://github.com/philips-labs/terraform-aws-github-runner/issues/4260
Discussion: https://github.com/philips-labs/terraform-aws-github-runner/issues/3854

@winwinashwin winwinashwin changed the title [Feat] Natively support runner pre/post job hooks feat: Natively support runner pre/post job hooks Nov 16, 2024
@winwinashwin winwinashwin force-pushed the ashwin/hooks branch 4 times, most recently from 0318a59 to 1182106 Compare November 16, 2024 10:59
@winwinashwin
Copy link
Contributor Author

@npalm Could you please review?

@npalm
Copy link
Collaborator

npalm commented Nov 18, 2024 •
edited
Loading

A bit annoying that the pull request checks are not showing up after the docs workflows kicks off. Maybe any clue how to fix this.

npalm
npalm reviewed Nov 18, 2024
View reviewed changes
Copy link
Collaborator

@npalm npalm left a comment

Choose a reason for hiding this comment

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

Nice contribution! I will check the PR in detail later. But could you check if you can enrich the multi-runner example with the pre / post hook. I would suggest to use one of the amazon linux examples.

@winwinashwin
Copy link
Contributor Author

winwinashwin commented Nov 18, 2024 •
edited
Loading

A bit annoying that the pull request checks are not showing up after the docs workflows kicks off. Maybe any clue how to fix this.

  1. Is it necessary to generate the docs at a PR level? Can we do a daily/weekly sync?
    If the commit is being added to the PR, then github will override the check suites with the latest commit.

  2. Or we could run the tests for all commits (even for doc)

@winwinashwin
Copy link
Contributor Author

Enrich the multi-runner example with the pre / post hook.

Thanks. Added examples in the PR.

@winwinashwin winwinashwin requested a review from npalm November 18, 2024 18:58
@winwinashwin
Copy link
Contributor Author

@npalm Anything I can help with to get this merged? Thanks for your time.

@npalm
Copy link
Collaborator

npalm commented Nov 20, 2024 •
edited
Loading

@npalm Anything I can help with to get this merged? Thanks for your time.

Not much, clear my calendar. I started last week from the bottom of the PR list. Do my best to check the PR this week. Did a quicke check, see the comments. Will run asap tests based on the examples.

@winwinashwin
feat: Natively support runner job started/completed hooks (github-aws…
fd48501 
…-runners#4260)

Pre and post job hooks were added to github actions to help
administrators run custom scripts at the beginning and end of every job.
As of today the module doesn't support these options out of the box.

Add variables to accept these optional scripts and register the hook in
user-data.

Also enrich linux-arm64 example in multi-runner with pre/post hooks

Related to: https://github.com/philips-labs/terraform-aws-github-runner/issues/3854
@winwinashwin
Copy link
Contributor Author

@npalm Thank you for your review. Updated changes as suggested.

This was referenced Nov 22, 2024
Closed
Merged
@winwinashwin
Copy link
Contributor Author

@npalm Anything pending here?

@npalm
Copy link
Collaborator

npalm commented Nov 28, 2024

@npalm Anything pending here?

No, but my time is very limitted this week.

@npalm
Copy link
Collaborator

npalm commented Dec 20, 2024

pushed a change to simply the test script in examples, now it works

image

@npalm npalm merged commit 259a852 into github-aws-runners:main Dec 20, 2024
42 checks passed
@forest-releaser forest-releaser bot mentioned this pull request Dec 20, 2024
Merged
npalm pushed a commit that referenced this pull request Dec 20, 2024
@forest-releaser
chore(main): release 5.21.0 (#4313)
dd4591e 
🤖 I have created a release *beep* *boop*
---


##
[5.21.0](philips-labs/terraform-aws-github-runner@v5.20.1...v5.21.0)
(2024-12-20)


### Features

* Natively support runner pre/post job hooks
([#4263](https://github.com/philips-labs/terraform-aws-github-runner/issues/4263))
([259a852](philips-labs/terraform-aws-github-runner@259a852))


### Bug Fixes

* Incorrect syncer binary location in tf outputs
([#4274](https://github.com/philips-labs/terraform-aws-github-runner/issues/4274))
([401a373](philips-labs/terraform-aws-github-runner@401a373)),
closes
[#4137](https://github.com/philips-labs/terraform-aws-github-runner/issues/4137)
* **lambda:** bump @octokit/types from 13.6.1 to 13.6.2 in /lambdas in
the octokit group
([#4303](https://github.com/philips-labs/terraform-aws-github-runner/issues/4303))
([9f76c4c](philips-labs/terraform-aws-github-runner@9f76c4c))
* **lambda:** bump axios from 1.7.7 to 1.7.9 in /lambdas
([#4305](https://github.com/philips-labs/terraform-aws-github-runner/issues/4305))
([e3cd5b4](philips-labs/terraform-aws-github-runner@e3cd5b4))
* **lambda:** bump the aws group across 1 directory with 7 updates
([#4314](https://github.com/philips-labs/terraform-aws-github-runner/issues/4314))
([3f9b768](philips-labs/terraform-aws-github-runner@3f9b768))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npalm npalm npalm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] Configuration option for runner job hooks
2 participants
@winwinashwin @npalm