Fix providerID & getZone Implementation

[vishesh92]

Fix #58

[Copilot]

Pull request overview

This PR fixes issue #58 by correcting the providerID format and improving the GetZone implementation for CloudStack cloud provider integration with Kubernetes. The changes address two key problems: (1) providerID was being set to just the provider name instead of the full "provider://instanceID" format, and (2) GetZone was using os.Hostname() which returns the pod name in containerized environments rather than the actual node name.

• Added helper functions to convert between provider IDs and instance IDs in the correct "provider://instanceID" format
• Modified GetZone to query the Kubernetes API for the node name when the cloud controller manager runs in a pod
• Added RBAC permissions for the cloud controller manager to read pod information

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
deployment.yaml	Adds RBAC permissions (list, get) for pods resource to allow the cloud controller manager to query pod information
cloudstack_instances.go	Implements helper functions for providerID format conversion and updates all *ByProviderID methods to use the correct instance ID extraction
cloudstack.go	Adds clientBuilder field, implements getNodeNameFromPod to retrieve node name from pod spec via Kubernetes API, and updates GetZone to use this new method

Comments suppressed due to low confidence (1)

cloudstack.go:196

• The cs.zone field is being read and written without synchronization in the GetZone method. If GetZone is called concurrently from multiple goroutines when cs.zone is empty, there's a race condition where multiple goroutines could simultaneously query the CloudStack API and overwrite cs.zone. Consider using a sync.Once or mutex to ensure thread-safe initialization of the zone field.

	if cs.zone == "" {
		// In Kubernetes pods, os.Hostname() returns the pod name, not the node hostname.
		// We need to get the node name from the pod's spec.nodeName using the Kubernetes API.
		nodeName, err := cs.getNodeNameFromPod(ctx)
		if err != nil {
			return zone, fmt.Errorf("failed to get node name for retrieving the zone: %v", err)
		}

		instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByName(
			nodeName,
			cloudstack.WithProject(cs.projectID),
		)
		if err != nil {
			if count == 0 {
				return zone, fmt.Errorf("could not find CloudStack instance with name %s for retrieving the zone: %v", nodeName, err)
			}
			return zone, fmt.Errorf("error getting instance for retrieving the zone: %v", err)
		}

		cs.zone = instance.Zonename

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The getInstanceIDFromProviderID function could produce incorrect results if the providerID contains multiple "://" separators. The current implementation using strings.Split would create more than 2 parts, and parts[1] would only return the segment between the first and second "://" rather than everything after the first one. Use strings.SplitN(providerID, "://", 2) instead to split only on the first occurrence of "://".

[Copilot]

The new helper functions getProviderIDFromInstanceID and getInstanceIDFromProviderID are missing documentation comments. Following Go conventions, exported or internal helper functions should have comments explaining their purpose, parameters, and return values. Add documentation comments to clarify what these functions do and their expected input/output formats.

[DaanHoogland]

clgtm