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

Make failover work over IP families #7658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

thalman
Copy link
Contributor

@thalman thalman commented Oct 18, 2024

Originally the option ipv4_first and ipv6_first was taken into account
when resolving IP address.

When both families are resolvable but the primary is blocked on
firewall, the SSSD must switch to the socondary family.

@alexey-tikhonov
Copy link
Member

@thalman, could you please add a release note (or convert commit message to a release note)?

@thalman thalman marked this pull request as ready for review January 21, 2025 14:30
@thalman
Copy link
Contributor Author

thalman commented Jan 23, 2025

@thalman, could you please add a release note (or convert commit message to a release note)?

done

Originally the option ipv4_first and ipv6_first was taken into account
when resolving IP address.

When both families are resolvable but the primary is blocked on
firewall, the SSSD should try the secondary family before giving up.

:relnote:SSSD now attempts to connect to the server using a secondary
protocol if the server is not reachable using the primary one.
See the lookup_family_order option.
@alexey-tikhonov
Copy link
Member

All system tests fail.

Number of steps and results do not match in idm-sssd-tc::tests/test_failover.py::test_failover__connect_second_family 

Test that IPA server is still reachable when primary
address family is blocked but secondary is working.
Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

Minor changes, other than that, it looks good.

@@ -80,3 +80,30 @@ def test_failover__reactivation_timeout_is_honored(
assert (
f"Primary server reactivation timeout set to {expected} seconds" in log
), f"'Primary server reactivation timeout set to {expected} seconds' not found in logs!"


@pytest.mark.importance("low")
Copy link

Choose a reason for hiding this comment

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

Can we make this a generic provider test?

or

Move this test into test_ipa.py ?

client.fs.append("/etc/hosts", "cafe:cafe::3 %s" % ipa.host.hostname)
client.sssd.start()
result = client.tools.id(user)
assert result is not None
Copy link

Choose a reason for hiding this comment

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

Can you add an error message please

assert result is not None,  f"{user.name} was not found!"

client.sssd.domain["lookup_family_order"] = "ipv6_first"
client.fs.append("/etc/hosts", "cafe:cafe::3 %s" % ipa.host.hostname)
client.sssd.start()
result = client.tools.id(user)
Copy link

Choose a reason for hiding this comment

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

nitpick, line break after the setup

2. user is resolved
:customerscenario: False
"""
user = "testuser"
Copy link

Choose a reason for hiding this comment

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

squash these lines?

user = ipa.user("testuser").add() 

3. Set IPv6 address in /etc/hosts so it resolves but it
points to non-exesting machine
:steps:
1. Restart SSSD
Copy link

Choose a reason for hiding this comment

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

Technically, you are starting SSSD. This line can be the 4th step in the setup, so the test only contains one step.


@pytest.mark.importance("low")
@pytest.mark.topology(KnownTopology.IPA)
def test_failover__connect_second_family(client: Client, ipa: IPA):
Copy link

Choose a reason for hiding this comment

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

I'd add ipv6 in the name, since "families" can also refer to ipv4

tail_failover_connect_using_ipv6_second_family

{
if (server != NULL &&
server->common != NULL &&
server->common->rhostent) {
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 cosmetic and I'm not asking you to fix it. But if you want to, I will not oppose.

You compare against NULL in the two previous lines, but not in this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants