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

Add initial documentation for reactive mode #397

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented Oct 24, 2024

Applicable spec: ISD-116

Overview

Add a howto and some reference documentation.

Rationale

We have an initial deployment in production and need some documentation.

Checklist

  • The charm style guide was applied.
  • The contributing guide was applied.
  • The changes are compliant with ISD054 - Managing Charm Complexity
  • The documentation is generated using src-docs.
  • The documentation for charmhub is updated.
  • The PR is tagged with appropriate label (urgent, trivial, complex).
  • The changelog is updated with changes that affects the users of the charm.
  • The changes do not introduce any regression in code or tests related to LXD runner mode.

@cbartz cbartz added the documentation Improvements or additions to documentation label Oct 24, 2024
docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
@cbartz cbartz marked this pull request as ready for review November 14, 2024 08:38
@cbartz cbartz requested a review from a team as a code owner November 14, 2024 08:38
@cbartz cbartz requested a review from yhaliaw November 14, 2024 08:38
Copy link
Contributor

@erinecon erinecon left a comment

Choose a reason for hiding this comment

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

I approve of the doc changes 👍
(Not giving an official approval since tox.ini was also changed.)

docs/how-to/reactive.md Outdated Show resolved Hide resolved
docs/how-to/reactive.md Outdated Show resolved Hide resolved
Copy link
Contributor

Test coverage for 1be246b

Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
src/charm.py                   655    153    140     28    73%   254-256, 322-341, 359-361, 362->366, 392-396, 470-472, 481, 488-490, 511-516, 533-538, 559, 571-577, 592-593, 612-613, 622, 627, 657-658, 660->669, 664->669, 674-681, 715, 719-724, 776, 788->791, 814-826, 830-831, 864-865, 877-894, 918-920, 939-949, 964-966, 1029-1030, 1032-1033, 1035-1036, 1115->1117, 1182-1183, 1221-1223, 1231-1239, 1315-1348, 1362-1367, 1382-1425, 1433-1434, 1456
src/charm_state.py             450     17     82      3    95%   274-286, 505-509, 631-632, 687-688, 1123->1126, 1130-1131, 1178
src/errors.py                   25      0      0      0   100%
src/event_timer.py              52      6      0      0    88%   105-106, 143-144, 160-161
src/firewall.py                 51     18     10      0    67%   42-43, 66-69, 111-185
src/github_client.py            23      2      4      0    93%   71-72
src/logrotate.py                43      0      2      0   100%
src/lxd_type.py                 35      0      0      0   100%
src/runner_manager_type.py      39      0      0      0   100%
src/runner_type.py              38      0      0      0   100%
src/shared_fs.py                98     17     10      1    83%   60-61, 132-133, 162-163, 171-172, 178-179, 210, 213-214, 226-227, 270-271
src/utilities.py                32      4      6      2    79%   66-69, 111
------------------------------------------------------------------------
TOTAL                         1541    217    254     34    84%

Static code analysis report

Run started:2024-11-19 10:00:35.951057

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 5084
  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):


### GitHub Runner flavors

For this how-to-guide, we decided to have deployed three runner flavors: `large`, `large-arm`, `small` . We need
Copy link
Collaborator

@javierdelapuente javierdelapuente Nov 19, 2024

Choose a reason for hiding this comment

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

I find it confusing the concept of runner flavor. A runner (an openstack intance) will be associated to a flavor, but we will deploy a github-runner application, that will manage runners with that flavor (at least for now, until github-runner applications handle several flavors).

The routing table, as far as I understand, routes labels to github-runner applications (of course, if they are not correctly configured it will not work correctly).

Copy link
Collaborator

@javierdelapuente javierdelapuente Nov 19, 2024

Choose a reason for hiding this comment

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

Also the architecture, not sure is that is related to an openstack flavor, but related to the type of image and hypervisors. If that is the case, then architecture is just part of the concept of flavor, but different to openstack flavor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@javierdelapuente runner flavor == runner application. Would a rename to github runner application resolve your confusion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that will make it clearer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation Libraries: Out of sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants