Copilot commented on code in PR #13354:
URL: https://github.com/apache/trafficserver/pull/13354#discussion_r3509437696
##########
src/traffic_ctl/jsonrpc/ctrl_yaml_codecs.h:
##########
@@ -41,6 +41,107 @@ template <> struct convert<ConfigSetRecordRequest::Params> {
return node;
}
};
+//------------------------------------------------------------------------------------------------------------------------------------
+
+template <> struct convert<ConfigReloadRequest::Params> {
+ static Node
+ encode(ConfigReloadRequest::Params const ¶ms)
+ {
+ Node node;
+ if (!params.token.empty()) {
+ node["token"] = params.token;
+ }
+
+ if (params.force) {
+ node["force"] = params.force;
+ }
+
+ // Include configs if present (triggers inline mode on server)
+ if (params.configs && params.configs.IsMap() && params.configs.size() > 0)
{
+ node["configs"] = params.configs;
+ }
+
+ return node;
+ }
+};
+
+template <> struct convert<FetchConfigReloadStatusRequest::Params> {
+ static Node
+ encode(FetchConfigReloadStatusRequest::Params const ¶ms)
+ {
+ Node node;
+ auto is_number = [](const std::string &s) {
+ return !s.empty() && std::find_if(s.begin(), s.end(), [](unsigned char
c) { return !std::isdigit(c); }) == s.end();
+ };
+ // either passed values or defaults.
+ node["token"] = params.token;
+
+ if (!params.count.empty() && (!is_number(params.count) && params.count !=
"all")) {
+ throw std::invalid_argument("Invalid 'count' value, must be numeric or
'all'");
+ }
+
+ if (!params.count.empty()) {
+ if (params.count == "all") {
+ node["count"] = 0; // 0 means all.
+ } else {
+ node["count"] = std::stoi(params.count);
+ }
+ }
+
+ return node;
+ }
+};
+
+template <> struct convert<ConfigReloadResponse> {
+ static bool
+ decode(Node const &node, ConfigReloadResponse &out)
+ {
+ auto get_info = [](auto &&self, YAML::Node const &from) ->
ConfigReloadResponse::ReloadInfo {
+ ConfigReloadResponse::ReloadInfo info;
+ info.config_token = helper::try_extract<std::string>(from,
"config_token");
+ info.status = helper::try_extract<std::string>(from, "status");
+ info.description = helper::try_extract<std::string>(from,
"description", false, std::string{"<none>"});
+ info.filename = helper::try_extract<std::string>(from, "filename",
false, std::string{"<none>"});
+ for (auto &&log : from["logs"]) {
+ info.logs.push_back(log.as<std::string>());
+ }
+
+ for (auto &&sub : from["sub_tasks"]) {
+ info.sub_tasks.push_back(self(self, sub));
+ }
+
+ if (auto meta = from["meta"]) {
+ info.meta.created_time_ms = helper::try_extract<int64_t>(meta,
"created_time_ms");
+ info.meta.last_updated_time_ms = helper::try_extract<int64_t>(meta,
"last_updated_time_ms");
+ info.meta.is_main_task = helper::try_extract<bool>(meta,
"main_task");
+ }
+ return info;
+ };
+
+ // Server sends "errors" (plural)
+ if (node["errors"]) {
+ for (auto &&err : node["errors"]) {
+ ConfigReloadResponse::Error e;
+ e.code = helper::try_extract<int>(err, "code");
+ e.message = helper::try_extract<std::string>(err, "message");
+ out.error.push_back(std::move(e));
+ }
+ }
+ out.created_time = helper::try_extract<std::string>(node, "created_time");
+ for (auto &&msg : node["message"]) {
+ out.messages.push_back(msg.as<std::string>());
+ }
+ out.config_token = helper::try_extract<std::string>(node, "token");
+
+ for (auto &&element : node["tasks"]) {
+ ConfigReloadResponse::ReloadInfo task = get_info(get_info, element);
+ out.tasks.push_back(std::move(task));
+ }
Review Comment:
ConfigReloadResponse YAML decode iterates node["message"] and node["tasks"]
without checking if those keys exist. Server responses from
get_reload_config_status (and some error paths) omit these fields, and YAML-cpp
will throw when range-iterating an undefined node, causing traffic_ctl to
terminate instead of reporting the RPC error/status. Guard these loops with `if
(node[...])` checks.
##########
doc/developer-guide/jsonrpc/jsonrpc-api.en.rst:
##########
@@ -951,18 +991,264 @@ Request:
{
"id": "89fc5aea-0740-11eb-82c0-001fc69cc946",
"jsonrpc": "2.0",
- "method": "admin_config_reload"
+ "method": "admin_config_reload",
+ "params": {
+ "token": "deploy-v2.1"
+ }
}
Response:
-The response will contain the default `success_response` or a proper rpc
error, check :ref:`jsonrpc-node-errors` for mode details.
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "token": "deploy-v2.1",
+ "message": ["Reload task scheduled"],
+ "created_time": "2025 Feb 17 12:00:00"
+ },
+ "id": "89fc5aea-0740-11eb-82c0-001fc69cc946"
+ }
+
+
+**Inline reload (with ``configs``):**
+
+The ``configs`` parameter is a map where each key is a **registry key** (e.g.
``ip_allow``, ``sni``)
+and each value is the **config content** — the inner data, not the full file
structure. For example,
+``ip_allow.yaml`` on disk wraps rules under an ``ip_allow:`` top-level key,
but when injecting via
+RPC, you pass the rules array directly. The handler receives this content via
+``ctx.supplied_yaml()`` and is responsible for using it without the file's
wrapper key.
+
+Request:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "id": "b1c2d3e4-0001-0001-0001-000000000001",
+ "jsonrpc": "2.0",
+ "method": "admin_config_reload",
+ "params": {
+ "token": "update-ip-and-sni",
+ "configs": {
+ "ip_allow": [
+ {
+ "apply": "in",
+ "ip_addrs": "0.0.0.0/0",
+ "action": "allow",
+ "methods": "ALL"
+ }
+ ],
+ "sni": [
+ {
+ "fqdn": "*.example.com",
+ "verify_client": "NONE"
+ }
+ ]
+ }
+ }
+ }
+
+Response:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "token": "update-ip-and-sni",
+ "message": ["Inline reload scheduled"],
+ "created_time": "2025 Feb 17 14:30:10"
+ },
+ "id": "b1c2d3e4-0001-0001-0001-000000000001"
+ }
+
+.. note::
+
+ Inline reload requires the target config handler to support
``ConfigSource::FileAndRpc``.
+ Handlers that only support ``ConfigSource::FileOnly`` will return an error
for the
+ corresponding key.
-Validation:
+**Reload already in progress:**
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "errors": [
+ {
+ "message": "Reload ongoing with token 'deploy-v2.1'",
+ "code": 1
Review Comment:
The example error code for "Reload already in progress" is shown as `1`, but
the implementation uses the ConfigReloadError range (e.g. RELOAD_IN_PROGRESS =
6004). The docs should match the actual code returned to clients.
##########
include/records/YAMLConfigReloadTaskEncoder.h:
##########
@@ -0,0 +1,69 @@
+/** @file
+
+ YAML encoder for ConfigReloadTask::Info — serializes reload task snapshots
to YAML nodes
+ for JSONRPC responses.
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#pragma once
+#include <string>
+#include <memory>
+#include <vector>
+#include <iostream>
+#include <swoc/Errata.h>
+#include <tscore/ink_platform.h>
+#include <yaml-cpp/yaml.h>
+
+#include "mgmt/config/ReloadCoordinator.h"
+
+namespace YAML
+{
+template <> struct convert<ConfigReloadTask::Info> {
+ static Node
+ encode(const ConfigReloadTask::Info &info)
+ {
+ Node node;
+ node["config_token"] = info.token;
+ node["status"] =
std::string(ConfigReloadTask::state_to_string(info.state));
+ node["description"] = info.description;
+ node["config_key"] = info.config_key;
+ node["filename"] = info.filename;
+ auto meta = YAML::Node(YAML::NodeType::Map);
+ meta["created_time_ms"] = info.created_time_ms;
+ meta["last_updated_time_ms"] = info.last_updated_time_ms;
+ meta["main_task"] = info.main_task ? "true" : "false";
+
Review Comment:
YAMLConfigReloadTaskEncoder encodes meta["main_task"] as the strings
"true"/"false". This makes the JSONRPC schema ambiguous for clients (string vs
boolean) and is inconsistent with the client-side decoder expecting a bool.
Encode this as a real boolean value.
##########
doc/developer-guide/jsonrpc/jsonrpc-api.en.rst:
##########
@@ -951,18 +991,264 @@ Request:
{
"id": "89fc5aea-0740-11eb-82c0-001fc69cc946",
"jsonrpc": "2.0",
- "method": "admin_config_reload"
+ "method": "admin_config_reload",
+ "params": {
+ "token": "deploy-v2.1"
+ }
}
Response:
-The response will contain the default `success_response` or a proper rpc
error, check :ref:`jsonrpc-node-errors` for mode details.
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "token": "deploy-v2.1",
+ "message": ["Reload task scheduled"],
+ "created_time": "2025 Feb 17 12:00:00"
+ },
+ "id": "89fc5aea-0740-11eb-82c0-001fc69cc946"
+ }
+
+
+**Inline reload (with ``configs``):**
+
+The ``configs`` parameter is a map where each key is a **registry key** (e.g.
``ip_allow``, ``sni``)
+and each value is the **config content** — the inner data, not the full file
structure. For example,
+``ip_allow.yaml`` on disk wraps rules under an ``ip_allow:`` top-level key,
but when injecting via
+RPC, you pass the rules array directly. The handler receives this content via
+``ctx.supplied_yaml()`` and is responsible for using it without the file's
wrapper key.
+
+Request:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "id": "b1c2d3e4-0001-0001-0001-000000000001",
+ "jsonrpc": "2.0",
+ "method": "admin_config_reload",
+ "params": {
+ "token": "update-ip-and-sni",
+ "configs": {
+ "ip_allow": [
+ {
+ "apply": "in",
+ "ip_addrs": "0.0.0.0/0",
+ "action": "allow",
+ "methods": "ALL"
+ }
+ ],
+ "sni": [
+ {
+ "fqdn": "*.example.com",
+ "verify_client": "NONE"
+ }
+ ]
+ }
+ }
+ }
+
+Response:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "token": "update-ip-and-sni",
+ "message": ["Inline reload scheduled"],
+ "created_time": "2025 Feb 17 14:30:10"
+ },
+ "id": "b1c2d3e4-0001-0001-0001-000000000001"
+ }
+
+.. note::
+
+ Inline reload requires the target config handler to support
``ConfigSource::FileAndRpc``.
+ Handlers that only support ``ConfigSource::FileOnly`` will return an error
for the
+ corresponding key.
-Validation:
+**Reload already in progress:**
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "errors": [
+ {
+ "message": "Reload ongoing with token 'deploy-v2.1'",
+ "code": 1
+ }
+ ],
+ "tasks": []
+ },
+ "id": "89fc5aea-0740-11eb-82c0-001fc69cc946"
+ }
+
+
+.. _get_reload_config_status:
+
+get_reload_config_status
+------------------------
+
+|method|
+
+Description
+~~~~~~~~~~~
-You can request for the record `proxy.process.proxy.reconfigure_time` which
will be updated with the time of the requested update.
+Query the status of configuration reloads. Can retrieve a specific reload by
token, or the last
+``N`` reloads from the history.
+
+Each reload status includes an overall result, a task tree with per-handler
status, durations,
+and optional log messages.
+
+
+Parameters
+~~~~~~~~~~
+
+All parameters are optional. If neither is provided, returns the most recent
reload.
+
+=================== =============
================================================================================================================
+Field Type Description
+=================== =============
================================================================================================================
+``token`` |str| Token of the reload to query. Takes
precedence over ``count`` if both are provided.
+``count`` ``number`` Number of recent reloads to return. Use
``0`` to return the full history. Default: ``1``.
+=================== =============
================================================================================================================
+
+``traffic_ctl`` maps ``--count all`` to ``count: 0`` in the RPC request. The
server keeps a
+rolling history of the last 100 reloads — when the limit is reached, the
oldest entry is evicted
+to make room for new ones. ``count: 0`` returns the full history, most recent
first.
+
+
+Result
+~~~~~~
+
+The response contains a ``tasks`` array. Each element represents a reload
operation with the following
+structure:
+
+======================= =============
===================================================================
+Field Type Description
+======================= =============
===================================================================
+``config_token`` |str| The reload token.
+``status`` |str| Overall status: ``success``, ``fail``,
``in_progress``, ``timeout``.
+``description`` |str| Human-readable description.
+``config_key`` |str| Registry key used for deduplication
(empty for main tasks).
+``filename`` |str| Associated filename (empty for the main
task).
+``meta`` ``object`` Metadata: ``created_time_ms``,
``last_updated_time_ms``, ``main_task``.
+``log`` ``array`` Log messages from the handler.
+``sub_tasks`` ``array`` Nested sub-tasks (same structure,
recursive).
+======================= =============
===================================================================
+
+
+Examples
+~~~~~~~~
+
+
+**Query a specific reload by token:**
+
+Request:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "id": "f1e2d3c4-0001-0001-0001-000000000001",
+ "jsonrpc": "2.0",
+ "method": "get_reload_config_status",
+ "params": {
+ "token": "deploy-v2.1"
+ }
+ }
+
+Response:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "tasks": [
+ {
+ "config_token": "deploy-v2.1",
+ "status": "success",
+ "description": "Main reload task - 2025 Feb 17 12:00:00",
+ "config_key": "",
+ "filename": "",
+ "meta": {
+ "created_time_ms": "1739808000123",
+ "last_updated_time_ms": "1739808000368",
+ "main_task": "true"
+ },
+ "logs": [],
+ "sub_tasks": [
+ {
+ "config_token": "deploy-v2.1",
+ "status": "success",
+ "description": "ip_allow",
+ "config_key": "ip_allow",
+ "filename": "/opt/ats/etc/trafficserver/ip_allow.yaml",
+ "meta": {
+ "created_time_ms": "1739808000200",
+ "last_updated_time_ms": "1739808000218",
+ "main_task": "false"
+ },
+ "logs": ["Finished loading"],
+ "sub_tasks": []
+ }
+ ]
+ }
+ ]
+ },
+ "id": "f1e2d3c4-0001-0001-0001-000000000001"
+ }
+
+
+**Query reload history (last 3):**
+
+Request:
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "id": "a1b2c3d4-0001-0001-0001-000000000002",
+ "jsonrpc": "2.0",
+ "method": "get_reload_config_status",
+ "params": {
+ "count": 3
+ }
+ }
+
+Response contains up to 3 entries in the ``tasks`` array.
+
+
+**Token not found:**
+
+.. code-block:: json
+ :linenos:
+
+ {
+ "jsonrpc": "2.0",
+ "result": {
+ "errors": [
+ {
+ "message": "Token 'nonexistent' not found",
+ "code": 4
Review Comment:
The "Token not found" example shows error code `4`, but the implementation
uses TOKEN_NOT_FOUND = 6001. Update the example so automation relying on the
numeric code sees the real value.
--
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]
