Skip to content

Commit 09862db

Browse files
dharaneeshvrddharaneeshvrd
committed
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
1 parent d3cc590 commit 09862db

File tree

4 files changed

+85
-34
lines changed

4 files changed

+85
-34
lines changed

Diff for: ‎api/v1beta2/ibmpowervscluster_webhook.go

+6
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterNetwork() *field.Error {
9999
if res, err := validateIBMPowerVSNetworkReference(r.Spec.Network); !res {
100100
return err
101101
}
102+
if (r.Spec.Network.Name != nil || r.Spec.Network.ID != nil) && (r.Spec.DHCPServer != nil && r.Spec.DHCPServer.Name != nil) {
103+
return field.Invalid(field.NewPath("spec.dhcpServer.name"), r.Spec.DHCPServer.Name, "either one of network or dhcpServer details can be provided")
104+
}
105+
if (r.Spec.Network.Name != nil || r.Spec.Network.ID != nil) && (r.Spec.DHCPServer != nil && r.Spec.DHCPServer.ID != nil) {
106+
return field.Invalid(field.NewPath("spec.dhcpServer.id"), r.Spec.DHCPServer.ID, "either one of network or dhcpServer details can be provided")
107+
}
102108
return nil
103109
}
104110

Diff for: ‎api/v1beta2/ibmpowervscluster_webhook_test.go

+60
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,66 @@ func TestIBMPowerVSCluster_create(t *testing.T) {
6868
},
6969
wantErr: true,
7070
},
71+
{
72+
name: "Should error if both Network name and DHCP name are set",
73+
powervsCluster: &IBMPowerVSCluster{
74+
Spec: IBMPowerVSClusterSpec{
75+
ServiceInstanceID: "capi-si-id",
76+
Network: IBMPowerVSResourceReference{
77+
Name: ptr.To("capi-net"),
78+
},
79+
DHCPServer: &DHCPServer{
80+
Name: ptr.To("capi-dhcp"),
81+
},
82+
},
83+
},
84+
wantErr: true,
85+
},
86+
{
87+
name: "Should error if both Network id and DHCP name are set",
88+
powervsCluster: &IBMPowerVSCluster{
89+
Spec: IBMPowerVSClusterSpec{
90+
ServiceInstanceID: "capi-si-id",
91+
Network: IBMPowerVSResourceReference{
92+
ID: ptr.To("capi-net-id"),
93+
},
94+
DHCPServer: &DHCPServer{
95+
Name: ptr.To("capi-dhcp"),
96+
},
97+
},
98+
},
99+
wantErr: true,
100+
},
101+
{
102+
name: "Should error if both Network name and DHCP id are set",
103+
powervsCluster: &IBMPowerVSCluster{
104+
Spec: IBMPowerVSClusterSpec{
105+
ServiceInstanceID: "capi-si-id",
106+
Network: IBMPowerVSResourceReference{
107+
Name: ptr.To("capi-net"),
108+
},
109+
DHCPServer: &DHCPServer{
110+
ID: ptr.To("capi-dhcp-id"),
111+
},
112+
},
113+
},
114+
wantErr: true,
115+
},
116+
{
117+
name: "Should error if both Network id and DHCP id are set",
118+
powervsCluster: &IBMPowerVSCluster{
119+
Spec: IBMPowerVSClusterSpec{
120+
ServiceInstanceID: "capi-si-id",
121+
Network: IBMPowerVSResourceReference{
122+
ID: ptr.To("capi-net-id"),
123+
},
124+
DHCPServer: &DHCPServer{
125+
ID: ptr.To("capi-dhcp-id"),
126+
},
127+
},
128+
},
129+
wantErr: true,
130+
},
71131
}
72132

73133
for _, tc := range tests {

Diff for: ‎cloud/scope/powervs_cluster.go

+11-18
Original file line numberDiff line numberDiff line change
@@ -865,10 +865,10 @@ func (s *PowerVSClusterScope) createServiceInstance() (*resourcecontrollerv2.Res
865865
}
866866

867867
// ReconcileNetwork reconciles network
868-
// 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.
868+
// 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.
869869
// 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.
870-
// 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.
871-
// If both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer is not set, by default DHCP service will be created to setup cluster's network.
870+
// Cannot set both IBMPowerVSCluster.Spec.Network & IBMPowerVSCluster.Spec.DHCPServer since it will cause collision during network creation if both are provided.
871+
// 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.
872872
func (s *PowerVSClusterScope) ReconcileNetwork() (bool, error) {
873873
if s.GetNetworkID() != nil {
874874
// Check the network exists
@@ -949,12 +949,7 @@ func (s *PowerVSClusterScope) checkDHCPServer() (*string, error) {
949949
}
950950

951951
// if user provides DHCP server name then we can use network name to match the existing DHCP server
952-
var networkName string
953-
if s.DHCPServer() != nil && s.DHCPServer().Name != nil {
954-
networkName = dhcpNetworkName(*s.DHCPServer().Name)
955-
} else {
956-
networkName = dhcpNetworkName(s.InfraCluster())
957-
}
952+
networkName := dhcpNetworkName(*s.GetServiceName(infrav1beta2.ResourceTypeDHCPServer))
958953

959954
s.V(3).Info("Checking DHCP server's network list by network name", "name", networkName)
960955
dhcpServers, err := s.IBMPowerVSClient.GetAllDHCPServers()
@@ -2466,11 +2461,14 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT
24662461
return ptr.To(fmt.Sprintf("%s-serviceInstance", s.InfraCluster()))
24672462
}
24682463
return s.ServiceInstance().Name
2469-
case infrav1beta2.ResourceTypeNetwork:
2470-
if s.Network() == nil || s.Network().Name == nil {
2471-
return ptr.To(fmt.Sprintf("DHCPSERVER%s_Private", s.InfraCluster()))
2464+
case infrav1beta2.ResourceTypeDHCPServer:
2465+
if s.DHCPServer() != nil && s.DHCPServer().Name != nil {
2466+
return s.DHCPServer().Name
24722467
}
2473-
return s.Network().Name
2468+
if s.Network() != nil && s.Network().Name != nil {
2469+
return s.Network().Name
2470+
}
2471+
return ptr.To(s.InfraCluster())
24742472
case infrav1beta2.ResourceTypeVPC:
24752473
if s.VPC() == nil || s.VPC().Name == nil {
24762474
return ptr.To(fmt.Sprintf("%s-vpc", s.InfraCluster()))
@@ -2481,11 +2479,6 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT
24812479
return ptr.To(fmt.Sprintf("%s-transitgateway", s.InfraCluster()))
24822480
}
24832481
return s.TransitGateway().Name
2484-
case infrav1beta2.ResourceTypeDHCPServer:
2485-
if s.DHCPServer() == nil || s.DHCPServer().Name == nil {
2486-
return ptr.To(s.InfraCluster())
2487-
}
2488-
return s.DHCPServer().Name
24892482
case infrav1beta2.ResourceTypeCOSInstance:
24902483
if s.COSInstance() == nil || s.COSInstance().Name == "" {
24912484
return ptr.To(fmt.Sprintf("%s-cosinstance", s.InfraCluster()))

Diff for: ‎cloud/scope/powervs_cluster_test.go

+8-16
Original file line numberDiff line numberDiff line change
@@ -2766,22 +2766,6 @@ func TestGetServiceName(t *testing.T) {
27662766
},
27672767
expectedName: ptr.To("ServiceInstanceName"),
27682768
},
2769-
{
2770-
name: "Resource type is network and Network is nil",
2771-
resourceType: infrav1beta2.ResourceTypeNetwork,
2772-
clusterScope: PowerVSClusterScope{
2773-
IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{ObjectMeta: metav1.ObjectMeta{Name: "ClusterName"}},
2774-
},
2775-
expectedName: ptr.To("DHCPSERVERClusterName_Private"),
2776-
},
2777-
{
2778-
name: "Resource type is network and Network is not nil",
2779-
resourceType: infrav1beta2.ResourceTypeNetwork,
2780-
clusterScope: PowerVSClusterScope{
2781-
IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{Spec: infrav1beta2.IBMPowerVSClusterSpec{Network: infrav1beta2.IBMPowerVSResourceReference{Name: ptr.To("NetworkName")}}},
2782-
},
2783-
expectedName: ptr.To("NetworkName"),
2784-
},
27852769
{
27862770
name: "Resource type is vpc and VPC is nil",
27872771
resourceType: infrav1beta2.ResourceTypeVPC,
@@ -2830,6 +2814,14 @@ func TestGetServiceName(t *testing.T) {
28302814
},
28312815
expectedName: ptr.To("DHCPServerName"),
28322816
},
2817+
{
2818+
name: "Resource type is dhcp server and dhcpserver is not nil and network is not nil",
2819+
resourceType: infrav1beta2.ResourceTypeDHCPServer,
2820+
clusterScope: PowerVSClusterScope{
2821+
IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{Spec: infrav1beta2.IBMPowerVSClusterSpec{Network: infrav1beta2.IBMPowerVSResourceReference{Name: ptr.To("NetworkName")}}},
2822+
},
2823+
expectedName: ptr.To("NetworkName"),
2824+
},
28332825
{
28342826
name: "Resource type is cos instance and cos instance is nil",
28352827
resourceType: infrav1beta2.ResourceTypeCOSInstance,

0 commit comments

Comments
 (0)