CloudSnorkel / cdk-github-runners

CDK constructs for self-hosted GitHub Actions runners
https://constructs.dev/packages/@cloudsnorkel/cdk-github-runners/
Apache License 2.0
255 stars 37 forks source link

feat: Option to register runners on organization instead of repo #451

Closed pharindoko closed 7 months ago

pharindoko commented 7 months ago

App.svelte

Secrets.ts

Setup.lambda.ts

Tokenretriever.lambda.ts

Determine runner registration config command for:

Tests:

result: image

closes #442

kichik commented 7 months ago

Determine runner registration config command for:

This might be easier if we do #452 first.

pharindoko commented 7 months ago

Determine runner registration config command for:

This might be easier if we do #452 first.

Yes makes sense. I can continue to work on the your findings in the meantime.

pharindoko commented 7 months ago

I know it's still a draft, but I thought some early feedback might help.

That was the intention. Thanks for your feedback. Feel free to add changes directly. This PR affects way too much pieces that I can do it without your contribution.

pharindoko commented 7 months ago

@kichik

I`m ready :)

kichik commented 7 months ago

I am seeing issues with specifically Fargate failing to update the runner version and exiting. It doesn't feel related, but I have to confirm. Have you seen something like this?

  | 2023-10-26T21:40:18.885-04:00 | Current runner version: '2.310.2'
  | 2023-10-26T21:40:18.886-04:00 | 2023-10-27 01:40:18Z: Listening for Jobs
  | 2023-10-26T21:40:28.438-04:00 | Runner update in progress, do not shutdown runner.
  | 2023-10-26T21:40:28.551-04:00 | Downloading 2.311.0 runner
  | 2023-10-26T21:41:11.211-04:00 | Waiting for current job finish running.
  | 2023-10-26T21:41:11.262-04:00 | Generate and execute update script.
  | 2023-10-26T21:41:11.306-04:00 | Runner will exit shortly for update, should be back online within 10 seconds.
  | 2023-10-26T21:41:11.315-04:00 | Runner update process finished.
  | 2023-10-26T21:41:11.847-04:00 | Runner listener exit because of updating, re-launch runner after successful update
  | 2023-10-26T21:41:42.021-04:00 | Restarting runner...
  | 2023-10-26T21:41:42.058-04:00 | /home/runner/run-helper.sh: line 36: /home/runner/bin/Runner.Listener: No such file or directory
  | 2023-10-26T21:41:42.058-04:00 | Exiting with unknown error code: 127
  | 2023-10-26T21:41:42.058-04:00 | Exiting runner...
pharindoko commented 7 months ago

I am seeing issues with specifically Fargate failing to update the runner version and exiting. It doesn't feel related, but I have to confirm. Have you seen something like this?


  | 2023-10-26T21:40:18.885-04:00 | Current runner version: '2.310.2'

  | 2023-10-26T21:40:18.886-04:00 | 2023-10-27 01:40:18Z: Listening for Jobs

  | 2023-10-26T21:40:28.438-04:00 | Runner update in progress, do not shutdown runner.

  | 2023-10-26T21:40:28.551-04:00 | Downloading 2.311.0 runner

  | 2023-10-26T21:41:11.211-04:00 | Waiting for current job finish running.

  | 2023-10-26T21:41:11.262-04:00 | Generate and execute update script.

  | 2023-10-26T21:41:11.306-04:00 | Runner will exit shortly for update, should be back online within 10 seconds.

  | 2023-10-26T21:41:11.315-04:00 | Runner update process finished.

  | 2023-10-26T21:41:11.847-04:00 | Runner listener exit because of updating, re-launch runner after successful update

  | 2023-10-26T21:41:42.021-04:00 | Restarting runner...

  | 2023-10-26T21:41:42.058-04:00 | /home/runner/run-helper.sh: line 36: /home/runner/bin/Runner.Listener: No such file or directory

  | 2023-10-26T21:41:42.058-04:00 | Exiting with unknown error code: 127

  | 2023-10-26T21:41:42.058-04:00 | Exiting runner...

Hey @kichik,

no I haven't seen this message before. ... But I haven't triggered and tested an update of a fargate runner by intention.

Does the same issue occur on the main branch ?

kichik commented 7 months ago

@pharindoko I think I'm done with my adjustments. The only thing I still want to do is test the setup function to see that it can still create apps after my changes for both repo and org levels. Let me know if it looks good to you too.

pharindoko commented 7 months ago

@pharindoko I think I'm done with my adjustments. The only thing I still want to do is test the setup function to see that it can still create apps after my changes for both repo and org levels. Let me know if it looks good to you too.

Have seen your changes. I think it's fine.

Sorry that I missed to cleanup the runner level and check the formatting again.

The only thing I thought about is that it might be good to add some additional unit tests for the lambdas.

Will try to run the integration tests as well in my account when I have time - but don't wait for it...

kichik commented 7 months ago

We forgot to deal with runner deletion. I will have another pass tomorrow to see nothing else was missed.