diff --git a/api/v1beta2/ibmpowervscluster_types.go b/api/v1beta2/ibmpowervscluster_types.go index 17a37392d..6b58d9e45 100644 --- a/api/v1beta2/ibmpowervscluster_types.go +++ b/api/v1beta2/ibmpowervscluster_types.go @@ -42,7 +42,7 @@ type IBMPowerVSClusterSpec struct { // 1. in the case of DHCPServer.Name is not set the name will be DHCPSERVER_Private. // 2. if DHCPServer.Name is set the name will be DHCPSERVER_Private. // when Network.ID is set, its expected that there exist a network in PowerVS workspace with id or else system will give error. - // when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist network will be created by DHCP service. + // when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist system will check DHCP network with given Network.name, if that also not exist, it will create a new DHCP service and name will be DHCPSERVER_Private. // Network.RegEx is not yet supported and system will ignore the value. Network IBMPowerVSResourceReference `json:"network"` diff --git a/cloud/scope/powervs_cluster.go b/cloud/scope/powervs_cluster.go index 536bad1d3..9bfcfe05f 100644 --- a/cloud/scope/powervs_cluster.go +++ b/cloud/scope/powervs_cluster.go @@ -865,10 +865,11 @@ func (s *PowerVSClusterScope) createServiceInstance() (*resourcecontrollerv2.Res } // ReconcileNetwork reconciles network -// If only IBMPowerVSCluster.Spec.Network is set, network would be validated and if exists already will get used as cluster’s network or a new network will be created via DHCP service. +// If only IBMPowerVSCluster.Spec.Network is set, network would be validated and if exists already will get used as cluster’s network or DHCP network would be validated with this name if not exits then a new network will be created via DHCP service. // If only IBMPowerVSCluster.Spec.DHCPServer is set, DHCP server would be validated and if exists already, will use DHCP server’s network as cluster network. If not a new DHCP service will be created and it’s network will be used. -// If both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer is set, network and DHCP server would be validated and if both exists already then network is belongs to given DHCP server or not would be validated. -// If both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer is not set, by default DHCP service will be created to setup cluster's network. +// Cannot set both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer since it will cause collision during network creation if both are provided. +// If both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer is not set, by default DHCP service will be created with the cluster name to setup cluster's network. +// Note: DHCP network name would be in `DHCPSERVER_Private` this format. func (s *PowerVSClusterScope) ReconcileNetwork() (bool, error) { if s.GetNetworkID() != nil { // Check the network exists @@ -949,12 +950,7 @@ func (s *PowerVSClusterScope) checkDHCPServer() (*string, error) { } // if user provides DHCP server name then we can use network name to match the existing DHCP server - var networkName string - if s.DHCPServer() != nil && s.DHCPServer().Name != nil { - networkName = dhcpNetworkName(*s.DHCPServer().Name) - } else { - networkName = dhcpNetworkName(s.InfraCluster()) - } + networkName := dhcpNetworkName(*s.GetServiceName(infrav1beta2.ResourceTypeDHCPServer)) s.V(3).Info("Checking DHCP server's network list by network name", "name", networkName) dhcpServers, err := s.IBMPowerVSClient.GetAllDHCPServers() @@ -2466,11 +2462,14 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT return ptr.To(fmt.Sprintf("%s-serviceInstance", s.InfraCluster())) } return s.ServiceInstance().Name - case infrav1beta2.ResourceTypeNetwork: - if s.Network() == nil || s.Network().Name == nil { - return ptr.To(fmt.Sprintf("DHCPSERVER%s_Private", s.InfraCluster())) + case infrav1beta2.ResourceTypeDHCPServer: + if s.DHCPServer() != nil && s.DHCPServer().Name != nil { + return s.DHCPServer().Name } - return s.Network().Name + if s.Network() != nil && s.Network().Name != nil { + return s.Network().Name + } + return ptr.To(s.InfraCluster()) case infrav1beta2.ResourceTypeVPC: if s.VPC() == nil || s.VPC().Name == nil { return ptr.To(fmt.Sprintf("%s-vpc", s.InfraCluster())) @@ -2481,11 +2480,6 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT return ptr.To(fmt.Sprintf("%s-transitgateway", s.InfraCluster())) } return s.TransitGateway().Name - case infrav1beta2.ResourceTypeDHCPServer: - if s.DHCPServer() == nil || s.DHCPServer().Name == nil { - return ptr.To(s.InfraCluster()) - } - return s.DHCPServer().Name case infrav1beta2.ResourceTypeCOSInstance: if s.COSInstance() == nil || s.COSInstance().Name == "" { return ptr.To(fmt.Sprintf("%s-cosinstance", s.InfraCluster())) diff --git a/cloud/scope/powervs_cluster_test.go b/cloud/scope/powervs_cluster_test.go index 8cb7e8f0e..01569dfcf 100644 --- a/cloud/scope/powervs_cluster_test.go +++ b/cloud/scope/powervs_cluster_test.go @@ -2766,22 +2766,6 @@ func TestGetServiceName(t *testing.T) { }, expectedName: ptr.To("ServiceInstanceName"), }, - { - name: "Resource type is network and Network is nil", - resourceType: infrav1beta2.ResourceTypeNetwork, - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}}, - }, - expectedName: ptr.To("DHCPSERVERClusterName_Private"), - }, - { - name: "Resource type is network and Network is not nil", - resourceType: infrav1beta2.ResourceTypeNetwork, - clusterScope: PowerVSClusterScope{ - IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{Spec: infrav1beta2.IBMPowerVSClusterSpec{Network: infrav1beta2.IBMPowerVSResourceReference{Name: ptr.To("NetworkName")}}}, - }, - expectedName: ptr.To("NetworkName"), - }, { name: "Resource type is vpc and VPC is nil", resourceType: infrav1beta2.ResourceTypeVPC, @@ -2830,6 +2814,14 @@ func TestGetServiceName(t *testing.T) { }, expectedName: ptr.To("DHCPServerName"), }, + { + name: "Resource type is dhcp server and dhcpserver is not nil and network is not nil", + resourceType: infrav1beta2.ResourceTypeDHCPServer, + clusterScope: PowerVSClusterScope{ + IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{Spec: infrav1beta2.IBMPowerVSClusterSpec{Network: infrav1beta2.IBMPowerVSResourceReference{Name: ptr.To("NetworkName")}}}, + }, + expectedName: ptr.To("NetworkName"), + }, { name: "Resource type is cos instance and cos instance is nil", resourceType: infrav1beta2.ResourceTypeCOSInstance, diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml index 7094b9e1a..397c92a28 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml @@ -460,7 +460,7 @@ spec: 1. in the case of DHCPServer.Name is not set the name will be DHCPSERVER_Private. 2. if DHCPServer.Name is set the name will be DHCPSERVER_Private. when Network.ID is set, its expected that there exist a network in PowerVS workspace with id or else system will give error. - when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist network will be created by DHCP service. + when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist system will check DHCP network with given Network.name, if that also not exist, it will create a new DHCP service and name will be DHCPSERVER_Private. Network.RegEx is not yet supported and system will ignore the value. properties: id: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml index 149570c39..5ff3be0ac 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml @@ -493,7 +493,7 @@ spec: 1. in the case of DHCPServer.Name is not set the name will be DHCPSERVER_Private. 2. if DHCPServer.Name is set the name will be DHCPSERVER_Private. when Network.ID is set, its expected that there exist a network in PowerVS workspace with id or else system will give error. - when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist network will be created by DHCP service. + when Network.Name is set, system will first check for network with Name in PowerVS workspace, if not exist system will check DHCP network with given Network.name, if that also not exist, it will create a new DHCP service and name will be DHCPSERVER_Private. Network.RegEx is not yet supported and system will ignore the value. properties: id: diff --git a/internal/webhooks/ibmpowervscluster_test.go b/internal/webhooks/ibmpowervscluster_test.go index 3c48b7655..8d6c287d4 100644 --- a/internal/webhooks/ibmpowervscluster_test.go +++ b/internal/webhooks/ibmpowervscluster_test.go @@ -70,6 +70,66 @@ func TestIBMPowerVSCluster_create(t *testing.T) { }, wantErr: true, }, + { + name: "Should error if both Network name and DHCP name are set", + powervsCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstanceID: "capi-si-id", + Network: infrav1beta2.IBMPowerVSResourceReference{ + Name: ptr.To("capi-net"), + }, + DHCPServer: &infrav1beta2.DHCPServer{ + Name: ptr.To("capi-dhcp"), + }, + }, + }, + wantErr: true, + }, + { + name: "Should error if both Network id and DHCP name are set", + powervsCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstanceID: "capi-si-id", + Network: infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("capi-net-id"), + }, + DHCPServer: &infrav1beta2.DHCPServer{ + Name: ptr.To("capi-dhcp"), + }, + }, + }, + wantErr: true, + }, + { + name: "Should error if both Network name and DHCP id are set", + powervsCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstanceID: "capi-si-id", + Network: infrav1beta2.IBMPowerVSResourceReference{ + Name: ptr.To("capi-net"), + }, + DHCPServer: &infrav1beta2.DHCPServer{ + ID: ptr.To("capi-dhcp-id"), + }, + }, + }, + wantErr: true, + }, + { + name: "Should error if both Network id and DHCP id are set", + powervsCluster: &infrav1beta2.IBMPowerVSCluster{ + Spec: infrav1beta2.IBMPowerVSClusterSpec{ + ServiceInstanceID: "capi-si-id", + Network: infrav1beta2.IBMPowerVSResourceReference{ + ID: ptr.To("capi-net-id"), + }, + DHCPServer: &infrav1beta2.DHCPServer{ + ID: ptr.To("capi-dhcp-id"), + }, + }, + }, + wantErr: true, + }, } for _, tc := range tests {