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

Functional tests for IPv6 Support #2238

Closed
wants to merge 3 commits into from

Conversation

salonichf5
Copy link
Contributor

@salonichf5 salonichf5 commented Jul 15, 2024

Proposed changes

Write a clear and concise description that helps reviewers understand the purpose and impact of your changes. Use the
following format:

Problem: Users need a way to verify if IPv6/IPv4 works as expected.

Solution: Added functional tests for IPv6/IPv4 configuration.

Testing: Ran make tests under /tests/

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #2087

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main // (rebased off of "salonichf5:feat/ipv6", PR still out)
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.


@salonichf5 salonichf5 requested review from a team as code owners July 15, 2024 16:06
@github-actions github-actions bot added documentation Improvements or additions to documentation tests Pull requests that update tests helm-chart Relates to helm chart labels Jul 15, 2024
Copy link

codecov bot commented Jul 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.76%. Comparing base (1476fe0) to head (6ec7f0d).
Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2238   +/-   ##
=======================================
  Coverage   87.76%   87.76%           
=======================================
  Files          96       96           
  Lines        6793     6793           
  Branches       50       50           
=======================================
  Hits         5962     5962           
  Misses        774      774           
  Partials       57       57           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/suite/ip_family_test.go Outdated Show resolved Hide resolved
tests/suite/ip_family_test.go Outdated Show resolved Hide resolved
tests/suite/system_suite_test.go Outdated Show resolved Hide resolved
@kate-osborn
Copy link
Contributor

@salonichf5 let's mark this as a draft since it includes commits from your other open PR.

nodes:
- role: control-plane
networking:
ipFamily: dual
Copy link
Member

Choose a reason for hiding this comment

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

Can you actually get dual IP in the pipeline? AFAIK GitHub doesn't support ipv6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the seems like that it is failing due to cluster not supporting Dual IP. Thinking on how to approach this.

Copy link
Contributor

Choose a reason for hiding this comment

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

actions/runner-images#668 (comment)

Maybe a way to get it working? Should look into it closely...

Copy link
Contributor Author

@salonichf5 salonichf5 Jul 23, 2024

Choose a reason for hiding this comment

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

I am updating the kind create step with the current config for dual IP and adding the steps mentioned above !

Copy link
Contributor Author

@salonichf5 salonichf5 Jul 25, 2024

Choose a reason for hiding this comment

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

2 out of the 3 tests pass for IPFamily in kind cluster. Unable to figure out why the 3rd is failing.

I am unable to reproduce the issue locally but have asked @lucacome for some help

Passing tests

  • Service with IPv4 and IPv6 configured, when NGF is enabled on dual stack
  • Service with IPv4 configured, Service with IPv6 errors on the HTTPRoute , when NGF is enabled on IPv4 stack

Failing Tests

  • Service with IPv6 configured, Service with IPv4 errors on the HTTPRoute, when NGF is enabled on IPv6 stack ( traffic passes successfully through the IPv6 service)

@salonichf5 salonichf5 marked this pull request as draft July 16, 2024 18:06
@github-actions github-actions bot removed documentation Improvements or additions to documentation helm-chart Relates to helm chart labels Jul 18, 2024
@salonichf5 salonichf5 self-assigned this Jul 18, 2024
@salonichf5 salonichf5 marked this pull request as ready for review July 18, 2024 20:17
@salonichf5 salonichf5 requested a review from lucacome July 18, 2024 20:17
@salonichf5 salonichf5 force-pushed the tests/ipv6-functional branch 2 times, most recently from 461cc25 to b845e88 Compare July 23, 2024 21:36
@salonichf5 salonichf5 force-pushed the tests/ipv6-functional branch 6 times, most recently from 6bfa3d5 to 04e4f4f Compare July 24, 2024 20:45
@salonichf5 salonichf5 force-pushed the tests/ipv6-functional branch 5 times, most recently from 0cf1671 to 843328e Compare July 25, 2024 01:04
@salonichf5
Copy link
Contributor Author

Closing this PR because we need a dual IPFamily cluster to support IPv6 functional tests on kind but doesn't support IPv6.

@salonichf5 salonichf5 closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Pull requests that update tests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Functional test for IP version support
4 participants