On 1/9/24 23:49, Ilya Maximets wrote: > The original problem was summarized on the OVS+OVN Conf'22 last year: > https://www.openvswitch.org/support/ovscon2022/#t19 > Slides: > https://www.openvswitch.org/support/ovscon2022/slides/ovsdb-a-database.pdf > > In short, there are way to many ways to configure remotes and databases > but not a single one of them can cover all the situations. And there > are cases like max-backoff on active-backup connection that are not > configurable at all. > > The proposed solution for this problem was to unify configurations > within a new Local_Config database that will be local to a server > and will have support for all we need in the schema. > > However, there are few issues with this solution: > > 1. It doesn't reduce the number of ways things can be configured, > but increases, because we'll need to add things that are covered > by other methods to the database, while those other methods > are still in use. And it is actually required to use the > command line or appctl in order to use the values stored in the > database, i.e. --remote=db:db:table:row. > > 2. Not particularly user-friendly. Database files are user-readable > -ish, but not writable, i.e. require special tools in order to > make changes and in most cases require a running ovsdb-server > in order to make changes. > > 3. We need a way to restrict write access for external users, > but allow local administrators to change the configuration. > Hence, potentially complex access management and potential > security issues. > > Taking all of that into account and spending another year on a > solution :) , I believe, introduction of a configuration file can > make things easier. This patch set adds support for a new command > line argument --config-file that is mutually exclusive with many > other command line arguments and appctl commands that can configure > the same thing. > > A few key points: > > 1. Configuration file is a plain text JSON file, so it can be edited > with just a text editor without need for any extra tools. > (JSON because we have a decent JOSN parser and there is no need > for extra dependencies. And no other format is actually superior, > all of them have their own issues.) > > 2. Configuration file is a replacement for many command line options > and appctls, because it is mutually exclusive with them. > If --config-file is set, configuration that can be done via > configuration file can only be done via configuration file. > (It still allows to point to the database for remotes, but there > is no actual need for this, except in very rare cases, and it is > visible from the file that it points to the database.) > > 3. If configuration change is needed in runtime: change the file > and call ovsdb-server/reload. Ihar also suggested to use SIGHUP > for this, can be added in the future. > > > SSL configuration is not moved to a config-file in this patch set, > but can be done as a follow up. > > > Structure of this patch set: > > - v1 contained 5 bug and test fixes, these are no longer included. > (already applied) > > - There are only 4 actually large patches that are not tests. > And they are mainly refactoring of the existing code without > externally visible changes. This is because we have to > isolate all the databases from each other so they do not > share configuration. But at the same time we have to preserve > all the current interfaces for backward compatibility and > that takes a lot of code. > > - Most other patches in the set a small simple changes. > > More description and examples are in individual patches and the > documentation. > > TL;DR; A small example of a config file from the patch #20: > > { > "remotes": { > "punix:db.sock": {}, > "pssl:6641": { > "inactivity-probe": 16000, > "read-only": false, > "role": "ovn-controller" > } > }, > "databases": { > "conf.db": {}, > "sb.db": { > "service-model": "active-backup", > "backup": true, > "source": { > "tcp:127.0.0.1:6644": null > } > }, > "OVN_Northbound": { > "service-model": "relay", > "source": { > "ssl:[fe:::1]:6642,ssl:[fe:::2]:6642": { > "max-backoff": 8000, > "inactivity-probe": 10000 > } > } > } > } > } > > > Version 2: > > * Dropped first 5 patches with various bug and test fixes since these > are already applied. > > * Added Ack from Mike on the now first two patches. > > * Documentation reworked to avoid suggesting the "service-model" > configuration for standalone and clustered databases to avoid > possible confusion. It's still allowed to specify the model in > case administrators will want ovsdb-server to assert a particular > service model. [Frode, Dumitru] > > * Added strict validation of the "service-model" option, i.e. the > parsing will fail in case of typos or unsupported service models, > e.g. "service-model": "backup" will fail the parsing, because > the model is named "active-backup". > > * Fixed a couple of typos in commit messages. > > > Ilya Maximets (17): > ovsdb: Allow database itself to be read-only. > jsonrpc-server: Add functions to convert jsonrpc options to/from json. > ovsdb: Track jsonrpc options per remote. > ovsdb: Extract relay string parsing into a separate function. > ovsdb: replication: Isolate databases from each other. > ovsdb: replication: Automatically switch read-only mode. > ovsdb-server: Database config isolation. > ovsdb-server: Add no-op config-file option. > jsonrpc-server: Re-add remotes on role changes. > jsonrpc: Add function to update all options at once. > ovsdb: Embed jsonrpc session options into ovsdb jsonrpc options. > ovsdb: replication: Allow to set all jsonrpc options. > ovsdb-cs: Add function to set all jsonrpc session options. > ovsdb: relay: Allow setting all jsonrpc session options. > ovsdb-server: Allow user-provided config files. > tests: ovsdb: Add relay and replication execution with config file. > tests: ovsdb: Add configuration tests with config file. > > Documentation/ref/ovsdb.7.rst | 86 +- > Documentation/topics/ovsdb-relay.rst | 19 + > NEWS | 4 + > lib/jsonrpc.c | 9 + > lib/jsonrpc.h | 8 + > lib/ovsdb-cs.c | 10 + > lib/ovsdb-cs.h | 3 + > ovsdb/jsonrpc-server.c | 124 ++- > ovsdb/jsonrpc-server.h | 18 +- > ovsdb/ovsdb-server.1.in | 96 +- > ovsdb/ovsdb-server.c | 1405 +++++++++++++++++++++----- > ovsdb/ovsdb.c | 2 + > ovsdb/ovsdb.h | 3 + > ovsdb/relay.c | 6 +- > ovsdb/relay.h | 4 +- > ovsdb/replication.c | 677 ++++++------- > ovsdb/replication.h | 37 +- > tests/ovsdb-server.at | 782 +++++++++++++- > 18 files changed, 2585 insertions(+), 708 deletions(-) >
Thanks, Dumitru, Frode, Mike, Terry and Simon for reviews! I fixed all the small nits from Dumitru (except for a couple marked 'up to you' :) ) and applied the set. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
