Implement yaml extensions overwriting the default pod/container spec #75
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
When running |
|
Hi. Is there any ETA for this PR to be merged? We're eagerly awaiting it as I understand it will allow us to request K8s resources such as GPUs for the build jobs. |
| @@ -23,6 +27,47 @@ describe('Run container step', () => { | |||
| expect(exitCode).toBe(0) | |||
| }) | |||
|
|
|||
| it('should run pod with extensions applied', async () => { | |||
There was a problem hiding this comment.
leave a // explaining that we only assert successful run here not individual overrides
| ] | ||
| } as k8s.V1Container | ||
|
|
||
| const from = { |
There was a problem hiding this comment.
NIT: maybe from -> extension
| ): void { | ||
| for (const [key, value] of Object.entries(from)) { | ||
| if (key === 'containers') { | ||
| base.containers.push( |
There was a problem hiding this comment.
NIT: extract function mergeContainers like a few lines below with mergeLists ?
| @@ -0,0 +1,30 @@ | |||
| metadata: | |||
There was a problem hiding this comment.
NIT: comments indicating section by section if it's an 'replace' or a 'merge'?
No need if the ADR explains it
|
A huge thanks for your work on this! We plan on using it for a build farm at Diamond Light Source (the UK's national synchrotron). So you are directly helping us with valuable scientific research into batteries, drugs for cancer, and materials science for renewables. |
|
Hey @cjreyn, Sorry that we did not respond earlier! Thank you for using the container hook and having interest in it! We are glad if it helps you with your work! |
|
Happy to see this merged! Seems you went with the template file after all 😉 I'll close my PR here: #50 Are there also plans for integrating this change in the actions-runner-controller downstream? We internally would love to move away from our current complex fork 😄 |
Implements ADR