brbzull0 commented on code in PR #13842:
URL: https://github.com/apache/trafficserver/pull/13842#discussion_r4348736759
##########
src/traffic_ctl/TrafficCtlStatus.h:
##########
@@ -126,3 +127,49 @@ appExitCodeFromResponse(shared::rpc::JSONRPCResponse const
&response)
return exit_severity_of(*worst) >= App_Exit_Level_Error ? CTRL_EX_ERROR :
CTRL_EX_OK;
}
+
+/// Exit status for a config reload task, graded against @c
App_Exit_Level_Error like a handler failure. The most severe log
+/// entry of the task and its sub-tasks decides. An entry without a severity
(DL_Undefined) is not graded, and one whose level
+/// the server sent malformed counts as the highest severity. The state of
each task bounds its entries: a task that ended
+/// `fail` or `timeout` counts as at least an error, the way an annotation
without a severity does, and the entries of any
+/// other task count as at most a warning, since its handler did not fail it.
A reload with a task that ended `timeout` exits
+/// CTRL_EX_TEMPFAIL below the level, since it never finished. A reload that
is still running, has no state, or is in a state
+/// this client does not know, is not graded.
+inline int
+appExitCodeFromReload(ConfigReloadResponse::ReloadInfo const &task)
+{
+ auto failed = [](ConfigReloadResponse::ReloadInfo const &info) { return
info.status == "fail" || info.status == "timeout"; };
+
+ if (task.status != "success" && !failed(task)) {
+ return CTRL_EX_OK;
+ }
+
+ bool timed_out = false;
+ auto worst = [&failed, &timed_out](auto &&self,
ConfigReloadResponse::ReloadInfo const &info) -> int {
+ int level = -1;
+ for (auto const &entry : info.logs) {
+ if (entry.unknown_level) {
+ level = std::max(level, static_cast<int>(DL_Emergency));
+ } else if (entry.level != DL_Undefined) {
+ level = std::max(level, static_cast<int>(entry.level));
+ }
+ }
+ if (failed(info)) {
+ level = std::max(level, static_cast<int>(DL_Error));
+ timed_out = timed_out || info.status == "timeout";
+ } else {
+ level = std::min(level, static_cast<int>(DL_Warning));
+ }
+ for (auto const &sub : info.sub_tasks) {
+ level = std::max(level, self(self, sub));
+ }
+ return level;
+ };
+
+ int const level = worst(worst, task);
+
+ if (level >= 0 && swoc::Errata::severity_type(level) >=
App_Exit_Level_Error) {
+ return CTRL_EX_ERROR;
+ }
+ return timed_out ? CTRL_EX_TEMPFAIL : CTRL_EX_OK;
Review Comment:
`DiagsLevel` and the errata severities are one scale by definition:
`include/tsutil/ts_errata.h` builds each `ERRATA_*` constant from its `DL_*`
value
([ts_errata.h#L32-L40](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/include/tsutil/ts_errata.h#L32-L40)).
The annotation grade from #13032 converts the same way
([TrafficCtlStatus.h#L102](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/src/traffic_ctl/TrafficCtlStatus.h#L102)),
so the reload grade keeps the direct conversion
([#L171](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/src/traffic_ctl/TrafficCtlStatus.h#L171)).
A separate mapping would restate that header.
##########
src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h:
##########
@@ -95,6 +95,31 @@ template <> struct
convert<FetchConfigReloadStatusRequest::Params> {
};
template <> struct convert<ConfigReloadResponse> {
+ /// A missing level leaves the entry DL_Undefined. A level that is not one
of the DiagsLevel values marks it @c unknown_level,
+ /// which the exit status grades as the highest severity. Such a level still
prints as it always did: an integer below or above
+ /// the known range as the lowest or the highest level, anything else
without a tag. The server sends the level quoted, as it
+ /// does every scalar.
+ static void
+ decode_level(Node const &level, ConfigReloadResponse::LogEntry &entry)
+ {
+ if (!level) {
+ return;
+ }
+ if (level.IsScalar()) {
+ try {
+ auto const value = level.as<int>();
+ if (value >= DL_Diag && value <= DL_Undefined) {
+ entry.level = static_cast<DiagsLevel>(value);
+ return;
+ }
+ entry.level = value < DL_Diag ? DL_Diag : DL_Emergency;
+ } catch (YAML::Exception const &) {
+ // Not an integer.
+ }
+ }
+ entry.unknown_level = true;
+ }
Review Comment:
Agreed. Inside a task that did not fail, an unknown level counts as at most
a warning, like every other entry there. The comment now says the state of the
task bounds it
([ctrl_yaml_codecs.h#L98-L101](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h#L98-L101)),
in 58c923178d1ec3cecde0baae1ef5fef2324a549d.
##########
src/traffic_ctl/CtrlCommands.cc:
##########
@@ -102,6 +102,21 @@ mark_not_done()
}
}
+/// Grade a reload into the exit status and return the grade, see
appExitCodeFromReload(). A failure overrides an earlier status,
+/// and a reload that never finished only replaces a success, the same as the
grade of any other response.
+int
+grade_reload(ConfigReloadResponse::ReloadInfo const &task)
+{
+ auto const code = appExitCodeFromReload(task);
+
+ if (code == CTRL_EX_ERROR) {
+ App_Exit_Status_Code = CTRL_EX_ERROR;
+ } else if (code == CTRL_EX_TEMPFAIL && App_Exit_Status_Code == CTRL_EX_OK) {
+ App_Exit_Status_Code = CTRL_EX_TEMPFAIL;
+ }
+ return code;
+}
Review Comment:
`grade_reload` already has internal linkage: it is inside the unnamed
namespace that opens at
[CtrlCommands.cc#L43](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/src/traffic_ctl/CtrlCommands.cc#L43)
and closes at
[#L146](https://github.com/apache/trafficserver/blob/58c923178d1ec3cecde0baae1ef5fef2324a549d/src/traffic_ctl/CtrlCommands.cc#L146),
with the other file-local helpers.
--
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]
