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

Use previous token to flush runners #218

Merged
merged 8 commits into from
Feb 7, 2024
Merged

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented Feb 6, 2024

Overview

Use the previously saved token to remove existing runners. If the previous token has expired, use a force removal (ignoring gh api errors and just removing lxd instances).

In addition, any invalid token errors in the charm code will be caught and the charm will go into BlockedStatus, requiring an operator to update the token. A message is included in the BlockedStatus, so this PR should fix #89.

Rationale

If the new token on config changed is no longer valid for the current organisation/repo, the removal of existing runners may fail (due to HTTP 403 Forbidden).

Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-github-runner-1/charm/./src/charm.py", line 926, in <module>
    main(GithubRunnerCharm)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ops/main.py", line 456, in main
    _emit_charm_event(charm, dispatcher.event_name)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ops/main.py", line 144, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ops/framework.py", line 351, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ops/framework.py", line 853, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ops/framework.py", line 943, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/./src/charm.py", line 78, in func_with_catch_errors
    func(self, event)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/./src/charm.py", line 505, in _on_config_changed
    prev_runner_manager.flush(FlushMode.FORCE_FLUSH_BUSY_WAIT_REPO_CHECK)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/src/runner_manager.py", line 564, in flush
    remove_token = self._clients.github.get_runner_remove_token(self.config.path)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/src/github_client.py", line 118, in get_runner_remove_token
    token = self._client.actions.create_remove_token_for_repo(
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ghapi/core.py", line 62, in __call__
    return self.client(self.path, self.verb, headers=headers, route=route_p, query=query_p, data=data_p)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/ghapi/core.py", line 121, in __call__
    res,self.recv_hdrs = urlsend(path, verb, headers=headers or None, debug=debug, return_headers=True,
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/fastcore/net.py", line 218, in urlsend
    return urlread(req, return_json=return_json, return_headers=return_headers)
  File "/var/lib/juju/agents/unit-github-runner-1/charm/venv/fastcore/net.py", line 119, in urlread
    if 400 <= e.code < 500: raise ExceptionsHTTP[e.code](e.url, e.hdrs, e.fp, msg=e.msg) from None
fastcore.net.HTTP403ForbiddenError: HTTP Error 403: Forbidden
====Error Body====
{
  "message": "Must have admin rights to Repository.",
  "documentation_url": "https://docs.github.com/rest/actions/self-hosted-runners#create-a-remove-token-for-a-repository"
}

Juju Events Changes

config_changed event is adapted

Module Changes

Library Changes

Checklist

@cbartz cbartz added bug Something isn't working trivial labels Feb 6, 2024
merkata
merkata previously approved these changes Feb 6, 2024
@cbartz cbartz marked this pull request as ready for review February 6, 2024 16:42
@cbartz cbartz requested a review from a team as a code owner February 6, 2024 16:42
@cbartz cbartz requested a review from merkata February 7, 2024 09:22
Copy link

@gtrkiller gtrkiller 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

github-actions bot commented Feb 7, 2024

Test coverage for 0935ad0

Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
src/charm.py                   468    104    121     24    76%   87-88, 95-96, 120-121, 125-127, 167-169, 178->190, 188, 224-243, 257-259, 260->264, 288-292, 323, 403-408, 431, 448-449, 463-464, 477-500, 524-527, 534, 542, 558, 562-567, 612->615, 628-629, 631-632, 666-673, 697-698, 731-732, 760-761, 763-764, 766-767, 841->843, 904-905, 921, 942-944, 948-951, 955
src/charm_state.py             138     18     22      3    86%   131-138, 221-224, 266-268, 279-280, 286-291, 302-304
src/errors.py                   41      0      0      0   100%
src/event_timer.py              63      9      6      1    86%   94-95, 103-104, 126, 143-144, 160-161
src/firewall.py                 52     18     20      0    61%   42-43, 66-69, 110-184
src/github_client.py            89     14     38      4    80%   38-45, 146, 169, 182-189, 207->242, 236
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     98     25    73%   46->48, 130, 183->197, 237-238, 264-265, 306-315, 339-344, 349, 369, 373-383, 432->435, 438-440, 447, 461, 471, 475, 477, 492, 526-531, 547-560, 571-610, 615, 653, 706-708, 712, 730, 765, 791, 796-808, 822, 827->829, 832-834
src/runner_logs.py              35      2      6      1    93%   62->61, 66-67
src/runner_manager.py          308     76    108      9    73%   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, 566-571, 592-597, 609-616, 711-724, 734, 739-750
src/runner_manager_type.py      40      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, 175, 228
------------------------------------------------------------------------
TOTAL                         2097    344    507     77    81%

Static code analysis report

Run started:2024-02-07 07:20:43.469076

Test results:
  No issues identified.

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

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 c9f6bfa into main Feb 7, 2024
57 checks passed
@cbartz cbartz deleted the ISD-1563-token-change-error branch February 7, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Libraries: OK trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

if token has insufficient permissions, not clearly reflected in juju status
4 participants