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

[20629] GUIDLess Discovery Server #4716

Merged
merged 51 commits into from
Jul 3, 2024

Conversation

cferreiragonz
Copy link
Contributor

@cferreiragonz cferreiragonz commented Apr 23, 2024

Description

This PR drastically changes Discovery Server behavior to be able to work without a fixed GUID.

  • Servers and Clients now connect between each other by exclusively specifying a locator, the GUID is not needed anymore. The remote server list has been replaced with LocatorList.

  • Hence, the server_id parameter used in the CLI and the positional arguments in the environment variable are deprecated. The -i parameter in the CLI has been kept in order to guarantee backward compatibility, but it is not mandatory any longer.

  • Servers now use a mesh topology, meaning that every server will directly connect with all of the available servers.

  • In order to accomplish last point, Servers now send the Data(p) of connected servers to new servers connections and send the Data(p) of the new server to all connected servers. To connect a new server to the mesh, it is only needed to specify the locator of an already connected server.

  • Servers only redirect discovery information of their direct clients, and will stop redirecting discovery information of the clients of other servers.

  • Remote Discovery servers connection list can now be updated at runtime without any restrictions. The previous list does not need to be a subset of the new one.

  • Servers now use the lease announcements routine to make connections with the remote servers specified in the remote Discovery servers list. Initial announcements will be called again every time a known server is dropped or the remote server list is updated.

  • Clients now accept connections of any server, not only the ones specified in their remote Discovery servers list. Although in normal behavior a client will only receive discovery information of the servers pinged by itself, if it receives a Data(p) of an other server (sending it manually) it will try to make a connection.

  • ServerAttributes is now private, as it is no longer needed to configure Clients or Servers

  • This PR is on top of [20936] Change backup restore order #4740 and must be merged after it

  • It also affects the python API: [20629] GUIDLess Discovery Server - Make ServerAttributes private Fast-DDS-python#147

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally: [20629] GUIDLess Discovery Server Discovery-Server#76
  • Any new/modified methods have been properly documented using Doxygen.
  • Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • ❌ Changes are ABI compatible.
  • ❌ Changes are API compatible. (RemoteServers are now specified by a LocatorList. Also, Fast DDS tool CLI and ROS_DISCOVERY_SERVER env. var. behavior have been modified)
  • New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation. Please uncomment following line with the corresponding PR to the documentation project: -->
  • N/A Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@cferreiragonz cferreiragonz added the needs-review PR that is ready to be reviewed label Apr 23, 2024
@cferreiragonz cferreiragonz added this to the v3.0.0 milestone Apr 23, 2024
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from dcea616 to 5209472 Compare April 23, 2024 13:15
@cferreiragonz cferreiragonz requested review from elianalf and removed request for elianalf April 25, 2024 08:46
@github-actions github-actions bot added the ci-pending PR which CI is running label Apr 25, 2024
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch 2 times, most recently from f90602a to a12d3ef Compare April 26, 2024 11:01
@cferreiragonz cferreiragonz requested review from elianalf and removed request for elianalf April 26, 2024 11:02
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch 2 times, most recently from dcc705f to 6380fb3 Compare May 6, 2024 11:06
@cferreiragonz cferreiragonz requested review from elianalf and removed request for elianalf May 6, 2024 11:06
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from 6380fb3 to 77acce1 Compare May 6, 2024 11:11
@cferreiragonz cferreiragonz requested review from elianalf and removed request for elianalf May 6, 2024 11:11
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from 77acce1 to 4d2244d Compare May 7, 2024 05:53
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from 2f68ecc to 105294a Compare May 10, 2024 11:09
@cferreiragonz cferreiragonz requested review from elianalf and removed request for elianalf May 10, 2024 11:11
@cferreiragonz cferreiragonz changed the base branch from 3.0.x-devel to bugfix/backup_restore May 10, 2024 11:13
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from 105294a to a9dfd62 Compare May 13, 2024 08:16
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from a9dfd62 to 904b75b Compare May 14, 2024 08:28
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
…scovery of TST

Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz cferreiragonz force-pushed the feature/mesh_ds_guid_less_3.0.x branch from 09e9f9a to 688cd0f Compare June 28, 2024 06:45
@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima June 28, 2024 06:49
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
MiguelCompany
MiguelCompany previously approved these changes Jul 1, 2024
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@MiguelCompany MiguelCompany added versions-pending and removed needs-review PR that is ready to be reviewed labels Jul 1, 2024
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
MiguelCompany
MiguelCompany previously approved these changes Jul 2, 2024
Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
@cferreiragonz
Copy link
Contributor Author

Green CI. Updating versions.md and RTM

@cferreiragonz cferreiragonz removed the ci-pending PR which CI is running label Jul 3, 2024
@MiguelCompany MiguelCompany added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed versions-pending labels Jul 3, 2024
@MiguelCompany MiguelCompany merged commit ee84af2 into master Jul 3, 2024
3 checks passed
@MiguelCompany MiguelCompany deleted the feature/mesh_ds_guid_less_3.0.x branch July 3, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants