Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tarantool config storage #4056

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Tarantool config storage #4056

merged 2 commits into from
Mar 7, 2024

Conversation

andreyaksenov
Copy link
Contributor

@andreyaksenov andreyaksenov commented Feb 21, 2024

Updated topics:

New and updated samples used in topics and reference pages:

  1. Created a new centralized_config folder for samples related to centralized configurations.
  2. Moved the existing etcd sample (etcd) to this folder and renamed it to config_etcd.
  3. Removed the etcd_full sample. The updated config_etcd sample now includes additional settings (auth and timeout).
  4. Added the etcd_config_storage.sh script for quick setting up a protected etcd storage.
  5. Created the tarantool_config_storage sample that shows how to set up a Tarantool-based storage.
  6. Created the config_storage sample that shows how to connect to the Tarantool-based storage.
  7. Added the source.yaml file to a tt environment directory. Can be used to test tt cluster publish.
image

What's new in 3.0:

Reference:

@andreyaksenov andreyaksenov force-pushed the tarantool-config-storage branch 26 times, most recently from 8dc9157 to 12f10f9 Compare February 26, 2024 07:28
@andreyaksenov andreyaksenov linked an issue Feb 26, 2024 that may be closed by this pull request
@andreyaksenov andreyaksenov force-pushed the tarantool-config-storage branch 3 times, most recently from f8e49d2 to 3219f51 Compare February 26, 2024 12:14
@andreyaksenov andreyaksenov force-pushed the tarantool-config-storage branch 7 times, most recently from b16f435 to 462dedd Compare February 29, 2024 12:50
@andreyaksenov andreyaksenov marked this pull request as ready for review February 29, 2024 12:53
@andreyaksenov andreyaksenov force-pushed the tarantool-config-storage branch 7 times, most recently from 053bebe to 5e7e9e3 Compare February 29, 2024 14:29
Copy link

@askalt askalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! lgtm

@andreyaksenov andreyaksenov requested a review from p7nov March 6, 2024 07:07
Copy link
Contributor

@p7nov p7nov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job; some notes from my side regarding lack of explanations for some conceptual and specific things.


Tarantool enables you to store configuration data in one reliable place using `etcd <https://etcd.io/>`_.
Tarantool enables you to store configuration data in one reliable place, for example, a Tarantool or etcd-based configuration storage.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not for example - they are exact two supported ways of centralized config. Let's explain this clear from the start.


.. literalinclude:: /code_snippets/snippets/config/instances.enabled/etcd/config.yaml
3. Configure a connection to the storage by providing a local YAML configuration with an endpoint address and key prefix in the ``config`` section:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: there can also be other connection parameters, maybe say it in a more general wording?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sample shows this minimum configuration and works more like a demo version of a centralized storage.

@@ -1,94 +1,228 @@
.. _configuration_etcd:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this anchor to reflect the new content?


Tarantool enables you to store configuration data in one place using a Tarantool or etcd-based storage.
With a :ref:`local YAML configuration <configuration_file>`, you need to make sure that all cluster instances use identical configuration files:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like the explanation about the YAML file and the diagram with files on this page. Its a practical information for those who use local file config, while this page is not for them.
I think the proper place for the first diagram is the Configuration in a file section.
The centralized diagram can be used both on the general Configuration (Centralized section) page and here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the top block Configuration servers is left-aligned? I think the pic will look better with this block centered.

Copy link
Contributor Author

@andreyaksenov andreyaksenov Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the space for the potential diagram with TCM/tt/other tools (for example, tarantool_config_centralized_tcm.png, which is not used yet).


.. _centralized_configuration_storage_tarantool_interact:

Interacting with the storage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this section is here. I expected the section about setting up etcd storage right here.
This content seems relevant after at least one config is published to the storage.

Copy link
Contributor Author

@andreyaksenov andreyaksenov Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • dropped the Interacting with the storage and Watching configuration updates sections
  • moved the net.box-related content to a reference page
  • created a new Publishing configuration using the ‘config’ module section that acts as an entry point for the config.storage API

Publishing a cluster's configuration to etcd
--------------------------------------------
Publishing a cluster's configuration
------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An intro sentence about ways to publish?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope, section names are self-explanatory and the intro won't add any additional info.


.. code-block:: console

$ tt cluster publish "http://localhost:2379/example" instances.enabled/app/cluster.yaml
$ tt cluster publish "http://localhost:2379/myapp" source.yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that credentials are required in most cases, so let's maybe show them to save readers some time on reading the tt cluster reference?



.. _etcd_publishing_configuration_etcdctl:
.. _centralized_configuration_storage_publish_config_etcdctl:

Publishing configuration using etcdctl
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This maybe obvious but I'd start this section from an intro that it's only for etcd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because in the previous section we said that there is no difference for tt between etcd and Tarantool storage :)
Somebody can be confused.

Configuring connection to a storage
-----------------------------------

To use a centralized cluster's configuration, you need to provide connection settings in a local configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where I would put a thought from your intro: despite we are talking about the centralized config, it's the reader's job to keep these files consistent and available on all cluster hosts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads as centralized cluster's. Maybe rephrase to
To use a configuration from a centralized storage for your cluster... ?

Copy link
Contributor

@p7nov p7nov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved; just a couple more questions.

Tarantool-based storage
~~~~~~~~~~~~~~~~~~~~~~~

To make a replica set act as a configuration storage, use the built-in ``config.storage`` :ref:`role <configuration_reference_roles_options>`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this replicaset be used for storing data at the same time? If not, this should be stated explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, it can - it's just a regular replica set.

Configuring connection to a storage
-----------------------------------

To use a centralized cluster's configuration, you need to provide connection settings in a local configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads as centralized cluster's. Maybe rephrase to
To use a configuration from a centralized storage for your cluster... ?

@andreyaksenov andreyaksenov merged commit f56c25d into latest Mar 7, 2024
1 check passed
@andreyaksenov andreyaksenov deleted the tarantool-config-storage branch March 7, 2024 07:32
p7nov pushed a commit that referenced this pull request Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants