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

Configure Discovery Server using names [12349] #2140

Merged
merged 8 commits into from
Aug 13, 2021

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Aug 11, 2021

This PR allows configuring both Discovery Servers and Clients specifying names instead of IPv4 addresses (port is still required). If a name is specified, the DNS is queried for IPv4 addresses, which are in turn used to configure the Server and Client.

Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Windows is issuing a warning due to the getenv method that has to be removed. However, there are two tests related to this PR that are failing in Windows:

  • LocatorDNSTests.resolve_name
  • LocatorDNSTests.dns_locator

src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp Outdated Show resolved Hide resolved
tools/fds/server.cpp Outdated Show resolved Hide resolved
tools/fds/server.cpp Show resolved Hide resolved
test/blackbox/common/BlackboxTestsDiscovery.cpp Outdated Show resolved Hide resolved
@EduPonz
Copy link
Author

EduPonz commented Aug 12, 2021

  • LocatorDNSTests.resolve_name
  • LocatorDNSTests.dns_locator

This tests have failed in today's nightly, even though they did not failed in #2061. This is related to the known hosts in the testing machines, and has anything to do with the tests of the implementation.

JLBuenoLopez
JLBuenoLopez previously approved these changes Aug 12, 2021
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM if the CI does not give any unexpected failure.

@EduPonz
Copy link
Author

EduPonz commented Aug 12, 2021

@richiprosima please test windows

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

Not really from your PR but I suggest fixing these typos in this PR.

tools/fds/README.txt Outdated Show resolved Hide resolved
tools/fds/server.h Outdated Show resolved Hide resolved
@EduPonz EduPonz added the skip-ci Automatically pass CI label Aug 13, 2021
JLBuenoLopez
JLBuenoLopez previously approved these changes Aug 13, 2021
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz force-pushed the feature/create_servers_dns branch from 1cc6e32 to 9434449 Compare August 13, 2021 06:00
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

LGTM

@EduPonz EduPonz merged commit 4c92fe8 into master Aug 13, 2021
@EduPonz EduPonz deleted the feature/create_servers_dns branch August 13, 2021 06:03
SamuelWHalodi pushed a commit to SamuelWHalodi/Fast-DDS that referenced this pull request Oct 4, 2021
* Refs 12349: Use cross-platform get_env

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Specify locators as DNS in fastdds CLI

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Specify locators as DNS in environment variable

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Allow names in locators using XML

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Fix ServerClientEnvironmentSetUp test

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Update fds help

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12349: Apply suggestions

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Samuel Wilhelmsson <samuel@halodi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-ci Automatically pass CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants