IPv6 address support in node addresses #561
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Sachin Tiptur <[email protected]>
timoreimann
reviewed
Dec 12, 2022
| @@ -128,18 +130,28 @@ func allLoadBalancerList(ctx context.Context, client *godo.Client) ([]godo.LoadB | |||
| func nodeAddresses(droplet *godo.Droplet) ([]v1.NodeAddress, error) { | |||
| var addresses []v1.NodeAddress | |||
| addresses = append(addresses, v1.NodeAddress{Type: v1.NodeHostName, Address: droplet.Name}) | |||
| ipFamilies := os.Getenv(doIPAddrFamiliesEnv) | |||
There was a problem hiding this comment.
I think it'd be great if we parsed the env var during the startup phase only (and use the opportunity to error out if something unsupported is passed in). The logic processing the parameters (i.e., this function) can then work off of more strongly typed data (say, a typed string with two constant values).
added 2 commits
December 13, 2022 11:18
Signed-off-by: Sachin Tiptur <[email protected]>
Signed-off-by: Sachin Tiptur <[email protected]>
sachintiptur
force-pushed
the
ipv6_support
branch
from
14215b8 to
d066d36
Compare
timoreimann
reviewed
Jan 11, 2023
README.md
Outdated
| @@ -62,7 +62,7 @@ package-hosted directory like this: | |||
|
|
|||
| ```bash | |||
| cd cloud-controller-manager/cmd/digitalocean-cloud-controller-manager | |||
| REGION=fra1 DO_ACCESS_TOKEN=your_access_token go run main.go \ | |||
| REGION=fra1 DO_ACCESS_TOKEN=your_access_token DO_IP_ADDR_FAMILIES=ipv4 go run main.go \ | |||
There was a problem hiding this comment.
I'd omit the usage from this example since it's not strictly needed when running locally.
README.md
Outdated
| @@ -79,6 +79,11 @@ You might also need to provide your DigitalOcean access token in | |||
| the cloud controller to start, but in that case, you will not be able to | |||
| validate integration with DigitalOcean API. | |||
|
|
|||
| The `DO_IP_ADDR_FAMILIES` is used to configure the required IP familes and the | |||
README.md
Outdated
| @@ -79,6 +79,11 @@ You might also need to provide your DigitalOcean access token in | |||
| the cloud controller to start, but in that case, you will not be able to | |||
| validate integration with DigitalOcean API. | |||
|
|
|||
| The `DO_IP_ADDR_FAMILIES` is used to configure the required IP familes and the | |||
| order in which address should be populated in nodes status. The accepted values | |||
There was a problem hiding this comment.
Suggested change
| order in which address should be populated in nodes status. The accepted values | |
| order in which the addresses should be populated in nodes status. The accepted values |
README.md
Outdated
| @@ -79,6 +79,11 @@ You might also need to provide your DigitalOcean access token in | |||
| the cloud controller to start, but in that case, you will not be able to | |||
| validate integration with DigitalOcean API. | |||
|
|
|||
| The `DO_IP_ADDR_FAMILIES` is used to configure the required IP familes and the | |||
| order in which address should be populated in nodes status. The accepted values | |||
| are one of the `{"", "ipv4", "ipv6", "ipv4,ipv6", "ipv6,ipv4"}`.IPv4 is the | |||
There was a problem hiding this comment.
Why do we have curly braces here? It almost looks like JSON but we don't require JSON, do we?
Can we rephrase this in a more user friendly manner, e.g., say one of "ipv4", "ipv6", or a comma-separated list of multiple IP address families?
Signed-off-by: Sachin Tiptur <[email protected]>
sachintiptur
force-pushed
the
ipv6_support
branch
from
6aa9dc1 to
6b1a54d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Sachin Tiptur [email protected]
This PR adds a new env
DO_IP_ADDR_FAMILIESto configure IP families required to add IP addresses in node's status.This also populates the nodes address in the same order as
DO_IP_ADDR_FAMILIESis configured.This fixes the issue #555