Skip to content

gh-82082: Make our test suite pass on an IPv6-only Linux host #26225

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

Closed
wants to merge 28 commits into from

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented May 19, 2021

Background: A true IPv6-only host has no IPv4 localhost 127.0.0.1 or IPv4 AF_INET stack enabled at all. I realize these are rare to most people. They're not to me & I have buildbot ready and waiting to go once I get things in our tree into decent shape. I'll post how that was setup in a gist later.

This is a collection of changes, not intended for submission via this branch. Likely to be split up across a variety of PRs addressing each individual test and a pre-PR for the test.support.socket_helper changes. But it makes for good initial viewing and study of patterns by showing this branch as a draft PR.

Status

Where I started

24 tests failed:
    test_asynchat test_asyncore test_docxmlrpc test_eintr test_epoll
    test_ftplib test_httplib test_imaplib test_multiprocessing_fork
    test_multiprocessing_forkserver test_multiprocessing_spawn
    test_nntplib test_os test_poplib test_robotparser test_smtplib
    test_socket test_ssl test_support test_sys test_telnetlib
    test_urllib2_localnet test_venv test_wsgiref

Where I'm at

At the time of writing this message, I have fixed all of the above to work in this branch!

At least four tests were hanging and still hang. Separate BPO issues are open for them: test_asyncio test_httpservers test_logging test_xmlrpc

I haven't yet run regrtest with all of the -u network flags enabled so I'm no doubt missing some more failures or hangs.

https://bugs.python.org/issue37901

@gpshead gpshead added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir sprint DO-NOT-MERGE labels May 19, 2021
@gpshead gpshead self-assigned this May 19, 2021
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

I suggest to start with a PR only to add get_family() and get_bound_ip_socket_and_port() (and IPV4_ENABLED, needed by get_family) to socket_helper. You can pick a few modified tests which only use these functions, like test_asynchat. It's easier to review a smaller PR.

Then you can write a PR for multiprocessing which adds tcp_socket().

When you add a function in test.support, it's good to document it at:
https://docs.python.org/dev/library/test.html#module-test.support.socket_helper

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

"TODO(gpshead): We should support a https://pypi.org/project/portpicker/ portserver or equivalent process running on our buildbot hosts and use that portpicker library"

What is the advantage compared to find_unused_port()?

One find_unused_port() flaw is that there is a race condition if two processes call it at the same time: https://bugs.python.org/issue38697

But I'm not convinced by the find_unused_port() design. It sounds better to simply... not use it at all. Like create a socket with port=0 and then get the port number from the socket object.

@gpshead
Copy link
Member Author

gpshead commented May 19, 2021

I suggest to start with a PR only to add get_family() and get_bound_ip_socket_and_port() (and IPV4_ENABLED, needed by get_family) to socket_helper. You can pick a few modified tests which only use these functions, like test_asynchat. It's easier to review a smaller PR.

This is a draft PR that isn't intended for direct submission. It'll be broken up into sub-PRs exactly like that once I'm happy with how it looks. :)

For starters, this is a way to test my branch in CI and on buildbots as I evolve the changes.

"TODO(gpshead): We should support a https://pypi.org/project/portpicker/ portserver or equivalent process running on our buildbot hosts and use that portpicker library"

What is the advantage compared to find_unused_port()?

Thats a long term idea that I don't intend to solve as it'd require infrastructure changes on every buildbot worker's part. I'll probably just drop the todo. It has advantages in that it is centralized port management so that multiple tests running in parallel don't hit a race condition when binding to port 0 to obtain a "unique" port to reuse. It'd solve issue38697 in the face of tests that still want to use the unfortunate "give me a port number" approach.

find_unused_port is always the worst API option. I'm not happy with the changes to it in socket_helper either. I'll step back and see which existing messy tests can easily be refactored to not use it after I get more things working.

Some things tests by their nature and code API structure need to start with a port number rather than a bound socket. I'm attempting to avoid introducing non-test API changes and features when possible for now.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 20, 2021
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 8, 2022
@arhadthedev arhadthedev changed the title bpo-37901: Make our test suite pass on an IPv6-only Linux host gh-82082: Make our test suite pass on an IPv6-only Linux host Apr 29, 2023
@arhadthedev
Copy link
Member

Removing sprint because the PR missed the ones in 2021, 2022, and 2023.

@arhadthedev
Copy link
Member

test_nntplib was removed in gh-104894, asyncore/asynchat in gh-96580.

Could you resolve merge conflicts to rerun the tests, please?

@gpshead
Copy link
Member Author

gpshead commented May 7, 2024

i'm not intending to continue with this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review DO-NOT-MERGE tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants