JSONRPC based interface for administrative API #7478

brbzull0 · 2021-02-03T19:27:51Z

Hello community, we have been talking about this for quite a while, so it's now an official PR.
Before gettint into any details just so you now We(yahoo) have this implementation currently running in production, it's been running for while now and so far no issues. So:

This PR contains several main things

A new RPC server which uses JSONRPC 2.0 as the underlying protocol. The server implements an IPC iterative style server, docs here
A new set of function handlers to serve the administrative JSONRPC API, docs here
A new traffic_ctl tool that uses jsonrpc as protocol behind the scenes, docs here
Unit test for basic protocol specs/rpc server.

Some other important documentation:

Basic Architecture notes(RPC, JSON Parsing, Protocol, Errors, etc) can be found here
Dev notes on how to implementing a new API handler, docs here
Dev notes on how to implementing new traffic_ctl commands. This still keeps the old input arguments and output, docs here. Can’t remember all the input args? No worries, you can use this which will give you bash completion for it.

A new configuration file is introduced:
jsonrpc.yaml

rpc:
   enabled: true
   unix:
      lock_path_name: /tmp/jsonrpc.lock
      sock_path_name: /tmp/jsonrpc.sock
      restricted_api: true

You can get the details here

Records API
Raw interaction with the RPC server using traffic_ctl(no specific api), here

All the documentation is online here I will keep this up to date with any changes.

Now, this draft contains several commits, but some of the newest commits contain significant changes that can invalidate the early ones, so I do not recommend reviewing commit by commit. Just look at it as a whole. We have been iteraing on this internally so (if any)I will keep pushing changes from our repo to this PR.

Few other things to consider:

Although you do not need it, we still have traffic_manager in this branch. If cloning this branch to play with, just start TS and use traffic_ctl as usual. 🤞 . Removing the traffic_manager code will be tackle in a different effort.
There is a new folder called mgmt2 which is where I’m putting all the code that is relevant to this new rpc system, also to start clean and avoid issues in the future when removing traffic_manager. I will be slowly moving what can/should be kept inside this new folder. At some point will revert the name to mgmt or something more relevant.
There is some code like FileManager.cc/h which was copy/paste from traffic_manager sources with some modifications in order to be reused. For instance FileManager now combines FileManager and ConfigManager together.
Existing management handler(mgmt_*) lives along with the new ones from the JSONRPC handlers, this is useful during dev so I can compare and see if both behave similarly.
JSONRPC protocol is not 100% honoured as the parsing library(yamlcpp) has some limitations when dealing with straight json.
Some of the validity checks when setting a new config variable was copy/paste (like 'recordValidityCheck') from the original source, there are better ways to achieve this, this code may change in the future.

Some known issues that I will be fixing.

There are some coupled modules that have dependencies from the current mgmt folder, this will be changed and if anything needs to be removed a proposal will be submitted eventually.

Breaking changes

You do not need to start traffic_manager anymore, so if you have scripts that runs traffic_manager they need to be updated.
traffic_ctl have minimal dependencies with ATS which make traffic_ctl not able to find the socket unless you specify it by either using runroot (--run-root) or using the default build location. It is recommended to use TS_RUNROOT env variable which make all the binaries use the same layout.
Some of the traffic_ctl server (restart, start, backtrace, etc) commands do not work because there is no traffic_manager running anymore. This may need to involve some scripting that can be executed in a different way or work with the OS system managers, like systemctl, etc, depending on the OS.

Review notes

I know this is a big PR, but a lot of the new code is documentation. I will highlight few things to help understanding the structure of this PR. So basically the entire JSORPC server and admin handler logic is inside the mgmt2/rpc folder where you will find the implementation for:

config JSONRPC node configuration
handlers Public Admin handlers implementation, this contains the implementation for admin rpc handlers(config, metrics, storage, etc). Things that you'd normally fetch using traffic_ctl.
jsonrpc JSONRPC 2.0 protocol implementation, this contains the JSONRPC Manager class which keeps track of the registered handlers and do the validation for requests and responses. It also contains the YAML codecs for reading and emitting JSON.
schema JSONRPC 2.0 schema definitions for the protocol and some of the exposed API.
server JSONRPC Server implementation, Unix Socket for now.

