Fix SubnetPort allocate address for DHCP

Signed-off-by: Yanjun Zhou <[email protected]>

pkg/nsx/services/subnet/builder.go

@@ -2,14 +2,13 @@ package subnet
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
-	util2 "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
+	nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 	"github.com/vmware-tanzu/nsx-operator/pkg/util"
 )
 
@@ -58,6 +57,7 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
 	tags = append(service.buildBasicTags(obj), tags...)
 	var nsxSubnet *model.VpcSubnet
 	var staticIpAllocation bool
+	var err error
 	switch o := obj.(type) {
 	case *v1alpha1.Subnet:
 		staticIpAllocation = o.Spec.SubnetDHCPConfig.Mode == "" || o.Spec.SubnetDHCPConfig.Mode == v1alpha1.DHCPConfigMode(v1alpha1.DHCPConfigModeDeactivated)
@@ -74,7 +74,9 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
 		if useLegacyAPI {
 			nsxSubnet.DhcpConfig = service.buildDHCPConfig(dhcpMode != v1alpha1.DHCPConfigModeDeactivated)
 		} else {
-			nsxSubnet.SubnetDhcpConfig = service.buildSubnetDHCPConfig(dhcpMode)
+			if nsxSubnet.SubnetDhcpConfig, err = service.buildSubnetDHCPConfig(dhcpMode); err != nil {
+				return nil, err
+			}
 		}
 		nsxSubnet.IpAddresses = o.Spec.IPAddresses
 	case *v1alpha1.SubnetSet:
@@ -95,15 +97,17 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
 		if useLegacyAPI {
 			nsxSubnet.DhcpConfig = service.buildDHCPConfig(dhcpMode != v1alpha1.DHCPConfigModeDeactivated)
 		} else {
-			nsxSubnet.SubnetDhcpConfig = service.buildSubnetDHCPConfig(dhcpMode)
+			if nsxSubnet.SubnetDhcpConfig, err = service.buildSubnetDHCPConfig(dhcpMode); err != nil {
+				return nil, err
+			}
 		}
 	default:
 		return nil, SubnetTypeError
 	}
 	// tags cannot exceed maximum size 26
 	if len(tags) > common.MaxTagsCount {
 		errorMsg := fmt.Sprintf("tags cannot exceed maximum size 26, tags length: %d", len(tags))
-		return nil, util2.ExceedTagsError{Desc: errorMsg}
+		return nil, nsxutil.ExceedTagsError{Desc: errorMsg}
 	}
 	nsxSubnet.Tags = tags
 	nsxSubnet.AdvancedConfig = &model.SubnetAdvancedConfig{
@@ -123,15 +127,15 @@ func (service *SubnetService) buildDHCPConfig(enableDHCP bool) *model.VpcSubnetD
 	return dhcpConfig
 }
 
