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

Remove race condition between restart repo-policy-compliance and request of the service #166

Merged
merged 48 commits into from
Feb 1, 2024

Conversation

yhaliaw
Copy link
Collaborator

@yhaliaw yhaliaw commented Dec 11, 2023

Overview

On update of service, flush the runners, restart the services, then spawn runners to replace the flushed ones.
Prior to flushing the runners, wait until all running job ran at least 1 minute.

Rationale

The race condition can cause the pre-job stage to fail from time to time.

Checklist

@yhaliaw yhaliaw marked this pull request as ready for review January 16, 2024 06:00
@yhaliaw yhaliaw requested a review from a team as a code owner January 16, 2024 06:00
src/runner_manager.py Show resolved Hide resolved
src/runner_manager.py Outdated Show resolved Hide resolved
src/runner_manager.py Show resolved Hide resolved
@yhaliaw yhaliaw marked this pull request as draft January 17, 2024 11:02
@yhaliaw yhaliaw marked this pull request as ready for review January 22, 2024 07:22
yanksyoon
yanksyoon previously approved these changes Jan 22, 2024
Copy link
Collaborator

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM, a few questions, thanks!

tests/integration/test_charm_one_runner.py Outdated Show resolved Hide resolved
tests/integration/test_charm_one_runner.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tests/integration/test_charm_one_runner.py Outdated Show resolved Hide resolved
@yhaliaw yhaliaw dismissed stale reviews from yanksyoon and cbartz via 5955379 January 31, 2024 03:18
yanksyoon
yanksyoon previously approved these changes Jan 31, 2024
Copy link
Collaborator

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

Minor nitpicks, LGTM!

src/runner.py Show resolved Hide resolved
tests/integration/test_self_hosted_runner.py Show resolved Hide resolved
cbartz
cbartz previously approved these changes Jan 31, 2024
@yhaliaw yhaliaw dismissed stale reviews from cbartz and yanksyoon via 9bea9e2 January 31, 2024 08:52
yanksyoon
yanksyoon previously approved these changes Jan 31, 2024
cbartz
cbartz previously approved these changes Jan 31, 2024
Copy link
Contributor

github-actions bot commented Feb 1, 2024

Test coverage for abfc217

Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
src/charm.py                   456    110    117     25    74%   87-89, 113-114, 118-120, 158-160, 170->182, 180, 215-234, 248-250, 251->255, 279-283, 314, 394-399, 422, 439-440, 449-472, 492-497, 503-506, 513, 516-517, 533, 537-542, 587->590, 603-604, 606-607, 641-648, 672-673, 685-687, 702-703, 731-732, 734-735, 737-738, 812->814, 875-876, 892, 913-915, 919-922, 926
src/charm_state.py             138     18     22      3    86%   131-138, 221-224, 266-268, 279-280, 286-291, 302-304
src/errors.py                   39      0      0      0   100%
src/event_timer.py              45      9      6      1    73%   80, 100-103, 123-126
src/firewall.py                 52     18     20      0    61%   42-43, 66-69, 110-184
src/github_client.py            68      6     32      3    87%   124, 146, 158-165, 183->216
src/github_metrics.py           14      0      0      0   100%
src/github_type.py              50      0      0      0   100%
src/lxd_type.py                 37      0      2      0   100%
src/metrics.py                  73      2     10      1    96%   61->64, 170-171
src/metrics_type.py              6      0      0      0   100%
src/runner.py                  329     72     96     24    73%   46->48, 130, 235-236, 262-263, 304-313, 337-342, 347, 367, 371-381, 430->433, 436-438, 445, 459, 469, 473, 475, 490, 524-529, 545-558, 569-608, 613, 651, 704-706, 710, 728, 763, 789, 794-806, 820, 825->827, 830-832
src/runner_logs.py              35      2      6      1    93%   62->61, 66-67
src/runner_manager.py          301     70    106      9    75%   123, 168-170, 183-184, 196-198, 204-209, 213-214, 224-225, 244, 287-288, 332-334, 399, 419-423, 448, 500-503, 535-553, 584-589, 597-604, 699-712, 722, 727-738
src/runner_manager_type.py      39      0      6      0   100%
src/runner_metrics.py          123      8     20      2    93%   147-148, 160, 191-192, 307-311
src/runner_type.py              47      0     10      0   100%
src/shared_fs.py               116     16     20      0    88%   108-109, 133-134, 217-218, 241-242, 254-255, 260-261, 267-268, 292-293
src/utilities.py                68      5     20      7    86%   74->76, 78->84, 91, 121, 135, 173, 226
------------------------------------------------------------------------
TOTAL                         2036    336    493     76    81%

Static code analysis report

Run started:2024-02-01 01:58:03.948013

Test results:
  No issues identified.

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

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):

@yhaliaw yhaliaw merged commit 7da7cde into main Feb 1, 2024
57 checks passed
@yhaliaw yhaliaw deleted the bug/update-dep-race-condition branch February 1, 2024 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants