brbzull0 opened a new pull request, #13842:
URL: https://github.com/apache/trafficserver/pull/13842
Follow-up to #13032. Fixes #13832.
`traffic_ctl --error-level` (#13032) grades a JSONRPC handler failure by the
severity of its annotations. A `config reload` outcome is not a JSONRPC error:
it comes back inside a successful response, as a task state plus task logs, so
the level did not apply to it. On master, `config reload --monitor` exits 2
when the top-level task ends `fail` or `timeout` and 0 on `success`, whatever
its log entries say, and `config status` exits 0 for every reload it shows.
This PR grades a finished reload by its task states and log entries, in
`config reload --monitor` and in `config status`. Nothing changes on the server
or the wire.
### What it does
- **The grade** (`appExitCodeFromReload` in `TrafficCtlStatus.h`): the most
severe log entry of the task and of its sub-tasks, at any depth, is compared
with `--error-level`.
- The state of each task bounds its entries. A task that ended `fail` or
`timeout` counts as at least `error`, also when it logged nothing. The entries
of any other task count as at most `warn`, since its handler did not fail it.
With the default level the exit status therefore follows the task states: 2
when any task, at any depth, ended `fail` or `timeout`.
- A task that ended `timeout` has no outcome: the progress checker marked
it after `proxy.config.admin.reload.timeout`, and its handler can still finish
afterwards. So a reload with such a task exits 75 (EX_TEMPFAIL) below the
level, not 0.
- An entry without a level is not graded: the one-argument `ctx.log(text)`
entries, the reason traffic_server logs when it marks a reload `timeout`, and
the plain-string logs of a traffic_server built before #13090. An entry whose
level is not a known level (an integer out of range, or not an integer) counts
as `emergency`. It is printed as before.
- A reload that is still running, has no state, or is in a state
`traffic_ctl` does not know, is not graded.
- **`config reload --monitor`** grades the first finished state it sees. The
`Details :` hint is printed when the reload did not end `success`, or when its
grade is not 0, which includes 75 for a sub-task that ended `timeout`.
`--monitor` prints text whatever `-f` says, as on master.
- **`config status`** grades the newest reload it shows. With `--count N`,
the older reloads are history and do not change the exit status. `--min-level`
still only filters what is printed: the grade covers every entry with a level,
shown or hidden. `-f json` is graded the same as text.
- **Inline reloads (`--data`)**: the exit status now depends on
`--error-level`, so the "not persisted to disk" note follows how the reload
ended instead.
- Without `--monitor`, it is printed once the reload is scheduled.
- With `--monitor`, it is printed when the reload ended `success`,
whatever its grade, or was still running when the monitor stopped, and not when
it ended `fail` or `timeout`. When the monitor cannot read the state (a status
poll failed, or the reply had no task), it is printed unless the exit status is
2, as before.
- It is not printed when the server refused the request because another
reload was in progress.
How the open points of #13832 were decided:
| Open point | Decision |
|---|---|
| A `fail` task with no entry at or above the level, or no entries | Counts
as at least `error`: exits 2 at the default level. `--error-level fatal` lets
it exit 0 unless a task that failed logged `fatal` or above. |
| `timeout` | Same as `fail`, and 75 instead of 0 below the level. |
| Sub-task entries; `DL_Undefined` | Sub-task entries count, at any depth.
Entries without a level are not graded. |
| `config status --count N` | The newest reload shown decides. |
| `--min-level` and `--error-level` | `--min-level` filters the display
only. |
| (not in the issue) A `success` task with `error` or worse entries | Counts
as at most `warn`: exits 0 at the default level, 2 at `--error-level warn`. Its
handler chose to complete, as `cache.config` does when it drops a bad line. |
### Behaviour changes at the default level
- `config status` exits 2 when the newest reload it shows ended `fail` or
`timeout`. It used to exit 0, so a script that runs it to print the report must
now accept 2. Eight runs in four AuTests under `tests/gold_tests/jsonrpc/`
asserted 0 for that case and now assert 2.
- `config status` and `config reload --monitor` also exit 2, and `--monitor`
prints the `Details :` hint, for a reload that reports `success` while one of
its sub-tasks ended `fail` or `timeout`. traffic_server does not report such a
reload: its parent task takes the worst state of its sub-tasks. Another JSONRPC
node could.
- The inline-reload note is no longer printed when the server refused the
request because another reload was in progress.
For the reloads traffic_server reports, `config reload --monitor`, and so
the `AddConfigReload` test helper, keep their exit status: 0 for `success`,
also with `[Err]` entries, and 2 for `fail` or `timeout`. There, 75 comes from
the monitor's own `--timeout`, or from a request it depends on that failed
below the level (#13032), as before. The first two changes are listed in
`doc/release-notes/upgrading.en.rst`.
### What this PR does not do
- `config reload` without `--monitor`, and with `--show-details`, is not
graded. It returns once the reload is scheduled, or after one snapshot taken
after `--initial-wait`, before there is an outcome. `--monitor` is the way to
wait for one.
- A reload that is still running, has no state, or is in an unknown state is
not graded. `config status` exits 0 for it, not 75. `--monitor` keeps polling
it until `--timeout`, as on master.
- No line explains why a graded reload failed. `config status` prints the
entries as before, minus those `--min-level` hides, and `--monitor` prints the
`Details :` hint.
- Some traffic_server warnings carry no level in the task log, so they are
not graded. The `records.yaml` handler logs its errata with `ctx.log("{}",
zret)`, which stores no level, so `--error-level warn` does not fail a
`records.yaml` reload that only logged warnings. Logging them with their level
is a server change, left for a follow-up.
- After `config reload -D` without `--data`, the "not persisted to disk"
note is still printed, as on master: `-D` writes into the same `configs` node
as `--data` (#13110), and the note is keyed on that node. `virtualhost.yaml`
(#13108) is the first core handler that reads a directive, so `-D` now has a
core use. The fix needs a flag of its own for `--data` content, separate from
the grade; left for a follow-up.
- `config reload --monitor` refused because another reload is in progress
follows that other reload and exits with its grade, as on master it exited with
its state. With `-d` it can exit 0 although the content was refused; only the
note now says nothing was applied.
### Tests
- `src/traffic_ctl/unit_tests/test_traffic_ctl_status.cc`: 12 new `[reload]`
cases: the grade, the bounds of each state, sub-tasks at any depth (including
"A sub-task that failed counts as at least an error, whatever its parent
says"), timeouts, unfinished and unknown states, malformed levels, and decoding
the level.
- `tests/gold_tests/traffic_ctl/traffic_ctl_reload_error_level.test.py`
(new, 71 runs):
- against traffic_server: a `cache.config` reload that rejects a line
(`success` with an `[Err]` entry), a clean reload, and a directory in place of
`cache.config` (`fail`);
- against a second traffic_server with the `cfg_plugin_deferred_test`
plugin and `proxy.config.admin.reload.timeout` at 1 s, so the reload ends
`timeout`;
- against the stand-in JSONRPC node from #13032 (`jsonrpc_error_stub.py`),
for task trees traffic_server does not produce on demand: levels out of range
or not integers, unknown states, sub-tasks that failed or timed out under a
parent that succeeded, `--count`, poll errors, and the inline-reload note.
- The four reload tests under `tests/gold_tests/jsonrpc/`: `config status`
on a failed reload now expects 2.
- Red/green: with the `src/traffic_ctl` sources reverted to master and the
tests kept, the new test and the four updated ones fail. 47 runs fail, 8 in the
four updated tests and 39 in the new one: 46 on the exit code, and one on the
inline note. With the change, all five pass.
- Mutation: two passes over earlier versions of the change, 39 and 46
mutants. Each mutant that survived got a run in the new AuTest that killed it
at the time. The last fix round came after both passes and reworked the
inline-note condition and the clamping of out-of-range levels; no mutation pass
has run on those lines.
### Docs
- `traffic_ctl.en.rst`: how a reload is graded, under `--error-level`; the
`config reload` exit codes; a new exit-code list for `config status`;
`--min-level`; the `--data` note.
- `upgrading.en.rst`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
