search

actions / runner-container-hooks

Runner Container Hooks for GitHub Actions
MIT License
63 stars 41 forks source link

Possibility to specify resources and limits for services containers #132

Open katarzynainit opened 5 months ago

katarzynainit commented 5 months ago

Hi, I'm using ARC with runner-container-hooks in kubernetes mode. I'd like to be able to define resources/limits for the services. When service container is added to the container job pod, it has no resources defined, which, in my case, is not accepted (I need to provide some resources, otherwise I will get default values, which are way to small to operate).

Currently, the only way to do it, is to use ACTIONS_RUNNER_CONTAINER_HOOK_TEMPLATE providing services as fixed containers in the spec, which means I cannot use them from workflow - I will end up with containers with names that are duplicated (from service and from pod spec).

Am I missing something?

Would it be possible to either provide resources from workflow, or to follow $job pattern (from the hook) to only adjust service containers?

nikola-jokic commented 5 months ago

Hey @katarzynainit,

I think the best approach is to follow the $job pattern and re-use it for services. This would be a good improvement of the hook extension for sure.

katarzynainit commented 5 months ago

Understood! Any possibility to see such option on the "backlog"? Or you'd rather expect contribution? :)

nikola-jokic commented 5 months ago

I can take this one, unless you would like to contribute :relaxed: I can review it

katarzynainit commented 5 months ago

Will ping you with PR once it's ready :)

katarzynainit commented 4 months ago

Hi @nikola-jokic
Can you please check https://github.com/actions/runner-container-hooks/pull/136 ?

David9902 commented 4 months ago

Hi @katarzynainit, thanks that you brought up this topic! What I am wondering here https://github.com/actions/actions-runner-controller/discussions/3338 is it possible to set just one spec for all service containers?

Or would this be part of an additional implementation phase @nikola-jokic ?

nikola-jokic commented 4 months ago

Hey @katarzynainit,

I'm sorry for the delay. I hope I'll review these changes this week, and we can issue a new hook release for them :relaxed:

Thank you again for your contributions!

@David9902 With the current change, no, but there could be a way of having $service as container to specify it. I'll reach out to the team to get a second opinion, and I'll let you know :smile:

katarzynainit commented 4 months ago

Thanks for merging the PR :)

I'm leaving the issue open as there is additional discussion with @David9902