Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 67 additions & 8 deletions cloudstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/apache/cloudstack-go/v2/cloudstack"
"gopkg.in/gcfg.v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/klog/v2"
Expand All @@ -50,9 +51,10 @@ type CSConfig struct {

// CSCloud is an implementation of Interface for CloudStack.
type CSCloud struct {
client *cloudstack.CloudStackClient
projectID string // If non-"", all resources will be created within this project
zone string
client *cloudstack.CloudStackClient
projectID string // If non-"", all resources will be created within this project
zone string
clientBuilder cloudprovider.ControllerClientBuilder
}

func init() {
Expand Down Expand Up @@ -100,6 +102,7 @@ func newCSCloud(cfg *CSConfig) (*CSCloud, error) {

// Initialize passes a Kubernetes clientBuilder interface to the cloud provider
func (cs *CSCloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
cs.clientBuilder = clientBuilder
}

// LoadBalancer returns an implementation of LoadBalancer for CloudStack.
Expand Down Expand Up @@ -172,15 +175,20 @@ func (cs *CSCloud) GetZone(ctx context.Context) (cloudprovider.Zone, error) {
zone := cloudprovider.Zone{}

if cs.zone == "" {
hostname, err := os.Hostname()
// 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 hostname for retrieving the zone: %v", err)
return zone, fmt.Errorf("failed to get node name for retrieving the zone: %v", err)
}

instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByName(hostname)
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 instance for retrieving the zone: %v", err)
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)
}
Expand All @@ -200,7 +208,7 @@ func (cs *CSCloud) GetZoneByProviderID(ctx context.Context, providerID string) (
zone := cloudprovider.Zone{}

instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID(
providerID,
cs.getInstanceIDFromProviderID(providerID),
cloudstack.WithProject(cs.projectID),
)
if err != nil {
Expand Down Expand Up @@ -238,3 +246,54 @@ func (cs *CSCloud) GetZoneByNodeName(ctx context.Context, nodeName types.NodeNam

return zone, nil
}

// getNodeNameFromPod gets the node name where this pod is running by querying the Kubernetes API.
// It uses the pod's name and namespace (from environment variables or hostname) to look up the pod
// and retrieve its spec.nodeName field.
func (cs *CSCloud) getNodeNameFromPod(ctx context.Context) (string, error) {
if cs.clientBuilder == nil {
return "", fmt.Errorf("clientBuilder not initialized, cannot query Kubernetes API")
}

client, err := cs.clientBuilder.Client("cloud-controller-manager")
if err != nil {
return "", fmt.Errorf("failed to get Kubernetes client: %v", err)
}

// Get pod name and namespace
// In Kubernetes, the pod name is available as HOSTNAME environment variable
// or we can use os.Hostname() which returns the pod name
podName := os.Getenv("HOSTNAME")
if podName == "" {
var err error
podName, err = os.Hostname()
if err != nil {
return "", fmt.Errorf("failed to get pod name: %v", err)
}
}

// Get namespace from environment variable or default to kube-system for CCM
namespace := os.Getenv("POD_NAMESPACE")
if namespace == "" {
// Try reading from service account namespace file (available in pods)
if data, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
namespace = string(data)
Comment on lines +279 to +280
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The namespace trimming logic doesn't handle potential whitespace. When reading from /var/run/secrets/kubernetes.io/serviceaccount/namespace, the data might include trailing newlines or whitespace. Consider using strings.TrimSpace(string(data)) instead of just string(data) to ensure clean namespace values.

Copilot uses AI. Check for mistakes.
} else {
// Default namespace for cloud controller manager
namespace = "kube-system"
}
}

// Get the pod object from Kubernetes API
pod, err := client.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{})
if err != nil {
return "", fmt.Errorf("failed to get pod %s/%s from Kubernetes API: %v", namespace, podName, err)
}

if pod.Spec.NodeName == "" {
return "", fmt.Errorf("pod %s/%s does not have a nodeName assigned yet", namespace, podName)
}

klog.V(4).Infof("found node name %s for pod %s/%s", pod.Spec.NodeName, namespace, podName)
return pod.Spec.NodeName, nil
}
Comment on lines +253 to +299
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The new getNodeNameFromPod function, which queries the Kubernetes API to get pod information, lacks test coverage. Since other parts of this codebase have test coverage (see cloudstack_test.go), this new functionality should also include tests to verify correct behavior in various scenarios (e.g., missing HOSTNAME env var, missing namespace file, pod not scheduled yet).

Copilot uses AI. Check for mistakes.
26 changes: 22 additions & 4 deletions cloudstack_instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"regexp"
"strings"

"github.com/apache/cloudstack-go/v2/cloudstack"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -53,7 +54,7 @@ func (cs *CSCloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]co
// NodeAddressesByProviderID returns the addresses of the specified instance.
func (cs *CSCloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]corev1.NodeAddress, error) {
instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID(
providerID,
cs.getInstanceIDFromProviderID(providerID),
cloudstack.WithProject(cs.projectID),
)
if err != nil {
Expand Down Expand Up @@ -125,7 +126,7 @@ func (cs *CSCloud) InstanceType(ctx context.Context, name types.NodeName) (strin
// InstanceTypeByProviderID returns the type of the specified instance.
func (cs *CSCloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID(
providerID,
cs.getInstanceIDFromProviderID(providerID),
cloudstack.WithProject(cs.projectID),
)
if err != nil {
Expand All @@ -151,7 +152,7 @@ func (cs *CSCloud) CurrentNodeName(ctx context.Context, hostname string) (types.
// InstanceExistsByProviderID returns if the instance still exists.
func (cs *CSCloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
_, count, err := cs.client.VirtualMachine.GetVirtualMachineByID(
providerID,
cs.getInstanceIDFromProviderID(providerID),
cloudstack.WithProject(cs.projectID),
)
if err != nil {
Expand Down Expand Up @@ -185,6 +186,11 @@ func (cs *CSCloud) InstanceShutdown(ctx context.Context, node *corev1.Node) (boo

func (cs *CSCloud) InstanceMetadata(ctx context.Context, node *corev1.Node) (*cloudprovider.InstanceMetadata, error) {

instanceID, err := cs.InstanceID(ctx, types.NodeName(node.Name))
if err != nil {
return nil, err
}

instanceType, err := cs.InstanceType(ctx, types.NodeName(node.Name))
if err != nil {
return nil, err
Expand All @@ -201,10 +207,22 @@ func (cs *CSCloud) InstanceMetadata(ctx context.Context, node *corev1.Node) (*cl
}

return &cloudprovider.InstanceMetadata{
ProviderID: cs.ProviderName(),
ProviderID: cs.getProviderIDFromInstanceID(instanceID),
InstanceType: instanceType,
NodeAddresses: addresses,
Zone: cs.zone,
Region: zone.Region,
}, nil
}

func (cs *CSCloud) getProviderIDFromInstanceID(instanceID string) string {
return fmt.Sprintf("%s://%s", cs.ProviderName(), instanceID)
}

func (cs *CSCloud) getInstanceIDFromProviderID(providerID string) string {
parts := strings.Split(providerID, "://")
if len(parts) == 1 {
return providerID
}
return parts[1]
Comment on lines +223 to +227
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
Comment on lines +218 to +228
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The new helper functions getProviderIDFromInstanceID and getInstanceIDFromProviderID lack test coverage. These are critical functions for parsing and constructing provider IDs. Since the codebase has existing test coverage (see cloudstack_test.go), these utilities should include unit tests to verify correct behavior with various input formats (e.g., with "://", without "://", empty strings, etc.).

Copilot uses AI. Check for mistakes.
Comment on lines +218 to +228
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
7 changes: 7 additions & 0 deletions deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ rules:
- nodes
verbs:
- '*'
- apiGroups:
- ""
resources:
- pods
verbs:
- list
- get
- apiGroups:
- ""
resources:
Expand Down
Loading