Skip to content

Conversation

@TothFerenc
Copy link
Contributor

What type of PR is this?
feature

What does this PR give to us:
Add Dual stack IPv6 support for external CNIs and fakeipam.

Which issue(s) this PR fixes
Fixes #73

Special notes for your reviewer:
It is under testing, but please review it whether this PR is conceptually ok.

Does this PR introduce a user-facing change?:
NONE

@Levovar
Copy link
Collaborator

Levovar commented Apr 12, 2019

overall, looks good

@Levovar
Copy link
Collaborator

Levovar commented Apr 12, 2019

LGTM, once you are satisfied with it I can merge

@Levovar
Copy link
Collaborator

Levovar commented Apr 12, 2019

I merged my UT branch if you don't mind, case it will be easier to rebase this on top of mine, than via-versa.
But I can help in fixing the UTs, and creating a couple new cases for dual stack

@TothFerenc TothFerenc changed the title WIP: Dual stack IPv6 support for external CNIs and fakeipam Dual stack IPv6 support for external CNIs and fakeipam Apr 15, 2019
@TothFerenc
Copy link
Contributor Author

Tested with macvlan CNI inside a VM:

  • Dual stack: works
  • IPv6 only: macvlan CNI fails to parse the IPv4 address, which is nil this case, but that is not a DANM issue

So this PR is ready to merge.

Added new MACVLAN based TC to cover dual-stack case.
Fixed a small bug in the schema: IPs need the "omitempty" JSON deisgnation for proper marshalling / unmarshalling.
@Levovar
Copy link
Collaborator

Levovar commented Apr 15, 2019

UTs corrected, new code functionally covered, so everything looks to be in tip top shape :)

@Levovar Levovar merged commit 57911a5 into nokia:master Apr 15, 2019
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.

Dual stack IPv6 support for external CNIs and fakeipam

2 participants