Do not review individual commits, as the idea is to squash them, some newer commits invalidate previous ones.

[Update Oct 7 2021]

traffic_top now works with the jsonrpc node directly.
traffic_crashlog removes the traffic_manager connectivity to work, things like server stack-race will not be included in the report. This may be added latter.
Added jsonrpc unit test extension to help testing jsonrpc handlers. This also includes some basic python autest to validate the JSONRPC 2.0 schemas.

[Update Nov 11 2021]

Convert this to a PR(previously Draft)

TODOs:

Unit test for the actual API handlers, this can/should be done using autest and either traffic_ctl or a new extension that connects directly to the rpc.
Change the rc/* scripts/templates to start only TS
~~Performance test, Improve some code~~. Ongoing
Plugin API proposal available here (we are using it in our internal TS)

You can find the original Issue created to tackle this effort #6633

Please feel free to clone it and try it out, any feedback would be appreciated.

Warning - I will be rebasing on top of master regularly, which means that I will need to perform a --force push, consider this when cloning

Thanks,
Damian

This closes #5563

jrushford · 2021-02-16T20:59:58Z

doc/admin-guide/jsonrpc/index.en.rst

+
+.. note::
+
+   As today, there is only 1 communication mechanism supported. IPC Sockets.


"As of today"

jrushford · 2021-02-16T21:00:42Z

doc/admin-guide/jsonrpc/index.en.rst

+Administrative API
+------------------
+
+This section describe how to interact with the administrative RPC API to interact with |TS|


"describes"

jrushford

Mostly just minor issues in the docs and some includes in the source. Testing looks good so far.

jrushford · 2021-02-16T21:04:41Z

doc/admin-guide/jsonrpc/index.en.rst

+
+
+
+   We get a valid record that was  found base on the passed criteria, ``proxy.config.exec_thread.autoconfig.sca*`` and the ``rec_type`` *1*.


maybe "was found based on"?

jrushford · 2021-02-16T21:12:29Z

doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst

+
+.. option:: input
+
+   Input mode, traffic_ctl will provide a control input from a stream buffer. Once the content was write the terminal :program:`traffic_ctl`


Maybe use "content is written"?

jrushford · 2021-02-16T21:12:56Z

doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst

+
+   Input mode, traffic_ctl will provide a control input from a stream buffer. Once the content was write the terminal :program:`traffic_ctl`
+   will wait for the user to press Control-D to send the request to the rpc endpoint.
+   This feature allow you to directly interact with the jsonrpc endpoint and test your API easily and without the need to know the low level


jrushford · 2021-02-16T21:14:26Z

doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst

+Using the JSONRPC mechanism
+^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+As a user, currently,  :program:`traffic_ctl` exercise this new protocol, please refer to the :ref:`traffic_ctl_jsonrpc` section.


"exercises"

jrushford · 2021-02-16T21:16:11Z

doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst

+
+.. note::
+
+   Although most of the protocol specs are granted, we do have implemented some exceptions. All the modifications we have implemented will


Change "we do have" to "we have"

jrushford · 2021-02-16T21:18:28Z

doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst

+******************************
+
+Use this section as a guide for developing new rpc methods inside |TS| and how to expose them through the |RPC| endpoint.
+Before we start is worth mentioning some of the architecture of the current implementation. The whole RPC mechanism is divided in


should "start is worth" be "start, it is worth"?

jrushford · 2021-02-16T21:19:44Z

doc/developer-guide/jsonrpc/jsonrpc-handler-development.en.rst

+^^^^^^^^^^^^^^
+
+
+There are several ways to deal with internal handler errors. Errors are expected to be send back to the client if the API was expressed that way


"send back" perhaps should be "sent back"

jrushford · 2021-02-16T21:22:32Z

doc/developer-guide/jsonrpc/traffic_ctl-development.en.rst

+
+
+   The way you print and the destination of the message is up to the developer's needs, either a terminal or some other place. If the response
+   from the server is a complex object, always can model the response with your own class or structure and use the built-in yamlcpp mechanism


maybe change to "you can always model the response"

jrushford · 2021-02-16T21:26:07Z

mgmt2/config/AddConfigFilesHere.cc

+
+#include "tscore/ink_platform.h"
+#include "tscore/Filenames.h"
+//#include "MgmtUtils.h"


remove the include if not needed.

jrushford · 2021-02-16T21:26:47Z

mgmt2/config/AddConfigFilesHere.cc

+#include "tscore/Errata.h"
+// #include <string_view>
+
+// extern FileManager *configFiles;


Remove this and the #include <string_view> if not needed.

jrushford · 2021-02-16T22:01:44Z

[approve ci clang-analyzer]

jrushford · 2021-02-16T22:28:02Z

@brbzull0 the clang analyzer fails building with the same error I got with devtoolset-9. Not sure why it still fails since you committed a fix for that.

brbzull0 · 2021-02-17T14:51:07Z

@brbzull0 the clang analyzer fails building with the same error I got with devtoolset-9. Not sure why it still fails since you committed a fix for that.

Yes, I'm working on this, thanks. Clang-analyzer seems not happy when ~~building libs~~ running make cleandist. As the new folder mgmt2 uses an object from another source folder, it tries to remove the shared one and it seems that it was already removed, so it fails. I will address this once I have a working solution.

ywkaras · 2021-03-01T20:05:34Z

src/traffic_ctl_jsonrpc/CtrlPrinters.cc

+{
+  std::string text;
+  auto my_print = [&text](auto const &disk) {
+    std::cout << ts::bwprint(text, "Device: {}\n", disk.path);


Why not just:

std::cout << "Device: " << disk.path << '\n';

Yes, for the sake of consistency I did that everywhere(in traffic_ctl), probably for legacy and simple output like this one I can just switch to the classic.

brbzull0 · 2021-03-02T16:36:11Z

src/traffic_ctl_jsonrpc/jsonrpc/RPCRequests.h

+  std::string
+  get_method() const
+  {
+    return "admin_clear_metrics_records";


Got this already, shouldn't be admin_clear_metrics_records. Will fix.

brbzull0 · 2021-03-04T14:45:21Z

[approve ci clang-analyzer]

ywkaras · 2021-03-09T23:50:44Z

doc/admin-guide/jsonrpc/index.en.rst

+   |TS| will start the |RPC| server without any need for configuration.
+
+
+In case a special configuration is needed, the following describes the structure.


Might be clearer to say non-default rather than special.

ywkaras · 2021-03-10T01:36:12Z

doc/admin-guide/jsonrpc/index.en.rst

+
+.. note::
+
+   Traffic Control does support this RPC mechanism for communication with |TS| . Please check :program:`traffic_ctl` documentation for


Do you mean to say "does not support" or "supports"?

ywkaras · 2021-03-10T01:43:30Z

doc/admin-guide/jsonrpc/index.en.rst

+.. include:: ../../common.defs
+
+.. highlight:: cpp
+.. default-domain:: cpp


github at least does not seem to understand default-domain , https://github.com/brbzull0/trafficserver/blob/jsonrpc20/wip/doc/admin-guide/jsonrpc/index.en.rst .

ywkaras · 2021-03-10T01:46:01Z

doc/admin-guide/jsonrpc/index.en.rst

+***********
+
+
+


Do multiple blank lines mean something in ReStructured Text? Or are you just missing the wide open spaces of The Pampas?

ywkaras · 2021-03-10T02:06:16Z

doc/appendices/command-line/traffic_ctl_jsonrpc.en.rst

+
+Mark down a host with `traffic_ctl` and view the associated host stats::
+
+   .. code-block:: bash


ywkaras · 2021-03-10T02:08:29Z

doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst

+.. |optional| replace:: ``optional``
+
+.. |arrayrecord| replace:: ``array[record]``
+.. |arrayerror| replace:: ``array[errors]``


Should these be in .defs include file.

…. This commit includes changes in the jsonrpc manager to allow different kind of handlers being implemented and registered, including handlers from plugins(API not included here). The core part of the manager was changed to use a single table for all the handler(previously one per handler type, method and notifications). Also includes a set of tools to allow writing unit tests around the jsonrpc API.

…ail and print to stderr in case of an error. (apache#560)

traffic.out used to be rotated by traffic_manager. Now that we transitioned to JSONRPC, traffic_manager no longer runs. This transitions log rotation handling to traffic_server if JSONRPC is running.

…a shared foler (apache#577) * [JSONRPC] - Move JSONRPC request/response definitions to a shared folder. - Split base request/response object into base objects and specific ones for traffic_ctl. - Add documentation. - Clean up some code in the RPC client code. * [JSONRPC] - rpc shared client, add invoke function that takes a JSONRPC object. * [JSONRPC] - fix typo * [JSONRPC] - rename namespace. Add default template type. * Fix typo in yaml_codecs.h Co-authored-by: Damian Meden <damian.meden@gmail.com>

…t clients be aware of the update logic. This also includes some tidy up on naming as well as making 'traffic_ctl config get' to display errors if they are back from the server. (apache#580)

…d test function.

- traffic_ctl: config describe/get to validate that the record is registered in ATS before showing any info - Protocol: record response - make record/error list always present even if contains no values. - Config: Make the jsonrpc socket permissions restricted by default.

…quest api. This change involves removing a hand call to traffic_ctl and using Request.admin_config_reload() instead. Also adjust the scripts to take the runroot file so it can be passed to traffic_ctl when executed.

The idea with this change is to expose the content of the bindings inside the FileManager singleton class. This commit also adds support to query this endpoint using traffic_ctl. Docs are also updated

This was making the build fail on FreeBSD.

…o carry the plugin responses to jsonrpc messages. (apache#614) This needs to be clear up after use otherwise the state may hold the old one which can end up giving the wrong response to rpc requests.

jrushford

Ran regression tests, unit tests and tested traffic_ctl commands. Looks good to me.

brbzull0 · 2022-01-24T18:46:31Z

[approve ci autest]

brbzull0 · 2022-01-24T18:48:44Z

There seems to be a transient issue when running this test on CI.
Could be the delay time, I get this pass locally.

Running Test regex_revalidate:............. Passed

Generating Report: --------------
Total of 1 test
  Unknown: 0
  Exception: 0
  Failed: 0
  Warning: 0
  Skipped: 0
  Passed: 1

SolidWallOfCode assigned SolidWallOfCode and brbzull0 and unassigned SolidWallOfCode Feb 3, 2021

SolidWallOfCode added Commands Core Manager labels Feb 3, 2021

SolidWallOfCode added this to the 10.0.0 milestone Feb 3, 2021

brbzull0 force-pushed the jsonrpc20/wip branch 3 times, most recently from d5a7f08 to 9d4b215 Compare February 8, 2021 17:15

jrushford requested review from bneradt, ezelkow1, jrushford and randall February 11, 2021 22:28

jrushford reviewed Feb 16, 2021

View reviewed changes

jrushford requested changes Feb 16, 2021

View reviewed changes

ywkaras reviewed Mar 1, 2021

View reviewed changes

brbzull0 commented Mar 2, 2021

View reviewed changes

brbzull0 force-pushed the jsonrpc20/wip branch from 5a28f2f to 87e1fd8 Compare March 3, 2021 12:22

ywkaras reviewed Mar 9, 2021

View reviewed changes

ywkaras reviewed Mar 10, 2021

View reviewed changes

brbzull0 and others added 17 commits January 24, 2022 16:33

[doc] - jsonrpc, update doc links.

706c89a

[traffic_ctl] - Make traffi_ctl binary to return non-zero code when f…

5fb9cc9

…ail and print to stderr in case of an error. (apache#560)

Fix compiler error. Match fwd with class definition (apache#566)

a99cbf6

Fix traffic.out rolling (apache#569)

64a90f5

traffic.out used to be rotated by traffic_manager. Now that we transitioned to JSONRPC, traffic_manager no longer runs. This transitions log rotation handling to traffic_server if JSONRPC is running.

[JSONRPC] - Add update_type to the set_record response in order to le…

6bd6465

…t clients be aware of the update logic. This also includes some tidy up on naming as well as making 'traffic_ctl config get' to display errors if they are back from the server. (apache#580)

Remove TM connectivity from traffic_crashlog (apache#583)

8279136

traffic_top - Make traffic_top work with the jsonrpc node. (apache#586)

b6b8932

Remove whitespaces from include/shared/rpc/README.md and remove unuse…

70b092d

…d test function.

JSONRPC:

1e6df5f

- traffic_ctl: config describe/get to validate that the record is registered in ATS before showing any info - Protocol: record response - make record/error list always present even if contains no values. - Config: Make the jsonrpc socket permissions restricted by default.

[JSONRPC] - Add new endpoint to fetch the FileManager registry content.

259d539

The idea with this change is to expose the content of the bindings inside the FileManager singleton class. This commit also adds support to query this endpoint using traffic_ctl. Docs are also updated

JSONRPC - doc. Fix doc indentation

6dd775a

JSONRPC - Add api doc link as reference for traffic_ctl

f794330

Fix unistd.h include so global functions gets correctly picked up.

333828b

This was making the build fail on FreeBSD.

Clean up the shared handler response data. This shared data is used t…

b96e697

…o carry the plugin responses to jsonrpc messages. (apache#614) This needs to be clear up after use otherwise the state may hold the old one which can end up giving the wrong response to rpc requests.

brbzull0 force-pushed the jsonrpc20/wip branch from e260f45 to b96e697 Compare January 24, 2022 16:42

jrushford approved these changes Jan 24, 2022

View reviewed changes

brbzull0 merged commit bc5b861 into apache:10-Dev Jan 25, 2022

brbzull0 added the JSONRPC JSONRPC 2.0 related work. label Jan 25, 2022

brbzull0 mentioned this pull request Jan 25, 2022

[JSONRPC] - Introduce TS Plugin API - Proposal #8630

Merged

zwoop modified the milestones: 10-Dev, 10.0.0 Feb 2, 2023

bryancall mentioned this pull request Aug 14, 2024

☂ ATS 10 Documentation #11677

Open

91 tasks

bryancall changed the title ~~JSONRPC based interface for administrative API.~~ JSONRPC based interface for administrative API Aug 15, 2024

brbzull0 mentioned this pull request Aug 16, 2024

Document JSON-RPC changes. #11701

Merged

brbzull0 added the Release Doc Done label Aug 16, 2024


		.. note::

		As today, there is only 1 communication mechanism supported. IPC Sockets.




		We get a valid record that was found base on the passed criteria, ``proxy.config.exec_thread.autoconfig.sca`` and the ``rec_type`` 1*.


		.. option:: input

		Input mode, traffic_ctl will provide a control input from a stream buffer. Once the content was write the terminal :program:`traffic_ctl`


		.. note::

		Although most of the protocol specs are granted, we do have implemented some exceptions. All the modifications we have implemented will

		^^^^^^^^^^^^^^


		There are several ways to deal with internal handler errors. Errors are expected to be send back to the client if the API was expressed that way



		The way you print and the destination of the message is up to the developer's needs, either a terminal or some other place. If the response
		from the server is a complex object, always can model the response with your own class or structure and use the built-in yamlcpp mechanism

		\|TS\| will start the \|RPC\| server without any need for configuration.


		In case a special configuration is needed, the following describes the structure.


		.. note::

		Traffic Control does support this RPC mechanism for communication with \|TS\| . Please check :program:`traffic_ctl` documentation for


		Mark down a host with `traffic_ctl` and view the associated host stats::

		.. code-block:: bash

		***********

JSONRPC based interface for administrative API #7478

JSONRPC based interface for administrative API #7478

Uh oh!

Conversation

brbzull0 commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR contains several main things

Some other important documentation:

Few other things to consider:

Some known issues that I will be fixing.

Breaking changes

Review notes

TODOs:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford commented Feb 16, 2021

Uh oh!

jrushford commented Feb 16, 2021

Uh oh!

brbzull0 commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brbzull0 commented Mar 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford left a comment

Choose a reason for hiding this comment

Uh oh!

brbzull0 commented Jan 24, 2022

Uh oh!

brbzull0 commented Jan 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

brbzull0 commented Feb 3, 2021 •

edited

Loading

brbzull0 commented Feb 17, 2021 •

edited

Loading