From 8a6e4fef5a594cac0c510432a3e6cce8357a810d Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 3 Jul 2025 06:25:54 +0530 Subject: [PATCH 01/12] Add Ipaddresses to the csMachine status --- pkg/cloud/instance.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 3da2705c..3a522bb2 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -44,7 +44,15 @@ func setMachineDataFromVMMetrics(vmResponse *cloudstack.VirtualMachinesMetric, c csMachine.Spec.ProviderID = ptr.To(fmt.Sprintf("cloudstack:///%s", vmResponse.Id)) // InstanceID is later used as required parameter to destroy VM. csMachine.Spec.InstanceID = ptr.To(vmResponse.Id) - csMachine.Status.Addresses = []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: vmResponse.Ipaddress}} + csMachine.Status.Addresses = []corev1.NodeAddress{} + for _, nic := range vmResponse.Nic { + if nic.Ipaddress != "" { + csMachine.Status.Addresses = append(csMachine.Status.Addresses, corev1.NodeAddress{ + Type: corev1.NodeInternalIP, + Address: nic.Ipaddress, + }) + } + } newInstanceState := vmResponse.State if newInstanceState != csMachine.Status.InstanceState || (newInstanceState != "" && csMachine.Status.InstanceStateLastUpdated.IsZero()) { csMachine.Status.InstanceState = newInstanceState From 5fe066664516d2adbc4c34c339afc633486784cc Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 3 Jul 2025 11:50:46 +0530 Subject: [PATCH 02/12] Add default network checks and more IP validations --- pkg/cloud/instance.go | 163 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 150 insertions(+), 13 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 3a522bb2..421572cc 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -31,6 +31,8 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" + + netpkg "net" ) type VMIface interface { @@ -310,38 +312,163 @@ func (c *client) CheckLimits( return nil } -func (c *client) resolveNetworkIDByName(name string) (string, error) { +func (c *client) isIpAvailableInNetwork(ip, networkID string) (bool, error) { + params := c.cs.Address.NewListPublicIpAddressesParams() + params.SetNetworkid(networkID) + params.SetIpaddress(ip) + params.SetAllocatedonly(false) + params.SetForvirtualnetwork(false) + params.SetListall(true) + + resp, err := c.cs.Address.ListPublicIpAddresses(params) + if err != nil { + return false, errors.Wrapf(err, "failed to list public IP addresses for network %q", networkID) + } + + for _, addr := range resp.PublicIpAddresses { + if addr.State == "Free" { + return true, nil + } + } + + return false, nil +} + +func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, error) { + params := c.cs.Address.NewListPublicIpAddressesParams() + params.SetNetworkid(resolvedNet.Id) + params.SetAllocatedonly(false) + params.SetForvirtualnetwork(false) + params.SetListall(true) + + resp, err := c.cs.Address.ListPublicIpAddresses(params) + if err != nil { + return false, errors.Wrapf(err, "failed to check free IPs for network %q", resolvedNet.Id) + } + + for _, addr := range resp.PublicIpAddresses { + if addr.State == "Free" { + return true, nil + } + } + + return false, nil +} + +func (c *client) buildStaticIPEntry(ip, networkID string, resolvedNet *cloudstack.Network) (map[string]string, error) { + if resolvedNet.Type == "Shared" { + if err := c.validateIPInCIDR(ip, resolvedNet, networkID); err != nil { + return nil, err + } + + isAvailable, err := c.isIpAvailableInNetwork(ip, networkID) + if err != nil { + return nil, err + } + if !isAvailable { + return nil, errors.Errorf("IP %q is already allocated in network %q or out of range", ip, networkID) + } + } + + return map[string]string{ + "networkid": networkID, + "ip": ip, + }, nil +} + +func (c *client) buildDynamicIPEntry(resolvedNet *cloudstack.Network) (map[string]string, error) { + if resolvedNet.Type == "Shared" { + freeIPExists, err := c.hasFreeIPInNetwork(resolvedNet) + if err != nil { + return nil, err + } + if !freeIPExists { + return nil, errors.Errorf("no free IPs available in network %q", resolvedNet.Id) + } + } + + return map[string]string{ + "networkid": resolvedNet.Id, + }, nil +} + +func (c *client) resolveNetworkByName(name string) (*cloudstack.Network, error) { net, count, err := c.cs.Network.GetNetworkByName(name, cloudstack.WithProject(c.user.Project.ID)) if err != nil { - return "", errors.Wrapf(err, "failed to look up network %q", name) + return nil, errors.Wrapf(err, "failed to look up network %q", name) } if count != 1 { - return "", errors.Errorf("expected 1 network named %q, but got %d", name, count) + return nil, errors.Errorf("expected 1 network named %q, but got %d", name, count) } - return net.Id, nil + return net, nil } func (c *client) buildIPToNetworkList(csMachine *infrav1.CloudStackMachine) ([]map[string]string, error) { - ipToNetworkList := []map[string]string{} + var ipToNetworkList []map[string]string for _, net := range csMachine.Spec.Networks { - networkID := net.ID - if networkID == "" { - var err error - networkID, err = c.resolveNetworkIDByName(net.Name) + networkID, resolvedNet, err := c.resolveNetworkReference(net) + if err != nil { + return nil, err + } + + var entry map[string]string + if net.IP != "" { + entry, err = c.buildStaticIPEntry(net.IP, networkID, resolvedNet) + if err != nil { + return nil, err + } + } else { + entry, err = c.buildDynamicIPEntry(resolvedNet) if err != nil { return nil, err } } - entry := map[string]string{"networkid": networkID} - if net.IP != "" { - entry["ip"] = net.IP - } + ipToNetworkList = append(ipToNetworkList, entry) } + return ipToNetworkList, nil } +func (c *client) resolveNetworkReference(net infrav1.NetworkSpec) (string, *cloudstack.Network, error) { + if net.ID == "" { + resolvedNet, err := c.resolveNetworkByName(net.Name) + if err != nil { + return "", nil, err + } + return resolvedNet.Id, resolvedNet, nil + } + + resolvedNet, _, err := c.cs.Network.GetNetworkByID(net.ID, cloudstack.WithProject(c.user.Project.ID)) + if err != nil { + return "", nil, errors.Wrapf(err, "failed to get network %q by ID", net.ID) + } + return net.ID, resolvedNet, nil +} + +func (c *client) validateIPInCIDR(ipStr string, net *cloudstack.Network, netID string) error { + if net == nil { + return errors.Errorf("network details not found for validation") + } + + ip := netpkg.ParseIP(ipStr) + if ip == nil { + return errors.Errorf("invalid IP address %q", ipStr) + } + + _, cidr, err := netpkg.ParseCIDR(net.Cidr) + if err != nil { + return errors.Wrapf(err, "invalid CIDR %q for network %q", net.Cidr, netID) + } + + if !cidr.Contains(ip) { + return errors.Errorf("IP %q is not within network CIDR %q", ipStr, net.Cidr) + } + + return nil +} + // DeployVM will create a VM instance, // and sets the infrastructure machine spec and status accordingly. func (c *client) DeployVM( @@ -366,6 +493,16 @@ func (c *client) DeployVM( if len(csMachine.Spec.Networks) == 0 && fd.Spec.Zone.Network.ID != "" { p.SetNetworkids([]string{fd.Spec.Zone.Network.ID}) } else { + firstNetwork := csMachine.Spec.Networks[0] + zoneNet := fd.Spec.Zone.Network + + if zoneNet.ID != "" && firstNetwork.ID != "" && firstNetwork.ID != zoneNet.ID { + return errors.Errorf("first network ID %q does not match zone network ID %q", firstNetwork.ID, zoneNet.ID) + } + if zoneNet.Name != "" && firstNetwork.Name != "" && firstNetwork.Name != zoneNet.Name { + return errors.Errorf("first network name %q does not match zone network name %q", firstNetwork.Name, zoneNet.Name) + } + ipToNetworkList, err := c.buildIPToNetworkList(csMachine) if err != nil { return err From 73243b52cb69968f0742ffac6f2a3dd62efbaa3f Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 00:37:25 +0530 Subject: [PATCH 03/12] Added template with multuple networks configuration --- .../cluster-template-multiple-networks.yaml | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 templates/cluster-template-multiple-networks.yaml diff --git a/templates/cluster-template-multiple-networks.yaml b/templates/cluster-template-multiple-networks.yaml new file mode 100644 index 00000000..24b57425 --- /dev/null +++ b/templates/cluster-template-multiple-networks.yaml @@ -0,0 +1,140 @@ +--- +apiVersion: cluster.x-k8s.io/v1beta1 +kind: Cluster +metadata: + name: ${CLUSTER_NAME} +spec: + clusterNetwork: + pods: + cidrBlocks: + - 192.168.0.0/16 + serviceDomain: "cluster.local" + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 + kind: CloudStackCluster + name: ${CLUSTER_NAME} + controlPlaneRef: + kind: KubeadmControlPlane + apiVersion: controlplane.cluster.x-k8s.io/v1beta1 + name: ${CLUSTER_NAME}-control-plane +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 +kind: CloudStackCluster +metadata: + name: ${CLUSTER_NAME} +spec: + syncWithACS: ${CLOUDSTACK_SYNC_WITH_ACS=false} + controlPlaneEndpoint: + host: ${CLUSTER_ENDPOINT_IP} + port: ${CLUSTER_ENDPOINT_PORT=6443} + failureDomains: + - name: ${CLOUDSTACK_FD1_NAME=failure-domain-1} + acsEndpoint: + name: ${CLOUDSTACK_FD1_SECRET_NAME=cloudstack-credentials} + namespace: ${CLOUDSTACK_FD1_SECRET_NAMESPACE=default} + zone: + name: ${CLOUDSTACK_ZONE_NAME} + network: + name: ${CLOUDSTACK_NETWORK_NAME} +--- +kind: KubeadmControlPlane +apiVersion: controlplane.cluster.x-k8s.io/v1beta1 +metadata: + name: "${CLUSTER_NAME}-control-plane" +spec: + kubeadmConfigSpec: + initConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" + joinConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" + preKubeadmCommands: + - swapoff -a + machineTemplate: + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 + kind: CloudStackMachineTemplate + name: "${CLUSTER_NAME}-control-plane" + replicas: ${CONTROL_PLANE_MACHINE_COUNT} + version: ${KUBERNETES_VERSION} +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 +kind: CloudStackMachineTemplate +metadata: + name: ${CLUSTER_NAME}-control-plane +spec: + template: + spec: + sshKey: ${CLOUDSTACK_SSH_KEY_NAME} + offering: + name: ${CLOUDSTACK_CONTROL_PLANE_MACHINE_OFFERING} + networks: + - name: ${CLOUDSTACK_NETWORK_NAME} # default primary network and should match with failureDomains.zone.network.name + ip: 10.1.1.21 # optional ip address in the network + - name: # extra network, it can be network name or ID + ip: 10.1.1.31 # optional ip address in the network + - id: # extra network, it can be network name or ID + template: + name: ${CLOUDSTACK_TEMPLATE_NAME} +--- +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachineDeployment +metadata: + name: "${CLUSTER_NAME}-md-0" +spec: + clusterName: "${CLUSTER_NAME}" + replicas: ${WORKER_MACHINE_COUNT} + selector: + matchLabels: null + template: + spec: + clusterName: "${CLUSTER_NAME}" + version: "${KUBERNETES_VERSION}" + bootstrap: + configRef: + name: "${CLUSTER_NAME}-md-0" + apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 + kind: KubeadmConfigTemplate + infrastructureRef: + name: "${CLUSTER_NAME}-md-0" + apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 + kind: CloudStackMachineTemplate +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 +kind: CloudStackMachineTemplate +metadata: + name: ${CLUSTER_NAME}-md-0 +spec: + template: + spec: + sshKey: ${CLOUDSTACK_SSH_KEY_NAME} + offering: + name: ${CLOUDSTACK_WORKER_MACHINE_OFFERING} + template: + name: ${CLOUDSTACK_TEMPLATE_NAME} + networks: + - name: ${CLOUDSTACK_NETWORK_NAME} # default primary network and should match with failureDomains.zone.network.name + ip: 10.1.1.41 # optional ip address in the network + - name: # extra network, it can be network name or ID + ip: 10.1.1.51 # optional ip address in the network + - id: # extra network, it can be network name or ID +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 +kind: KubeadmConfigTemplate +metadata: + name: ${CLUSTER_NAME}-md-0 +spec: + template: + spec: + joinConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" + preKubeadmCommands: + - swapoff -a From 4caa565b325a1066656a773fd1d8419458c7d634 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 09:00:18 +0530 Subject: [PATCH 04/12] Add cidr check for isolated networks also --- pkg/cloud/instance.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 421572cc..3343add7 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -356,11 +356,11 @@ func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, erro } func (c *client) buildStaticIPEntry(ip, networkID string, resolvedNet *cloudstack.Network) (map[string]string, error) { - if resolvedNet.Type == "Shared" { - if err := c.validateIPInCIDR(ip, resolvedNet, networkID); err != nil { - return nil, err - } + if err := c.validateIPInCIDR(ip, resolvedNet, networkID); err != nil { + return nil, err + } + if resolvedNet.Type == "Shared" { isAvailable, err := c.isIpAvailableInNetwork(ip, networkID) if err != nil { return nil, err From ff2c11a81993e926375494f1902db66294bedf0c Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 10:48:54 +0530 Subject: [PATCH 05/12] Refactoring code --- pkg/cloud/instance.go | 56 +++++++++---------- .../cluster-template-multiple-networks.yaml | 28 ++++++---- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 3343add7..8c71e186 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -312,20 +312,31 @@ func (c *client) CheckLimits( return nil } -func (c *client) isIpAvailableInNetwork(ip, networkID string) (bool, error) { +func (c *client) listPublicIPsInNetwork(networkID, ip string) ([]*cloudstack.PublicIpAddress, error) { params := c.cs.Address.NewListPublicIpAddressesParams() params.SetNetworkid(networkID) - params.SetIpaddress(ip) params.SetAllocatedonly(false) params.SetForvirtualnetwork(false) params.SetListall(true) + if ip != "" { + params.SetIpaddress(ip) + } resp, err := c.cs.Address.ListPublicIpAddresses(params) if err != nil { - return false, errors.Wrapf(err, "failed to list public IP addresses for network %q", networkID) + return nil, errors.Wrapf(err, "failed to list public IP addresses for network %q", networkID) + } + + return resp.PublicIpAddresses, nil +} + +func (c *client) isIPAvailableInNetwork(ip, networkID string) (bool, error) { + publicIPs, err := c.listPublicIPsInNetwork(networkID, ip) + if err != nil { + return false, err } - for _, addr := range resp.PublicIpAddresses { + for _, addr := range publicIPs { if addr.State == "Free" { return true, nil } @@ -335,18 +346,12 @@ func (c *client) isIpAvailableInNetwork(ip, networkID string) (bool, error) { } func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, error) { - params := c.cs.Address.NewListPublicIpAddressesParams() - params.SetNetworkid(resolvedNet.Id) - params.SetAllocatedonly(false) - params.SetForvirtualnetwork(false) - params.SetListall(true) - - resp, err := c.cs.Address.ListPublicIpAddresses(params) + publicIPs, err := c.listPublicIPsInNetwork(resolvedNet.Id, "") if err != nil { - return false, errors.Wrapf(err, "failed to check free IPs for network %q", resolvedNet.Id) + return false, err } - for _, addr := range resp.PublicIpAddresses { + for _, addr := range publicIPs { if addr.State == "Free" { return true, nil } @@ -355,13 +360,14 @@ func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, erro return false, nil } -func (c *client) buildStaticIPEntry(ip, networkID string, resolvedNet *cloudstack.Network) (map[string]string, error) { +func (c *client) buildStaticIPEntry(ip string, resolvedNet *cloudstack.Network) (map[string]string, error) { + networkID := resolvedNet.Id if err := c.validateIPInCIDR(ip, resolvedNet, networkID); err != nil { return nil, err } if resolvedNet.Type == "Shared" { - isAvailable, err := c.isIpAvailableInNetwork(ip, networkID) + isAvailable, err := c.isIPAvailableInNetwork(ip, networkID) if err != nil { return nil, err } @@ -407,14 +413,14 @@ func (c *client) buildIPToNetworkList(csMachine *infrav1.CloudStackMachine) ([]m var ipToNetworkList []map[string]string for _, net := range csMachine.Spec.Networks { - networkID, resolvedNet, err := c.resolveNetworkReference(net) + resolvedNet, err := c.resolveNetwork(net) if err != nil { return nil, err } var entry map[string]string if net.IP != "" { - entry, err = c.buildStaticIPEntry(net.IP, networkID, resolvedNet) + entry, err = c.buildStaticIPEntry(net.IP, resolvedNet) if err != nil { return nil, err } @@ -431,27 +437,19 @@ func (c *client) buildIPToNetworkList(csMachine *infrav1.CloudStackMachine) ([]m return ipToNetworkList, nil } -func (c *client) resolveNetworkReference(net infrav1.NetworkSpec) (string, *cloudstack.Network, error) { +func (c *client) resolveNetwork(net infrav1.NetworkSpec) (*cloudstack.Network, error) { if net.ID == "" { - resolvedNet, err := c.resolveNetworkByName(net.Name) - if err != nil { - return "", nil, err - } - return resolvedNet.Id, resolvedNet, nil + return c.resolveNetworkByName(net.Name) } resolvedNet, _, err := c.cs.Network.GetNetworkByID(net.ID, cloudstack.WithProject(c.user.Project.ID)) if err != nil { - return "", nil, errors.Wrapf(err, "failed to get network %q by ID", net.ID) + return nil, errors.Wrapf(err, "failed to get network %q by ID", net.ID) } - return net.ID, resolvedNet, nil + return resolvedNet, nil } func (c *client) validateIPInCIDR(ipStr string, net *cloudstack.Network, netID string) error { - if net == nil { - return errors.Errorf("network details not found for validation") - } - ip := netpkg.ParseIP(ipStr) if ip == nil { return errors.Errorf("invalid IP address %q", ipStr) diff --git a/templates/cluster-template-multiple-networks.yaml b/templates/cluster-template-multiple-networks.yaml index 24b57425..bb8c5d8d 100644 --- a/templates/cluster-template-multiple-networks.yaml +++ b/templates/cluster-template-multiple-networks.yaml @@ -74,11 +74,15 @@ spec: offering: name: ${CLOUDSTACK_CONTROL_PLANE_MACHINE_OFFERING} networks: - - name: ${CLOUDSTACK_NETWORK_NAME} # default primary network and should match with failureDomains.zone.network.name - ip: 10.1.1.21 # optional ip address in the network - - name: # extra network, it can be network name or ID - ip: 10.1.1.31 # optional ip address in the network - - id: # extra network, it can be network name or ID + - name: ${CLOUDSTACK_NETWORK_NAME} # (optional) default primary network; must match failureDomains.zone.network.name + # ip: 10.1.1.21 # (optional) static IP in the primary network + + # Additional (extra) networks can be specified below. Use either 'name' or 'id', and optionally an 'ip'. + # - name: isolated-net2 # (optional) extra network by name + # ip: 10.1.1.31 # (optional) static IP in this network + + # - id: a1b2c3d4-5678-90ef-gh12-3456789ijklm # (optional) extra network by ID + # ip: 10.1.1.32 # (optional) static IP in this network template: name: ${CLOUDSTACK_TEMPLATE_NAME} --- @@ -118,11 +122,15 @@ spec: template: name: ${CLOUDSTACK_TEMPLATE_NAME} networks: - - name: ${CLOUDSTACK_NETWORK_NAME} # default primary network and should match with failureDomains.zone.network.name - ip: 10.1.1.41 # optional ip address in the network - - name: # extra network, it can be network name or ID - ip: 10.1.1.51 # optional ip address in the network - - id: # extra network, it can be network name or ID + - name: ${CLOUDSTACK_NETWORK_NAME} # (optional) default primary network; must match failureDomains.zone.network.name + # ip: 10.1.1.41 # (optional) static IP in the primary network + + # Additional (extra) networks can be specified below. Use either 'name' or 'id', and optionally an 'ip'. + # - name: isolated-net3 # (optional) extra network by name + # ip: 10.1.1.51 # (optional) static IP in this network + + # - id: f5c93c15-d27b-4c93-b27c-00f48226b9a9 # (optional) extra network by ID + # ip: 10.1.1.52 # (optional) static IP in this network --- apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 kind: KubeadmConfigTemplate From a6cbcff77ae741d09950d10a0ff5543afec8f49d Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 11:02:09 +0530 Subject: [PATCH 06/12] more refactoring --- pkg/cloud/instance.go | 72 ++++++++++++------------------------------- 1 file changed, 20 insertions(+), 52 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 8c71e186..1a7265e2 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -312,46 +312,23 @@ func (c *client) CheckLimits( return nil } -func (c *client) listPublicIPsInNetwork(networkID, ip string) ([]*cloudstack.PublicIpAddress, error) { +func (c *client) isFreeIPAvailable(networkID, ip string) (bool, error) { params := c.cs.Address.NewListPublicIpAddressesParams() params.SetNetworkid(networkID) params.SetAllocatedonly(false) params.SetForvirtualnetwork(false) params.SetListall(true) + if ip != "" { params.SetIpaddress(ip) } resp, err := c.cs.Address.ListPublicIpAddresses(params) if err != nil { - return nil, errors.Wrapf(err, "failed to list public IP addresses for network %q", networkID) - } - - return resp.PublicIpAddresses, nil -} - -func (c *client) isIPAvailableInNetwork(ip, networkID string) (bool, error) { - publicIPs, err := c.listPublicIPsInNetwork(networkID, ip) - if err != nil { - return false, err - } - - for _, addr := range publicIPs { - if addr.State == "Free" { - return true, nil - } - } - - return false, nil -} - -func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, error) { - publicIPs, err := c.listPublicIPsInNetwork(resolvedNet.Id, "") - if err != nil { - return false, err + return false, errors.Wrapf(err, "failed to list public IP addresses for network %q", networkID) } - for _, addr := range publicIPs { + for _, addr := range resp.PublicIpAddresses { if addr.State == "Free" { return true, nil } @@ -360,42 +337,33 @@ func (c *client) hasFreeIPInNetwork(resolvedNet *cloudstack.Network) (bool, erro return false, nil } -func (c *client) buildStaticIPEntry(ip string, resolvedNet *cloudstack.Network) (map[string]string, error) { - networkID := resolvedNet.Id - if err := c.validateIPInCIDR(ip, resolvedNet, networkID); err != nil { - return nil, err - } - - if resolvedNet.Type == "Shared" { - isAvailable, err := c.isIPAvailableInNetwork(ip, networkID) - if err != nil { +func (c *client) buildIPEntry(resolvedNet *cloudstack.Network, ip string) (map[string]string, error) { + if ip != "" { + if err := c.validateIPInCIDR(ip, resolvedNet, resolvedNet.Id); err != nil { return nil, err } - if !isAvailable { - return nil, errors.Errorf("IP %q is already allocated in network %q or out of range", ip, networkID) - } } - return map[string]string{ - "networkid": networkID, - "ip": ip, - }, nil -} - -func (c *client) buildDynamicIPEntry(resolvedNet *cloudstack.Network) (map[string]string, error) { if resolvedNet.Type == "Shared" { - freeIPExists, err := c.hasFreeIPInNetwork(resolvedNet) + isAvailable, err := c.isFreeIPAvailable(resolvedNet.Id, ip) if err != nil { return nil, err } - if !freeIPExists { + if !isAvailable { + if ip != "" { + return nil, errors.Errorf("IP %q is already allocated in network %q or out of range", ip, resolvedNet.Id) + } return nil, errors.Errorf("no free IPs available in network %q", resolvedNet.Id) } } - return map[string]string{ + entry := map[string]string{ "networkid": resolvedNet.Id, - }, nil + } + if ip != "" { + entry["ip"] = ip + } + return entry, nil } func (c *client) resolveNetworkByName(name string) (*cloudstack.Network, error) { @@ -420,12 +388,12 @@ func (c *client) buildIPToNetworkList(csMachine *infrav1.CloudStackMachine) ([]m var entry map[string]string if net.IP != "" { - entry, err = c.buildStaticIPEntry(net.IP, resolvedNet) + entry, err = c.buildIPEntry(resolvedNet, net.IP) if err != nil { return nil, err } } else { - entry, err = c.buildDynamicIPEntry(resolvedNet) + entry, err = c.buildIPEntry(resolvedNet, "") if err != nil { return nil, err } From 9ade1818dacb9bb8813ddded48a23fc706c8b0a0 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 11:27:06 +0530 Subject: [PATCH 07/12] Fix lint --- pkg/cloud/instance.go | 65 ++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 1a7265e2..4173b765 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -19,6 +19,7 @@ package cloud import ( "encoding/base64" "fmt" + "net" "strconv" "strings" @@ -31,8 +32,6 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" - - netpkg "net" ) type VMIface interface { @@ -339,7 +338,7 @@ func (c *client) isFreeIPAvailable(networkID, ip string) (bool, error) { func (c *client) buildIPEntry(resolvedNet *cloudstack.Network, ip string) (map[string]string, error) { if ip != "" { - if err := c.validateIPInCIDR(ip, resolvedNet, resolvedNet.Id); err != nil { + if err := validateIPInCIDR(ip, resolvedNet.Cidr); err != nil { return nil, err } } @@ -417,21 +416,51 @@ func (c *client) resolveNetwork(net infrav1.NetworkSpec) (*cloudstack.Network, e return resolvedNet, nil } -func (c *client) validateIPInCIDR(ipStr string, net *cloudstack.Network, netID string) error { - ip := netpkg.ParseIP(ipStr) +func validateIPInCIDR(ipStr, cidrStr string) error { + ip := net.ParseIP(ipStr) if ip == nil { return errors.Errorf("invalid IP address %q", ipStr) } - _, cidr, err := netpkg.ParseCIDR(net.Cidr) + _, cidr, err := net.ParseCIDR(cidrStr) if err != nil { - return errors.Wrapf(err, "invalid CIDR %q for network %q", net.Cidr, netID) + return errors.Wrapf(err, "invalid CIDR %q", cidrStr) } if !cidr.Contains(ip) { - return errors.Errorf("IP %q is not within network CIDR %q", ipStr, net.Cidr) + return errors.Errorf("IP %q is not within network CIDR %q", ipStr, cidrStr) + } + + return nil +} + +func (c *client) configureNetworkParams( + p *cloudstack.DeployVirtualMachineParams, + csMachine *infrav1.CloudStackMachine, + fd *infrav1.CloudStackFailureDomain, +) error { + if len(csMachine.Spec.Networks) == 0 && fd.Spec.Zone.Network.ID != "" { + p.SetNetworkids([]string{fd.Spec.Zone.Network.ID}) + return nil + } + + firstNetwork := csMachine.Spec.Networks[0] + zoneNet := fd.Spec.Zone.Network + + // Validate match between zone network and first template network. + if zoneNet.ID != "" && firstNetwork.ID != "" && firstNetwork.ID != zoneNet.ID { + return errors.Errorf("first network ID %q does not match zone network ID %q", firstNetwork.ID, zoneNet.ID) + } + if zoneNet.Name != "" && firstNetwork.Name != "" && firstNetwork.Name != zoneNet.Name { + return errors.Errorf("first network name %q does not match zone network name %q", firstNetwork.Name, zoneNet.Name) + } + + ipToNetworkList, err := c.buildIPToNetworkList(csMachine) + if err != nil { + return err } + p.SetIptonetworklist(ipToNetworkList) return nil } @@ -456,24 +485,8 @@ func (c *client) DeployVM( p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offering.Id, templateID, fd.Spec.Zone.ID) - if len(csMachine.Spec.Networks) == 0 && fd.Spec.Zone.Network.ID != "" { - p.SetNetworkids([]string{fd.Spec.Zone.Network.ID}) - } else { - firstNetwork := csMachine.Spec.Networks[0] - zoneNet := fd.Spec.Zone.Network - - if zoneNet.ID != "" && firstNetwork.ID != "" && firstNetwork.ID != zoneNet.ID { - return errors.Errorf("first network ID %q does not match zone network ID %q", firstNetwork.ID, zoneNet.ID) - } - if zoneNet.Name != "" && firstNetwork.Name != "" && firstNetwork.Name != zoneNet.Name { - return errors.Errorf("first network name %q does not match zone network name %q", firstNetwork.Name, zoneNet.Name) - } - - ipToNetworkList, err := c.buildIPToNetworkList(csMachine) - if err != nil { - return err - } - p.SetIptonetworklist(ipToNetworkList) + if err := c.configureNetworkParams(p, csMachine, fd); err != nil { + return err } setIfNotEmpty(csMachine.Name, p.SetName) From 40e37e705101be04dafb73ba3e9be610cccc6b77 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 11:40:24 +0530 Subject: [PATCH 08/12] remove yaml file --- .../cluster-template-multiple-networks.yaml | 148 ------------------ 1 file changed, 148 deletions(-) delete mode 100644 templates/cluster-template-multiple-networks.yaml diff --git a/templates/cluster-template-multiple-networks.yaml b/templates/cluster-template-multiple-networks.yaml deleted file mode 100644 index bb8c5d8d..00000000 --- a/templates/cluster-template-multiple-networks.yaml +++ /dev/null @@ -1,148 +0,0 @@ ---- -apiVersion: cluster.x-k8s.io/v1beta1 -kind: Cluster -metadata: - name: ${CLUSTER_NAME} -spec: - clusterNetwork: - pods: - cidrBlocks: - - 192.168.0.0/16 - serviceDomain: "cluster.local" - infrastructureRef: - apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 - kind: CloudStackCluster - name: ${CLUSTER_NAME} - controlPlaneRef: - kind: KubeadmControlPlane - apiVersion: controlplane.cluster.x-k8s.io/v1beta1 - name: ${CLUSTER_NAME}-control-plane ---- -apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 -kind: CloudStackCluster -metadata: - name: ${CLUSTER_NAME} -spec: - syncWithACS: ${CLOUDSTACK_SYNC_WITH_ACS=false} - controlPlaneEndpoint: - host: ${CLUSTER_ENDPOINT_IP} - port: ${CLUSTER_ENDPOINT_PORT=6443} - failureDomains: - - name: ${CLOUDSTACK_FD1_NAME=failure-domain-1} - acsEndpoint: - name: ${CLOUDSTACK_FD1_SECRET_NAME=cloudstack-credentials} - namespace: ${CLOUDSTACK_FD1_SECRET_NAMESPACE=default} - zone: - name: ${CLOUDSTACK_ZONE_NAME} - network: - name: ${CLOUDSTACK_NETWORK_NAME} ---- -kind: KubeadmControlPlane -apiVersion: controlplane.cluster.x-k8s.io/v1beta1 -metadata: - name: "${CLUSTER_NAME}-control-plane" -spec: - kubeadmConfigSpec: - initConfiguration: - nodeRegistration: - name: '{{ local_hostname }}' - kubeletExtraArgs: - provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" - joinConfiguration: - nodeRegistration: - name: '{{ local_hostname }}' - kubeletExtraArgs: - provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" - preKubeadmCommands: - - swapoff -a - machineTemplate: - infrastructureRef: - apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 - kind: CloudStackMachineTemplate - name: "${CLUSTER_NAME}-control-plane" - replicas: ${CONTROL_PLANE_MACHINE_COUNT} - version: ${KUBERNETES_VERSION} ---- -apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 -kind: CloudStackMachineTemplate -metadata: - name: ${CLUSTER_NAME}-control-plane -spec: - template: - spec: - sshKey: ${CLOUDSTACK_SSH_KEY_NAME} - offering: - name: ${CLOUDSTACK_CONTROL_PLANE_MACHINE_OFFERING} - networks: - - name: ${CLOUDSTACK_NETWORK_NAME} # (optional) default primary network; must match failureDomains.zone.network.name - # ip: 10.1.1.21 # (optional) static IP in the primary network - - # Additional (extra) networks can be specified below. Use either 'name' or 'id', and optionally an 'ip'. - # - name: isolated-net2 # (optional) extra network by name - # ip: 10.1.1.31 # (optional) static IP in this network - - # - id: a1b2c3d4-5678-90ef-gh12-3456789ijklm # (optional) extra network by ID - # ip: 10.1.1.32 # (optional) static IP in this network - template: - name: ${CLOUDSTACK_TEMPLATE_NAME} ---- -apiVersion: cluster.x-k8s.io/v1beta1 -kind: MachineDeployment -metadata: - name: "${CLUSTER_NAME}-md-0" -spec: - clusterName: "${CLUSTER_NAME}" - replicas: ${WORKER_MACHINE_COUNT} - selector: - matchLabels: null - template: - spec: - clusterName: "${CLUSTER_NAME}" - version: "${KUBERNETES_VERSION}" - bootstrap: - configRef: - name: "${CLUSTER_NAME}-md-0" - apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 - kind: KubeadmConfigTemplate - infrastructureRef: - name: "${CLUSTER_NAME}-md-0" - apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 - kind: CloudStackMachineTemplate ---- -apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 -kind: CloudStackMachineTemplate -metadata: - name: ${CLUSTER_NAME}-md-0 -spec: - template: - spec: - sshKey: ${CLOUDSTACK_SSH_KEY_NAME} - offering: - name: ${CLOUDSTACK_WORKER_MACHINE_OFFERING} - template: - name: ${CLOUDSTACK_TEMPLATE_NAME} - networks: - - name: ${CLOUDSTACK_NETWORK_NAME} # (optional) default primary network; must match failureDomains.zone.network.name - # ip: 10.1.1.41 # (optional) static IP in the primary network - - # Additional (extra) networks can be specified below. Use either 'name' or 'id', and optionally an 'ip'. - # - name: isolated-net3 # (optional) extra network by name - # ip: 10.1.1.51 # (optional) static IP in this network - - # - id: f5c93c15-d27b-4c93-b27c-00f48226b9a9 # (optional) extra network by ID - # ip: 10.1.1.52 # (optional) static IP in this network ---- -apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 -kind: KubeadmConfigTemplate -metadata: - name: ${CLUSTER_NAME}-md-0 -spec: - template: - spec: - joinConfiguration: - nodeRegistration: - name: '{{ local_hostname }}' - kubeletExtraArgs: - provider-id: "cloudstack:///'{{ ds.meta_data.instance_id }}'" - preKubeadmCommands: - - swapoff -a From 3b6b33a8507712de00a48f9ae533a12defd2882f Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 12:16:43 +0530 Subject: [PATCH 09/12] Added docs --- .../src/clustercloudstack/configuration.md | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/book/src/clustercloudstack/configuration.md b/docs/book/src/clustercloudstack/configuration.md index 0734d0bf..73c51719 100644 --- a/docs/book/src/clustercloudstack/configuration.md +++ b/docs/book/src/clustercloudstack/configuration.md @@ -143,6 +143,45 @@ After setting the environment variables, execute the following command to genera clusterctl generate cluster capc-cluster --flavor with-kube-vip > capc-cluster-spec.yaml ``` +##### Option for Multiple Networks + +Multiple networks can be specified at each node configuration in CloudStackMachineTemplate. +This is configured under spec.template.spec.networks, where you can list one or more networks by name or id, +and optionally assign static IP addresses. + +When defining multiple networks for a VM in CAPC, the first network listed under spec.template.spec.networks is treated as +the primary network. This primary network must match the network defined in the failure domain’s zone (failureDomains[].zone.network), +either by name or by ID. It is used as the default NIC and is critical for VM boot and cluster communication. + +Any networks listed after the primary are considered extra networks. These extra networks are attached as secondary NICs +on the VM and can be used for purposes such as service segmentation or additional routing. Each network entry, primary or +extra can optionally include a static IP address. If an IP is not specified, CloudStack will dynamically allocate one. + +For example: + +```yaml +apiVersion: infrastructure.cluster.x-k8s.io/v1beta3 +kind: CloudStackMachineTemplate +metadata: + name: capc-cluster-control-plane + namespace: default +spec: + template: + spec: + offering: + name: Large Instance + networks: + - name: cloudstack-network # (optional) default primary network; must match with network at failureDomains.zone.network.name + ip: 10.1.1.21 # (optional) static IP in the primary network + + # Additional (extra) networks can be specified below. Use either 'name' or 'id', and optionally an 'ip'. + - name: cloudstack-network-2 # (optional) extra network by name + ip: 10.1.1.31 # (optional) static IP in this network + + - id: a1b2c3d4-5678-90ef-gh12-3456789ijklm # (optional) extra network by ID + ip: 10.1.1.41 # (optional) static IP in this network +``` + #### CloudStack Endpoint Credentials Secret (*optional for provided templates when used with provided getting-started process*) A reference to a Kubernetes Secret containing a YAML object containing credentials for accessing a particular CloudStack From aefdd492e2e2049b30738bcbefc395b84fea3dd0 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 12:51:20 +0530 Subject: [PATCH 10/12] Fix tests --- pkg/cloud/instance.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 4173b765..f7d19946 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -439,28 +439,28 @@ func (c *client) configureNetworkParams( csMachine *infrav1.CloudStackMachine, fd *infrav1.CloudStackFailureDomain, ) error { - if len(csMachine.Spec.Networks) == 0 && fd.Spec.Zone.Network.ID != "" { - p.SetNetworkids([]string{fd.Spec.Zone.Network.ID}) - return nil - } - - firstNetwork := csMachine.Spec.Networks[0] - zoneNet := fd.Spec.Zone.Network + if len(csMachine.Spec.Networks) == 0 { + if fd.Spec.Zone.Network.ID != "" { + p.SetNetworkids([]string{fd.Spec.Zone.Network.ID}) + } + } else { + firstNetwork := csMachine.Spec.Networks[0] + zoneNet := fd.Spec.Zone.Network - // Validate match between zone network and first template network. - if zoneNet.ID != "" && firstNetwork.ID != "" && firstNetwork.ID != zoneNet.ID { - return errors.Errorf("first network ID %q does not match zone network ID %q", firstNetwork.ID, zoneNet.ID) - } - if zoneNet.Name != "" && firstNetwork.Name != "" && firstNetwork.Name != zoneNet.Name { - return errors.Errorf("first network name %q does not match zone network name %q", firstNetwork.Name, zoneNet.Name) - } + if zoneNet.ID != "" && firstNetwork.ID != "" && firstNetwork.ID != zoneNet.ID { + return errors.Errorf("first network ID %q does not match zone network ID %q", firstNetwork.ID, zoneNet.ID) + } + if zoneNet.Name != "" && firstNetwork.Name != "" && firstNetwork.Name != zoneNet.Name { + return errors.Errorf("first network name %q does not match zone network name %q", firstNetwork.Name, zoneNet.Name) + } - ipToNetworkList, err := c.buildIPToNetworkList(csMachine) - if err != nil { - return err + ipToNetworkList, err := c.buildIPToNetworkList(csMachine) + if err != nil { + return err + } + p.SetIptonetworklist(ipToNetworkList) } - p.SetIptonetworklist(ipToNetworkList) return nil } From 27ac17fd9e5eef0a1c3d18e22ee19db348cd7553 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 13:26:28 +0530 Subject: [PATCH 11/12] use const instead of hardcoded string --- pkg/cloud/instance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index f7d19946..82ae2eb9 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -343,7 +343,7 @@ func (c *client) buildIPEntry(resolvedNet *cloudstack.Network, ip string) (map[s } } - if resolvedNet.Type == "Shared" { + if resolvedNet.Type == NetworkTypeShared { isAvailable, err := c.isFreeIPAvailable(resolvedNet.Id, ip) if err != nil { return nil, err From 054530e363a722aa2cb761f2e13960b81716d951 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 8 Jul 2025 16:08:26 +0530 Subject: [PATCH 12/12] Redundant calls and formatting --- docs/book/src/clustercloudstack/configuration.md | 6 +++--- pkg/cloud/instance.go | 14 +++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/docs/book/src/clustercloudstack/configuration.md b/docs/book/src/clustercloudstack/configuration.md index 73c51719..1c8b0c6a 100644 --- a/docs/book/src/clustercloudstack/configuration.md +++ b/docs/book/src/clustercloudstack/configuration.md @@ -146,11 +146,11 @@ clusterctl generate cluster capc-cluster --flavor with-kube-vip > capc-cluster-s ##### Option for Multiple Networks Multiple networks can be specified at each node configuration in CloudStackMachineTemplate. -This is configured under spec.template.spec.networks, where you can list one or more networks by name or id, +This is configured under `spec.template.spec.networks`, where you can list one or more networks by name or id, and optionally assign static IP addresses. -When defining multiple networks for a VM in CAPC, the first network listed under spec.template.spec.networks is treated as -the primary network. This primary network must match the network defined in the failure domain’s zone (failureDomains[].zone.network), +When defining multiple networks for a VM in CAPC, the first network listed under `spec.template.spec.networks` is treated as +the primary network. This primary network must match the network defined in the failure domain’s zone (`failureDomains[].zone.network`), either by name or by ID. It is used as the default NIC and is critical for VM boot and cluster communication. Any networks listed after the primary are considered extra networks. These extra networks are attached as secondary NICs diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index 82ae2eb9..ad6a7cdc 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -385,17 +385,9 @@ func (c *client) buildIPToNetworkList(csMachine *infrav1.CloudStackMachine) ([]m return nil, err } - var entry map[string]string - if net.IP != "" { - entry, err = c.buildIPEntry(resolvedNet, net.IP) - if err != nil { - return nil, err - } - } else { - entry, err = c.buildIPEntry(resolvedNet, "") - if err != nil { - return nil, err - } + entry, err := c.buildIPEntry(resolvedNet, net.IP) + if err != nil { + return nil, err } ipToNetworkList = append(ipToNetworkList, entry)