Implement GetZoneByProviderID and GetZoneByNodeName about digitalocean-cloud-controller-manager

[klausenbusk]

Did this fix the issue mentioned in #24 (comment) ?
The code still seems to contain the "broken" logic?

[andrewsykim]

@klausenbusk this should have been fixed. What code are you referring to? GetZone()?

[klausenbusk]

@klausenbusk this should have been fixed. What code are you referring to? GetZone()?

This code:

digitalocean-cloud-controller-manager/do/metadata.go

Lines 28 to 30 in 9fbae98

	func dropletRegion() (string, error) {
	return httpGet(dropletRegionMetadataURL)
	}

which is used here:

digitalocean-cloud-controller-manager/do/cloud.go

Line 68 in f029698

region, err := dropletRegion()

and then passed as arguments here:

digitalocean-cloud-controller-manager/do/cloud.go

Lines 75 to 77 in f029698

	instances: newInstances(doClient, region),
	zones: newZones(doClient, region),
	loadbalancers: newLoadbalancers(doClient, region),

Maybe I just don't understand the code :)

[andrewsykim]

The confusion there is how the Zones interface was designed. The GetZone method was originally for kubelets to get their own zones, usually using a meta data service. External cloud controllers have to adopt the same interfaces for compatibility. We technically don't even need to implement that method since it would never be called in our context, but it was used since GetZone was the only method at the time.

In v1.8, GetZoneByProviderID and GetZoneByName were added which is what's really called by cloud controllers upstream. Before v1.8, this was considered a bug as per kubernetes/kubernetes#49308

[andrewsykim]

Our implementation for GetZoneByProviderID and GetZoneByName https://github.com/digitalocean/digitalocean-cloud-controller-manager/blob/master/do/zones.go#L43-L70

And the only piece of code using the region field is GetZone https://github.com/digitalocean/digitalocean-cloud-controller-manager/blob/master/do/zones.go#L40 which as I mentioned previously is not really used in our context, might be worthwhile to remove the implementation there altogether to avoid future confusion :)