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.

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

fix: instance initialization cloud-init status check #11

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

yanksyoon
Copy link
Contributor

Applicable spec: N/A

Overview

Also include cloud-init status check for when instantiating runners on wait_runner checks.

Rationale

Fixes issue with short running jobs already being done before health checks.

Module Changes

Library/Dependency Changes

Checklist

  • The contributing guide was applied
  • The documentation is generated using src-docs
  • The PR is tagged with appropriate label (urgent, trivial, complex)
  • The library version in pyproject.toml is incremented

@yanksyoon yanksyoon added bug Something isn't working urgent labels Sep 13, 2024
@yanksyoon yanksyoon requested a review from a team as a code owner September 13, 2024 07:33
cbartz
cbartz previously approved these changes Sep 13, 2024
Copy link
Collaborator

@cbartz cbartz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

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

I think src/github_runner_manager/openstack_cloud/openstack_runner_manager.py line 654 (

) should be changed from
if RUNNER_STARTUP_PROCESS not in result.stdout: to if RUNNER_STARTUP_PROCESS not in result.stdout or RUNNER_WORKER_PROCESS not in result.stdout or RUNNER_LISTENER_PROCESS not in result.stdout.

Else if the runner is already at the stage of RUNNER_WORKER_PROCESS/RUNNER_LISTENER_PROCESS then this startup check will fail.

Edit: Yanks pointed out the RUNNER_STARTUP_PROCESS is the parent process of RUNNER_WORKER_PROCESS/RUNNER_LISTENER_PROCESS. Therefore this is not an issue.

yhaliaw
yhaliaw previously approved these changes Sep 13, 2024
@yanksyoon yanksyoon dismissed stale reviews from yhaliaw and cbartz via d0f8fc5 September 13, 2024 08:08
Copy link

Test coverage for d0f8fc5

Name                                                         Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------------------------------------------------------
src/github_runner_manager/__init__.py                            0      0      0      0   100%
src/github_runner_manager/errors.py                             22      0      0      0   100%
src/github_runner_manager/github_client.py                      83     36     34      2    47%   63-72, 100-135, 148-157, 171-180, 191-198, 220->260, 254
src/github_runner_manager/manager/__init__.py                    0      0      0      0   100%
src/github_runner_manager/manager/cloud_runner_manager.py       61      0      6      0   100%
src/github_runner_manager/manager/github_runner_manager.py      17      0      6      0   100%
src/github_runner_manager/manager/runner_manager.py            134     34     48      5    70%   175, 187, 191, 207-212, 230-231, 268-284, 298-305, 325-329, 338
src/github_runner_manager/manager/runner_scaler.py             106     10     38      3    90%   128-129, 143-144, 149, 155, 249-250, 263-264
src/github_runner_manager/metrics/__init__.py                    0      0      0      0   100%
src/github_runner_manager/metrics/events.py                     55      2      8      1    95%   56->59, 166-167
src/github_runner_manager/metrics/github.py                     16      0      0      0   100%
src/github_runner_manager/metrics/runner.py                    144     10     32      3    91%   164, 190-203, 239, 276, 458-459
src/github_runner_manager/metrics/runner_logs.py                24      5      4      1    79%   30-33, 47->46, 51-52
src/github_runner_manager/metrics/storage.py                    70      8     12      0    90%   89-90, 117-118, 182-183, 189-190
src/github_runner_manager/metrics/type.py                        5      0      0      0   100%
src/github_runner_manager/openstack_cloud/__init__.py           26      0      2      0   100%
src/github_runner_manager/reactive/__init__.py                   0      0      0      0   100%
src/github_runner_manager/reactive/consumer.py                  40      2      6      0    96%   98-101
src/github_runner_manager/reactive/runner.py                    18     18      6      0     0%   6-50
src/github_runner_manager/reactive/runner_manager.py            53      0     14      1    99%   103->exit
src/github_runner_manager/types_/__init__.py                    33      8      4      1    70%   39-52, 72, 80
src/github_runner_manager/types_/github.py                      72      9     10      0    82%   186, 207, 228-235
src/github_runner_manager/utilities.py                          46     28     14      0    30%   73-100, 127-145, 157-158
--------------------------------------------------------------------------------------------------------
TOTAL                                                         1025    170    244     17    80%

Static code analysis report

Run started:2024-09-13 08:10:25.939461

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 3711
  Total lines skipped (#nosec): 2
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 3

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@cbartz cbartz merged commit 6ee40e2 into main Sep 13, 2024
10 checks passed
@cbartz cbartz deleted the fix/wait_runner_check branch September 13, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants