-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Question: Is it possible to use fargate with the runners? #631
Comments
@frbk Hey!
This is working as intended but might be affecting your use-case, as Fargate requires your pods to have certain labels so that Fargate can discover which pods to be deployed onto it. Perhaps we need to fix how actions-runner-controller creates a registration-only runner pod, in a way that it doesn't rely on empty labels. Or perhaps you can wait for GitHub to add some API and system changes so that we can scale from/to zero without having a registration-only runner. #470 (comment)
The error says that you're trying to deploy it as a GitHub app and the private key you've provided was invalid. Check the content of the K8s secret that contains the private key. |
And most importantly, does Fargate supports deploying privileged containers today? In a standard setup, your runner pods and containers need to be privileged to work, especially for docker-in-docker. I thought there's some way to run dind without privileges but you need to set |
Hey @mumoshu . Thanks for the reply. Is |
I kinda assumed that I can replica what gitlab ci doing. |
@frbk Ah, gotcha! Then it should theoretically work if you set But the issue on empty private key would still be a blocker. BTW, to be extra clear- which pod showed the |
Nope. It's computed depending on the runner spec provided by you. |
I get this error on the runner pod. |
I have done a bit more investigating and these are the findings. It looks like runner pod is not mounting secrets when running on fargate. I was able to solve this by mounting this secrets in the
However, this doesn't seem to work because runner get stuck on authentication, also it looks like that the runner gets converted into a manager. Here is an example of the log:
|
@frbk Thanks. At glance, |
FYI, you can use |
OMG! Thanks for pointing out that I was using the wrong image. I am going to update it and redeploy. Will update you shortly! |
I have noticed that this isnt documented anywhere while working on #631 (comment)
@frbk Thanks for confirming! To be extra sure, let me point out that you should omit env like |
@mumoshu Here is an updated config which seem to work on fargate:
I only had to manually update config for registration-only pod to include labels as I mentioned before. |
@frbk Awesome! Thanks a lot for sharing your experience!
I was thinking about this a bit- this can possibly be automated by just removing this line from actions-runner-controller code: It would be great if you could try removing the code, building and pushing a custom image by running FYI, you can find definitions for |
Thanks for the info! Will give this a shot. |
@mumoshu Tried your suggestions and removing |
@frbk Awesome! Jus to be sure, did scale to/from zero both worked and replicas numbers shown in |
Looks like it @mumoshu .
Executed one job on the ci:
For testing purposes I set maxReplicas to 1. |
Just finished running a pipeline with 18 jobs in it and it was able to scale up and down with no issues 🎉 |
@frbk Thanks a lot for confirming! Let me add this to our documentation with a big "thanks to @frbk" note, and also apply the patch #631 (comment) to our main branch so that you no longer need to use the fork just for the one-line change. As this being an open-source and open-development project, I would also appreciate it very much if you could submit any pull request for any of (or even both) changes yourself! |
I have noticed that this isnt documented anywhere while working on #631 (comment)
Going to open a pr related to everything we talked about in this issue. Was gathering some info for documentation. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Probably this code change on runnerForScaleFromToZero isn't needed anymore. We no longer create registration-only runners for scale-from-to-zero in recent versions of ARC. |
@frbk Hey! How have your fargated-based runners been working since then? |
Hey @mumoshu. I have moved away to another company from then, however they were working fine when you didnt need to use docker in docker. I will try to provide a bit more info later this week. Just need to go over my old notes. Also, I see you changes the implementation for scaling from zero. I will try this over the weekend and will let you know. |
Give me couple more days. Had to setup a test eks cluster and it took a bit longer than I was expecting. Will update after I try out latest version of controller. |
Didn't forget about this. Schedule is a bit all over the place at the moment. 😭 |
@frbk Thanks! I'm looking forward to your report |
@NoamGoren Honestly, I have never tried it myself so I'm afraid I have nothing to share with you! What I can say, FWIW, is that ARC does not rely on registration-only runners anymore. So there may be a chance that it would work without any modifications now. |
hi @mumoshu the fargate is still not supporting the |
I thought the privileged containers were only necessary when using the docker sidecar? DinD has many implementations that do not require privileged? |
@mumoshu We are trying to use ARC with fargate, and I've come up with a very simple working hello-world deployment config that works, but it requires
|
Without dind, you cannot use service containers, container-based actions, and container-based steps in GitHub Actions! However, dind requires privileged containers, which are not available in Fargate. |
After setting dockerEnabled: false, we can use ARC in AWS EKS fargate. |
I have been trying to get runners deployed on fargate and wasn't able to find any info. So far I encountered couple of issues:
spec:template
which causes it to be stuck in the limbo. I was able to apply those labels using argocd.Here is an example of my config for fargate:
Please let me know if you have any suggestions.
The text was updated successfully, but these errors were encountered: