You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "masaori335 (via GitHub)" <gi...@apache.org> on 2026/07/01 22:39:18 UTC

[PR] ATS Configuration Reload with observability/tracing - Token model (#12892) [trafficserver]

masaori335 opened a new pull request, #13354:
URL: https://github.com/apache/trafficserver/pull/13354

   Backport #12892 
   
   ----
   
   ATS Configuration Reload with observability/tracing — Token model Replace the fire-and-forget configuration reload mechanism with a new token-based, observable reload framework. Every reload operation is now assigned a unique token, tracked through a task tree, and queryable via CLI or JSONRPC at any point after submission.
   
   Core components introduced:
   
   - ConfigRegistry: centralized singleton for config file registration, filename records, trigger records, and reload handlers. Replaces the scattered registration across AddConfigFilesHere.cc and individual modules.
   - ReloadCoordinator: manages reload session lifecycle including token generation, concurrency control (--force to override), timeout detection, and rolling history.
   - ConfigReloadTask: tracks a single reload as a tree of sub-tasks with per-handler status, timings, and logs.
   - ConfigContext: lightweight context passed to handlers providing in_progress(), complete(), fail(), log(), supplied_yaml(), and add_dependent_ctx(). Safe no-op at startup when no reload is active.
   - ConfigReloadProgress: periodic checker that detects stuck tasks and marks them as TIMEOUT.
   
   New traffic_ctl commands:
   
   - config reload [-m] [-t <token>] [-d @file] [--force]
   - config status [-t <token>] [-c all]
   
     All commands support --format json for automation and CI pipelines.
   
   New JSONRPC APIs:
   
   - admin_config_reload: unified file-based or inline reload with token, force, and configs parameters.
   - get_reload_config_status: query reload status by token or get the last N reloads.
   
   Migrated config handlers to ConfigRegistry: ip_allow, cache_control, cache_hosting, parent_proxy, split_dns, remap, logging, ssl_client_coordinator (with sni.yaml and ssl_multicert.config as dependencies), ssl_ticket_key, records, and pre-warm. Static configs (storage, volume, plugin, socks, jsonrpc) registered as inventory-only.
   
   Removed legacy ConfigUpdateHandler/ConfigUpdateContinuation from ConfigProcessor.h. Removed AddConfigFilesHere.cc in favor of per-module self-registration.
   
   Fixed duplicate handler execution for configs with multiple trigger records (e.g. ssl_client_coordinator) by deduplicating against the ConfigReloadTask subtask tree.
   
   Added RecFlushConfigUpdateCbs() to synchronously fire pending record callbacks after rereadConfig(), ensuring all subtasks are registered before the first status poll.
   
   New configuration records:
   - proxy.config.admin.reload.timeout (default: 1h)
   - proxy.config.admin.reload.check_interval (default: 2s)
   
   Backward compatible: existing `traffic_ctl config reload` works as before; internally it now uses the new framework with automatic token assignment and tracking.
   
   (cherry picked from commit 5bab268cb4c22e338d63a81f5da621a6b90776e6)
   
   Conflicts:
   	include/tscore/ArgParser.h
   	src/iocore/cache/P_CacheHosting.h
   	src/iocore/hostdb/CMakeLists.txt
   	src/proxy/ReverseProxy.cc
   	src/records/CMakeLists.txt
   	src/tscore/ArgParser.cc


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [10.2.x] ATS Configuration Reload with observability/tracing - Token model (#12892) [trafficserver]

Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13354:
URL: https://github.com/apache/trafficserver/pull/13354#issuecomment-4898145510

   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [10.2.x] ATS Configuration Reload with observability/tracing - Token model (#12892) [trafficserver]

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #13354:
URL: https://github.com/apache/trafficserver/pull/13354


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATS Configuration Reload with observability/tracing - Token model (#12892) [trafficserver]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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 &params)
+  {
+    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 &params)
+  {
+    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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org