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

Enable ipv6 for actions running on docker containers #12856

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

romeroalx
Copy link
Member

Short description

Fix for #12844. Adds configuration to enable IPv6 for actions running on docker containers: options: --sysctl net.ipv6.conf.all.disable_ipv6=0.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

@romeroalx
Copy link
Member Author

Hi @omoerbeek, before merging this PR there is something I would like to check with you.

Before introducing GH actions running on containers, we skipped some regression tests for Recursor, specifically in file regression-tests.recursor-dnssec/test_EDNSPadding.py, test addingAllowedAlwaysSameTagTest(RecursorEDNSPaddingTest) by setting the environment variable SKIP_IPV6_TESTS. More details here

I have tested removing that environment variable, and it seems that the test passes:

...
testQueryWithPadding (test_EDNSPadding.PaddingAllowedAlwaysSameTagTest) ... ok
testQueryWithoutEDNS (test_EDNSPadding.PaddingAllowedAlwaysSameTagTest) ... ok
testQueryWithoutPadding (test_EDNSPadding.PaddingAllowedAlwaysSameTagTest) ... ok
...

Do you remember why we were skipping those tests? Maybe we could leave those tests enabled again.

@romeroalx romeroalx requested a review from omoerbeek May 29, 2023 14:59
@omoerbeek
Copy link
Member

It looks like there were originally disabled since CircleCI (which we used then) did not support IPV6. See b2d3e2b
They should no longer be disabled.

@romeroalx
Copy link
Member Author

Good! Then no additional changes are required :)

Thank you @omoerbeek

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.

2 participants