From 6aa9dc1e4e9a7cd180d16262a50257fe34bd52fd Mon Sep 17 00:00:00 2001 From: Sachin Tiptur Date: Tue, 17 Jan 2023 00:04:11 +0100 Subject: [PATCH] Fixed review comments Signed-off-by: Sachin Tiptur --- README.md | 10 +++--- cloud-controller-manager/do/cloud.go | 23 +++++++------ cloud-controller-manager/do/common.go | 35 +++++++++++++++----- cloud-controller-manager/do/droplets_test.go | 10 ++---- 4 files changed, 46 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 3eac7076c..8dcb68197 100644 --- a/README.md +++ b/README.md @@ -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 DO_IP_ADDR_FAMILIES=ipv4 go run main.go \ +REGION=fra1 DO_ACCESS_TOKEN=your_access_token go run main.go \ --kubeconfig \ --leader-elect=false --v=5 --cloud-provider=digitalocean ``` @@ -79,10 +79,10 @@ 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 -default, if not set or empty. +The `DO_IP_ADDR_FAMILIES` is used to configure the required IP families and the +order in which the addresses should be populated in nodes status. The accepted values +are one of the `"ipv4", "ipv6"` or a comma-separated list of multiple IP address +families. IPv4 is the default, if not set or empty. Please note that if you use a Kubernetes cluster created on DigitalOcean, there will be a cloud controller manager running in the cluster already, so your local diff --git a/cloud-controller-manager/do/cloud.go b/cloud-controller-manager/do/cloud.go index 54720d2a1..cc3626b09 100644 --- a/cloud-controller-manager/do/cloud.go +++ b/cloud-controller-manager/do/cloud.go @@ -62,9 +62,6 @@ const ( var version string -var ipFamilyOpts = []string{"", "ipv4", "ipv6", "ipv4,ipv6", "ipv6,ipv4"} -var ipFamilies string - type tokenSource struct { AccessToken string } @@ -162,10 +159,8 @@ func newCloud() (cloudprovider.Interface, error) { addr = fmt.Sprintf("%s:%s", addrHost, addrPort) } - // var ipFamilies string ipf, set := os.LookupEnv(doIPAddrFamiliesEnv) - ipFamilies = ipf - if set && !containsString(ipFamilyOpts, ipFamilies) { + if set && !validateAndSetIPFamilies(ipf) { return nil, fmt.Errorf("invalid value set for environment variable %q", doIPAddrFamiliesEnv) } @@ -292,11 +287,17 @@ func (c *cloud) HasClusterID() bool { return false } -func containsString(vals []string, val string) bool { - for _, v := range vals { - if v == val { - return true +func validateAndSetIPFamilies(ipf string) bool { + for _, v := range strings.Split(ipf, ",") { + ipf := strings.TrimSpace(v) + switch ipf { + case "ipv4": + ipFamilies = append(ipFamilies, ipv4Family) + case "ipv6": + ipFamilies = append(ipFamilies, ipv6Family) + default: + return false } } - return false + return true } diff --git a/cloud-controller-manager/do/common.go b/cloud-controller-manager/do/common.go index 076570196..7507c1709 100644 --- a/cloud-controller-manager/do/common.go +++ b/cloud-controller-manager/do/common.go @@ -20,12 +20,20 @@ import ( "context" "errors" "fmt" - "strings" "github.com/digitalocean/godo" v1 "k8s.io/api/core/v1" ) +type IPFamily string + +var ipFamilies []IPFamily + +const ( + ipv4Family IPFamily = "ipv4" + ipv6Family IPFamily = "ipv6" +) + // apiResultsPerPage is the maximum page size that DigitalOcean's api supports. const apiResultsPerPage = 200 @@ -130,20 +138,31 @@ func nodeAddresses(droplet *godo.Droplet) ([]v1.NodeAddress, error) { var addresses []v1.NodeAddress addresses = append(addresses, v1.NodeAddress{Type: v1.NodeHostName, Address: droplet.Name}) - for _, i := range strings.Split(ipFamilies, ",") { - addr, err := discoverAddress(droplet, i) + // default case when DO_IP_ADDR_FAMILIES is not set + if ipFamilies == nil { + addr, err := discoverAddress(droplet, ipv4Family) if err != nil { - return nil, fmt.Errorf("could not get addresses for %s : %v", i, err) + return nil, fmt.Errorf("could not get addresses for %s : %v", ipv4Family, err) } addresses = append(addresses, addr...) + } else { + for _, i := range ipFamilies { + addr, err := discoverAddress(droplet, i) + if err != nil { + return nil, fmt.Errorf("could not get addresses for %s : %v", i, err) + } + addresses = append(addresses, addr...) + } } return addresses, nil } -func discoverAddress(droplet *godo.Droplet, family string) ([]v1.NodeAddress, error) { +func discoverAddress(droplet *godo.Droplet, family IPFamily) ([]v1.NodeAddress, error) { var addresses []v1.NodeAddress - if family == "ipv4" || family == "" { + + switch family { + case ipv4Family: privateIP, err := droplet.PrivateIPv4() if err != nil || privateIP == "" { return nil, fmt.Errorf("could not get private ip: %v", err) @@ -156,8 +175,7 @@ func discoverAddress(droplet *godo.Droplet, family string) ([]v1.NodeAddress, er } addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: publicIP}) return addresses, nil - } - if family == "ipv6" { + case ipv6Family: publicIPv6, err := droplet.PublicIPv6() if err != nil || publicIPv6 == "" { return nil, fmt.Errorf("could not get public ipv6: %v", err) @@ -165,6 +183,5 @@ func discoverAddress(droplet *godo.Droplet, family string) ([]v1.NodeAddress, er addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: publicIPv6}) return addresses, nil } - return addresses, nil } diff --git a/cloud-controller-manager/do/droplets_test.go b/cloud-controller-manager/do/droplets_test.go index d8862c584..5c213fa72 100644 --- a/cloud-controller-manager/do/droplets_test.go +++ b/cloud-controller-manager/do/droplets_test.go @@ -199,11 +199,11 @@ func TestNodeAddresses(t *testing.T) { Address: "99.99.99.99", }, { - Type: "public", + Type: v1.NodeExternalIP, Address: "2a01::10", }, } - + ipFamilies = []IPFamily{ipv4Family, ipv6Family} addresses, err := instances.NodeAddresses(context.TODO(), "test-droplet") if !reflect.DeepEqual(addresses, expectedAddresses) { @@ -238,12 +238,8 @@ func TestNodeAddressesByProviderID(t *testing.T) { Type: v1.NodeExternalIP, Address: "99.99.99.99", }, - { - Type: "public", - Address: "2a01::10", - }, } - + ipFamilies = []IPFamily{ipv4Family} addresses, err := instances.NodeAddressesByProviderID(context.TODO(), "digitalocean://123") if !reflect.DeepEqual(addresses, expectedAddresses) {