-func (service *SubnetService) buildSubnetDHCPConfig(mode string) *model.SubnetDhcpConfig {
-	// Transfer DHCPDeactivated to DHCP_DEACTIVATED
-	nsxMode := strings.ToUpper(mode)
-	nsxMode = nsxMode[:4] + "_" + nsxMode[4:]
-
+func (service *SubnetService) buildSubnetDHCPConfig(mode string) (*model.SubnetDhcpConfig, error) {
+	nsxMode, err := nsxutil.ParseDHCPMode(mode)
+	if err != nil {
+		return nil, err
+	}
 	subnetDhcpConfig := &model.SubnetDhcpConfig{
 		Mode: &nsxMode,
 	}
-	return subnetDhcpConfig
+	return subnetDhcpConfig, nil
 }
 
 func (service *SubnetService) buildBasicTags(obj client.Object) []model.Tag {

pkg/nsx/services/subnet/subnet.go

@@ -411,6 +411,10 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc
 	if dhcpMode == "" {
 		dhcpMode = v1alpha1.DHCPConfigModeDeactivated
 	}
+	nsxDhcpMode, err := service.buildSubnetDHCPConfig(dhcpMode)
+	if err != nil {
+		return err
+	}
 	staticIpAllocation := dhcpMode == v1alpha1.DHCPConfigModeDeactivated
 	for i, vpcSubnet := range vpcSubnets {
 		subnetSet := &v1alpha1.SubnetSet{}
@@ -449,7 +453,7 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc
 		} else {
 			updatedSubnet = &model.VpcSubnet{
 				Tags:             newTags,
-				SubnetDhcpConfig: service.buildSubnetDHCPConfig(dhcpMode),
+				SubnetDhcpConfig: nsxDhcpMode,
 			}
 		}
 		changed := common.CompareResource(SubnetToComparable(vpcSubnets[i]), SubnetToComparable(updatedSubnet))
@@ -461,7 +465,7 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc
 		vpcSubnets[i].Tags = newTags
 		// Update the SubnetSet DHCP Config
 		if vpcSubnets[i].SubnetDhcpConfig != nil {
-			vpcSubnets[i].SubnetDhcpConfig = service.buildSubnetDHCPConfig(dhcpMode)
+			vpcSubnets[i].SubnetDhcpConfig = nsxDhcpMode
 			vpcSubnets[i].AdvancedConfig.StaticIpAllocation.Enabled = &staticIpAllocation
 		}
 

pkg/nsx/services/subnetport/builder.go

@@ -17,6 +17,7 @@ import (
 	"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	controllercommon "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
+	nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 	"github.com/vmware-tanzu/nsx-operator/pkg/util"
 )
 
@@ -46,7 +47,11 @@ func (service *SubnetPortService) buildSubnetPort(obj interface{}, nsxSubnet *mo
 			allocateAddresses = "BOTH"
 		}
 	} else {
-		if nsxSubnet.SubnetDhcpConfig.Mode != nil && *nsxSubnet.SubnetDhcpConfig.Mode != v1alpha1.DHCPConfigModeDeactivated {
+		nsxDhcpModeDeactivated, err := nsxutil.ParseDHCPMode(v1alpha1.DHCPConfigModeDeactivated)
+		if err != nil {
+			return nil, err
+		}
+		if nsxSubnet.SubnetDhcpConfig.Mode != nil && *nsxSubnet.SubnetDhcpConfig.Mode != nsxDhcpModeDeactivated {
 			allocateAddresses = "DHCP"
 		} else {
 			allocateAddresses = "BOTH"

pkg/nsx/util/utils.go

@@ -708,3 +708,13 @@ func IsInvalidLicense(err error) bool {
 	}
 	return invalidLicense
 }
+
+func ParseDHCPMode(mode string) (string, error) {
+	// Transfer DHCPDeactivated to DHCP_DEACTIVATED
+	nsxMode := strings.ToUpper(mode)
+	if len(nsxMode) <= 4 {
+		return "", fmt.Errorf("failed to parse DHCP mode: %s", mode)
+	}
+	nsxMode = nsxMode[:4] + "_" + nsxMode[4:]
+	return nsxMode, nil
+}

pkg/nsx/util/utils_test.go

@@ -655,3 +655,21 @@ func TestTransNSXApiError(t *testing.T) {
 	assert.Equal(t, ok, true)
 	assert.Equal(t, gotErr.Type(), apierrors.ErrorType_NOT_FOUND)
 }
+
+func TestParseDHCPMode(t *testing.T) {
+	nsxMode, err := ParseDHCPMode("DHCPDeactivated")
+	assert.Nil(t, err)
+	assert.Equal(t, "DHCP_DEACTIVATED", nsxMode)
+
+	nsxMode, err = ParseDHCPMode("DHCPServer")
+	assert.Nil(t, err)
+	assert.Equal(t, "DHCP_SERVER", nsxMode)
+
+	nsxMode, err = ParseDHCPMode("DHCPRelay")
+	assert.Nil(t, err)
+	assert.Equal(t, "DHCP_RELAY", nsxMode)
+
+	nsxMode, err = ParseDHCPMode("None")
+	assert.Equal(t, fmt.Errorf("failed to parse DHCP mode: None"), err)
+	assert.Equal(t, "", nsxMode)
+}