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 units with stuck reconciliation #367

Merged
merged 7 commits into from
Sep 11, 2024

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented Sep 9, 2024

Applicable spec: n/a

Overview

1 Add a timeout command to the juju reconciliation event dispatch. We use 10 times the reconciliation interval as a heuristic.
2. Add a panel showing the average reconciliation interval (note that this is not the same as reconciliation duration)

Screenshot from 2024-09-10 11-55-01

Screenshot from 2024-09-10 11-55-08

  1. Fix the wording of another panel (use the term "charm application" instead of "flavour").

Screenshot from 2024-09-10 12-00-54

Rationale

  1. to prevent units that are no longer performing periodic reconciliations due to https://bugs.launchpad.net/juju/+bug/2055184

  2. to help identify bad reconciliation scheduling.

Juju Events Changes

Reconciliation event now times out after 10 * reconciliation interval

Module Changes

charm.py: pass in 10 * reconciliation interval as timeout
event_timer: make timeout argument non-optional and don't calculate fallback

Library Changes

Checklist

@cbartz cbartz added bug Something isn't working trivial labels Sep 9, 2024
@cbartz cbartz added the documentation Improvements or additions to documentation label Sep 10, 2024
@cbartz cbartz changed the title WIP - Fix units with stuck reconciliation Fix units with stuck reconciliation Sep 10, 2024
@cbartz cbartz marked this pull request as ready for review September 10, 2024 10:14
@cbartz cbartz requested a review from a team as a code owner September 10, 2024 10:14
Copy link
Contributor

Test coverage for 402da2d

Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
src/charm.py                   588    148    149     26    72%   237-239, 305-324, 342-344, 345->349, 375-379, 452, 459-461, 488-493, 510-516, 537, 549-555, 570-571, 584, 589, 619-620, 622->631, 626->631, 636-642, 676, 680-685, 736-741, 750->753, 776-788, 792-793, 821-848, 861-866, 885-895, 909-911, 958-959, 961-962, 964-965, 1044->1046, 1111-1112, 1150-1152, 1160-1166, 1244-1297, 1301
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              52      6      0      0    88%   105-106, 143-144, 160-161
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                         1483    212    309     35    83%

Static code analysis report

Run started:2024-09-10 10:15:02.659019

Test results:
  No issues identified.

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

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.

Doc changes look good to me 👍

@cbartz cbartz merged commit 0e891cc into main Sep 11, 2024
40 checks passed
@cbartz cbartz deleted the fix/unit-no-longer-performing-reconciliation-ISD-2172 branch September 11, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation Libraries: Out of sync trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants