Fix priority between network and DHCP server

Do not allow to set both network and DHCP server details which will create conflict while creating new DHCP server on whether to use network name or DHCP server name

Author: dharaneeshvrd

Diff for: api/v1beta2/ibmpowervscluster_webhook.go

@@ -99,6 +99,12 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterNetwork() *field.Error {
 	if res, err := validateIBMPowerVSNetworkReference(r.Spec.Network); !res {
 		return err
 	}
+	if (r.Spec.Network.Name != nil || r.Spec.Network.ID != nil) && (r.Spec.DHCPServer != nil && r.Spec.DHCPServer.Name != nil) {
+		return field.Invalid(field.NewPath("spec.dhcpServer.name"), r.Spec.DHCPServer.Name, "either one of network or dhcpServer details can be provided")
+	}
+	if (r.Spec.Network.Name != nil || r.Spec.Network.ID != nil) && (r.Spec.DHCPServer != nil && r.Spec.DHCPServer.ID != nil) {
+		return field.Invalid(field.NewPath("spec.dhcpServer.id"), r.Spec.DHCPServer.ID, "either one of network or dhcpServer details can be provided")
+	}
 	return nil
 }
 

Diff for: api/v1beta2/ibmpowervscluster_webhook_test.go

@@ -68,6 +68,36 @@ func TestIBMPowerVSCluster_create(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Should error if both Network name and DHCP are set",
+			powervsCluster: &IBMPowerVSCluster{
+				Spec: IBMPowerVSClusterSpec{
+					ServiceInstanceID: "capi-si-id",
+					Network: IBMPowerVSResourceReference{
+						Name: ptr.To("capi-net"),
+					},
+					DHCPServer: &DHCPServer{
+						Name: ptr.To("capi-dhcp"),
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Should error if both Network id and DHCP are set",
+			powervsCluster: &IBMPowerVSCluster{
+				Spec: IBMPowerVSClusterSpec{
+					ServiceInstanceID: "capi-si-id",
+					Network: IBMPowerVSResourceReference{
+						ID: ptr.To("capi-net-id"),
+					},
+					DHCPServer: &DHCPServer{
+						Name: ptr.To("capi-dhcp"),
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 
 	for _, tc := range tests {

Diff for: cloud/scope/powervs_cluster.go

@@ -949,12 +949,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 +2461,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 +2479,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 for: 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,