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

Add dual-stack support to the etcd registry #1522

Merged
merged 5 commits into from
Feb 21, 2022

Conversation

sjoerdsimons
Copy link
Contributor

@sjoerdsimons sjoerdsimons commented Jan 16, 2022

Description

Add dual-stack support to the etcd registry

Todos

  • Documentation
  • Release note ?

Release Note

None required

@manuelbuil
Copy link
Collaborator

This is still WIP, right?

@sjoerdsimons
Copy link
Contributor Author

This is still WIP, right?

It's good enough for my local testing; Imho what's mostly missing is some documentation in running.md on how to configure it, but code-wise it's complete. Unless there are some other bits you'd like to see?

@manuelbuil
Copy link
Collaborator

This is still WIP, right?

It's good enough for my local testing; Imho what's mostly missing is some documentation in running.md on how to configure it, but code-wise it's complete. Unless there are some other bits you'd like to see?

I wrote that because of the Wip in the title of the PR. If it is ready for review, I can review it today or tomorrow ;)

@sjoerdsimons
Copy link
Contributor Author

This is still WIP, right?

It's good enough for my local testing; Imho what's mostly missing is some documentation in running.md on how to configure it, but code-wise it's complete. Unless there are some other bits you'd like to see?

I wrote that because of the Wip in the title of the PR. If it is ready for review, I can review it today or tomorrow ;)

Oh sorry my bad; we have a convention to do our work in wip/xxxx branches which some git managements systems get confused about ; i'll fix the title.

@sjoerdsimons sjoerdsimons changed the title Wip/sjoerd/localregistry Add dual-stack support to the etcd registry Jan 17, 2022
@manuelbuil
Copy link
Collaborator

Seems good! Could you please also update the docs? Currently for dual-stack only kube-subnet-mgr appears as the correct config https://github.com/flannel-io/flannel/blob/master/Documentation/configuration.md#dual-stack

Copy link
Collaborator

@manuelbuil manuelbuil left a comment

Choose a reason for hiding this comment

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

Thanks for the code!

main.go Outdated Show resolved Hide resolved
subnet/subnet.go Show resolved Hide resolved
subnet/subnet.go Outdated Show resolved Hide resolved
subnet/subnet.go Outdated Show resolved Hide resolved
subnet/kube/kube.go Outdated Show resolved Hide resolved
subnet/etcdv2/local_manager.go Show resolved Hide resolved
subnet/etcdv2/local_manager.go Outdated Show resolved Hide resolved
Comparing two pointers in golang just compares the pointer addresses not
the value, hence the curremt Empty() implementation always returns
false. Fix that and add tests for it.

Signed-off-by: Sjoerd Simons <[email protected]>
To allow a lease to contain both an ipv4 and an (optional) ipv6 address
extend the subnet key parsing/generation with an optional
"&<ipv6 address>-<prefix length">

So examples of the various forms become:
  * 10.12.13.0-24 (ipv4 only)
  * 10.12.13.0-24&fd00:12:13::-56 (ipv4 and ipv6)

Ipv6 only is not currently supported (as flannel heavily assumes ipv4 is
always enabled throughout its code-base). Though an ipv6 only key could
be "&fd00:12:13::-56"

Signed-off-by: Sjoerd Simons <[email protected]>
Take advantage of the etcd keys now being able to serialize an ipv4 and
ipv6 address to allow dual-stack leases. ipv4 is mandatory, ipv6 is
optional.

The lease lookup in etcd is still done based on the external ipv4
address. Generated dual-stack addresses will use the same offset inside
the address range for v4 and v6, so the coupling is deterministic.

Signed-off-by: Sjoerd Simons <[email protected]>
@sjoerdsimons sjoerdsimons force-pushed the wip/sjoerd/localregistry branch from 7267f7b to 92a50ac Compare February 2, 2022 20:47
As a fallback, if no subnet lease can be foudn in etcd, flannel tries to
fallback to the subnets in the subnet.env file. Add support for
dual-stack to the fallback path

Signed-off-by: Sjoerd Simons <[email protected]>
@manuelbuil manuelbuil merged commit 4faee72 into flannel-io:master Feb 21, 2022
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