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

Enhance ipv4 ipv6 handling #363

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Enhance ipv4 ipv6 handling #363

merged 1 commit into from
Jun 22, 2023

Conversation

engelmi
Copy link
Member

@engelmi engelmi commented Jun 20, 2023

This PR is a follow-up of #353 and refactors the IPv4/IPv6 handling as well as adds setting the assembled IPv6 orchestrator address.

Still missing (and will be added in this PR):

  • integration test for domain name resolving

@engelmi engelmi requested review from dougsland and mwperina June 20, 2023 11:15
@engelmi engelmi force-pushed the enhance-ipv4-ipv6-handling branch from b2bd5bf to f41cfc0 Compare June 20, 2023 13:32
Copy link
Contributor

@dougsland dougsland left a comment

Choose a reason for hiding this comment

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

in general LGTM just small comments. Please let me know when is ready for Tests.

@engelmi engelmi force-pushed the enhance-ipv4-ipv6-handling branch from 5c6fa90 to 12478d4 Compare June 21, 2023 13:33
@engelmi engelmi marked this pull request as ready for review June 21, 2023 13:36
@dougsland
Copy link
Contributor

Also, if possible, could you please squash the commits? Would be easier to review :)

- enabled setting of ipv6 orch address
- renamed isIPv4Addr and isIPv6Addr to be snake case
- introduced agent_reconnect
- added integration test for fqdn to ip resolving

Signed-off-by: Michael Engel <[email protected]>
@engelmi engelmi force-pushed the enhance-ipv4-ipv6-handling branch from 12478d4 to 37d5c43 Compare June 22, 2023 07:25
@dougsland
Copy link
Contributor

make test is failing on my env but looks like it's something local, CI on github just worked:

test:         get_address_test
start time:   18:41:56
duration:     0.02s
result:       exit status 1
command:      MALLOC_PERTURB_=11 /root/hirte/builddir/src/libhirte/test/common/network/get_address_test
----------------------------------- stdout -----------------------------------
FAILED: get_address('localhost') - Expected 127.0.0.1, but got ::1
==============================================================================
# cat /etc/hosts
127.0.0.1	localhost localhost.localdomain localhost4 localhost4.localdomain4
::1	localhost localhost.localdomain localhost6 localhost6.localdomain6
192.168.122.151	devmaster
192.168.122.104	devmaster
10.90.0.1	host.containers.internal
fd00::1:8:7	control control
10.90.0.7	control control
# ping localhost
PING localhost(localhost (::1)) 56 data bytes
64 bytes from localhost (::1): icmp_seq=1 ttl=64 time=0.023 ms
64 bytes from localhost (::1): icmp_seq=2 ttl=64 time=0.046 ms

For the record: I am using a dualstack env.

@dougsland
Copy link
Contributor

We can handle these minor changes in a different patch, it's getting big here.

@dougsland dougsland merged commit e129e9c into main Jun 22, 2023
@dougsland dougsland deleted the enhance-ipv4-ipv6-handling branch June 22, 2023 18:47
@engelmi
Copy link
Member Author

engelmi commented Jun 23, 2023

make test is failing on my env but looks like it's something local, CI on github just worked:

test:         get_address_test
start time:   18:41:56
duration:     0.02s
result:       exit status 1
command:      MALLOC_PERTURB_=11 /root/hirte/builddir/src/libhirte/test/common/network/get_address_test
----------------------------------- stdout -----------------------------------
FAILED: get_address('localhost') - Expected 127.0.0.1, but got ::1
==============================================================================
# cat /etc/hosts
127.0.0.1	localhost localhost.localdomain localhost4 localhost4.localdomain4
::1	localhost localhost.localdomain localhost6 localhost6.localdomain6
192.168.122.151	devmaster
192.168.122.104	devmaster
10.90.0.1	host.containers.internal
fd00::1:8:7	control control
10.90.0.7	control control
# ping localhost
PING localhost(localhost (::1)) 56 data bytes
64 bytes from localhost (::1): icmp_seq=1 ttl=64 time=0.023 ms
64 bytes from localhost (::1): icmp_seq=2 ttl=64 time=0.046 ms

For the record: I am using a dualstack env.

This actually broke the COPR build: https://copr.fedorainfracloud.org/coprs/mperina/hirte-snapshot/build/6105555/
PR to fix the failing test (make it independent if domain resolves to ipv4 or ipv6): #366

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.

3 participants