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.

Sign up for GitHub

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

Jump to bottom

Externalise Runner Manager #358

Merged
merged 314 commits into from
Sep 9, 2024
Merged

Externalise Runner Manager #358

merged 314 commits into from
Sep 9, 2024

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented Aug 29, 2024 •
edited
Loading

Applicable spec: n/a

Overview

Externalise the refactored runner interface and dependent modules into a separate python package (canonical/github-runner-manager#2)

Rationale

A step forward in having a separate Runner Manager application that is independent of the Charm and Charm State.

Juju Events Changes

n/a

Module Changes

Move any modules that the refactored runner interface depends on.

Library Changes

n/a

Checklist

@yhaliaw
Spawn a manual test env
207913f
@yhaliaw
Disable spawning on manual test env
7af0589
@yhaliaw
Remove useless class
1c8ea0d
@yhaliaw
Fix runner deletion
ce05cf5
@yhaliaw
Fix import error
ea1a726
@yhaliaw
Add more docs
05a9016
@yhaliaw
Fix get no-existing openstack server
752aa0e
@yhaliaw
Add debug statement
280480b
@yhaliaw
Fix variable name and function name mixup
7df1802
@yhaliaw
Fix id variable name, function name mixup
d1ecfb6
@yhaliaw
Add debug statement
1ab8db6
@yhaliaw
Move debug
78fdac2
@yhaliaw
Add busy runner test
fd628e6
@yhaliaw
Add debug statement.
e688b6c
@yhaliaw
Disable some test
4296e04
@yhaliaw
Disable some test
7d2bc00
@yhaliaw
Fix runner label in workflow
d90f70b
@yhaliaw
Fix lambda
a868a83
@yhaliaw
Debug
11b9f78
@yhaliaw
Debug
4d25f2c
@yhaliaw
Add debug
d38d067
@yhaliaw
Start new manual test env
03dd839
@yhaliaw
Add none check
d42f2b5
@yhaliaw
Fix missing prefix
6aff0f1
@yhaliaw
Add more logging
c106fd2
@yhaliaw
Refactor runner manager one runner fixture
0f5b6ce
@yhaliaw
Fix error string formatting
d0a3173
@yhaliaw
Adding the docstring for github_runner_manager
dd14f44
@yhaliaw
Fix test fixture scope
8cc1325
@yhaliaw
Add docstring on cloud_runner_manager
fa0313a
jdkandersson
jdkandersson reviewed Sep 5, 2024
View reviewed changes
Copy link
Contributor

@jdkandersson jdkandersson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • for utilities let's import them from the package rather than duplicating
  • For repo policy compliance lets also import rather than duplicate

pyproject.toml Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
src-docs/charm_state.md Outdated Show resolved Hide resolved
src-docs/errors.md Outdated Show resolved Hide resolved
src-docs/github_client.md Outdated Show resolved Hide resolved
@cbartz
Merge branch 'main' into feat/externalise-runner-creation-ISD-1536
2c86775 
# Conflicts:
#	pyproject.toml
#	src-docs/errors.md
#	src/charm.py
#	src/charm_state.py
#	src/github_client.py
#	src/manager/cloud_runner_manager.py
#	src/manager/github_runner_manager.py
#	src/manager/runner_manager.py
#	src/metrics/github.py
#	src/openstack_cloud/openstack_runner_manager.py
#	src/runner.py
#	src/runner_manager_type.py
#	src/runner_type.py
#	tests/integration/test_runner_manager_openstack.py
#	tests/integration/test_self_hosted_runner.py
#	tests/unit/conftest.py
#	tests/unit/test_charm.py
#	tests/unit/test_charm_state.py
#	tests/unit/test_github_client.py
#	tests/unit/test_lxd_runner_manager.py
#	tests/unit/test_runner.py
@cbartz
fix merge
10a2f5c
@cbartz
remove reactive script
f16778c
@cbartz
Merge branch 'main' into feat/externalise-runner-creation-ISD-1536
b89e30d 
# Conflicts:
#	src/openstack_cloud/openstack_runner_manager.py
@cbartz
fix merge
34b4101
@cbartz
remove unused OpenstackUnauthorizedError
f293f8b
@cbartz
final new line
f3b926e
@cbartz
remove code duplication
5fc185d
@cbartz
remove duplicate src-docs
3e79e13
@cbartz
remove openstack-userdata.sh.j2
ed558eb
@cbartz cbartz added the complex label Sep 6, 2024
@cbartz cbartz changed the title WIP - Externalise Runner Manager Externalise Runner Manager Sep 9, 2024
jdkandersson
jdkandersson previously approved these changes Sep 9, 2024
View reviewed changes
yanksyoon
yanksyoon previously approved these changes Sep 9, 2024
View reviewed changes
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.

This seems like a lot of effort, thanks for the changes!

src/utilities.py Show resolved Hide resolved
tests/integration/test_runner_manager_openstack.py Show resolved Hide resolved
@cbartz cbartz dismissed stale reviews from yanksyoon and jdkandersson via 8533467 September 9, 2024 07:45
@cbartz cbartz marked this pull request as ready for review September 9, 2024 07:45
@cbartz cbartz requested a review from a team as a code owner September 9, 2024 07:45
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 9, 2024

Test coverage for 8533467

Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
src/charm.py                   587    148    149     26    72%   233-235, 301-320, 338-340, 341->345, 371-375, 448, 455-457, 484-489, 506-512, 533, 545-551, 565-566, 579, 584, 614-615, 617->626, 621->626, 631-637, 671, 675-680, 731-736, 745->748, 771-783, 787-788, 816-843, 856-861, 880-890, 904-906, 953-954, 956-957, 959-960, 1039->1041, 1106-1107, 1145-1147, 1155-1161, 1239-1292, 1296
src/charm_state.py             452     17     96      5    95%   271-283, 508-512, 634-635, 690-691, 1126->1129, 1133-1134, 1181
src/errors.py                   25      0      0      0   100%
src/event_timer.py              54      7      2      1    86%   105-106, 131, 148-149, 165-166
src/firewall.py                 51     18     20      0    61%   42-43, 66-69, 111-185
src/github_client.py            23      2      6      1    90%   66->exit, 71-72
src/logrotate.py                43      0      2      0   100%
src/lxd_type.py                 35      0      2      0   100%
src/runner_manager_type.py      51      0      8      0   100%
src/runner_type.py              38      0     10      0   100%
src/shared_fs.py                93     17     10      1    83%   52-53, 120-121, 146-147, 155-156, 162-163, 181, 184-185, 197-198, 241-242
src/utilities.py                32      4      6      2    79%   66-69, 111
------------------------------------------------------------------------
TOTAL                         1484    213    311     36    83%

Static code analysis report

Run started:2024-09-09 07:47:13.656154

Test results:
  No issues identified.

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

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 0292abb into main Sep 9, 2024
40 checks passed
@cbartz cbartz deleted the feat/externalise-runner-creation-ISD-1536 branch September 9, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdkandersson jdkandersson jdkandersson approved these changes

@yanksyoon yanksyoon yanksyoon approved these changes

@yhaliaw yhaliaw Awaiting requested review from yhaliaw

Assignees
No one assigned
Labels
complex Libraries: Out of sync
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cbartz @yanksyoon @jdkandersson @yhaliaw