Fix SubnetPort allocate address for DHCP (vmware-tanzu#934)

Signed-off-by: Yanjun Zhou <[email protected]>
TaoZou1 · Nov 29, 2024 · 3663545 · 3663545
1 parent a3a6318
commit 3663545
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 8 deletions.
diff --git a/pkg/nsx/services/subnet/builder.go b/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"
 )
 
@@ -103,7 +102,7 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag, u
 	// 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{
@@ -124,10 +123,7 @@ func (service *SubnetService) buildDHCPConfig(enableDHCP bool) *model.VpcSubnetD
 }
 
 func (service *SubnetService) buildSubnetDHCPConfig(mode string) *model.SubnetDhcpConfig {
-	// Transfer DHCPDeactivated to DHCP_DEACTIVATED
-	nsxMode := strings.ToUpper(mode)
-	nsxMode = nsxMode[:4] + "_" + nsxMode[4:]
-
+	nsxMode := nsxutil.ParseDHCPMode(mode)
 	subnetDhcpConfig := &model.SubnetDhcpConfig{
 		Mode: &nsxMode,
 	}

diff --git a/pkg/nsx/services/subnet/builder_test.go b/pkg/nsx/services/subnet/builder_test.go
@@ -107,4 +107,10 @@ func TestBuildSubnetForSubnetSet(t *testing.T) {
 	assert.Equal(t, false, *subnet.DhcpConfig.EnableDhcp)
 	assert.Nil(t, subnet.SubnetDhcpConfig)
 	assert.Equal(t, true, *subnet.AdvancedConfig.StaticIpAllocation.Enabled)
+
+	subnet, err = service.buildSubnet(subnetSet, tags, false)
+	assert.Nil(t, err)
+	assert.Equal(t, "DHCP_DEACTIVATED", *subnet.SubnetDhcpConfig.Mode)
+	assert.Nil(t, subnet.DhcpConfig)
+	assert.Equal(t, true, *subnet.AdvancedConfig.StaticIpAllocation.Enabled)
 }
diff --git a/pkg/nsx/services/subnetport/builder.go b/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,7 @@ func (service *SubnetPortService) buildSubnetPort(obj interface{}, nsxSubnet *mo
 			allocateAddresses = "BOTH"
 		}
 	} else {
-		if nsxSubnet.SubnetDhcpConfig.Mode != nil && *nsxSubnet.SubnetDhcpConfig.Mode != v1alpha1.DHCPConfigModeDeactivated {
+		if nsxSubnet.SubnetDhcpConfig.Mode != nil && *nsxSubnet.SubnetDhcpConfig.Mode != nsxutil.ParseDHCPMode(v1alpha1.DHCPConfigModeDeactivated) {
 			allocateAddresses = "DHCP"
 		} else {
 			allocateAddresses = "BOTH"

diff --git a/pkg/nsx/util/utils.go b/pkg/nsx/util/utils.go
@@ -708,3 +708,14 @@ func IsInvalidLicense(err error) bool {
 	}
 	return invalidLicense
 }
+
+func ParseDHCPMode(mode string) string {
+	// Transfer DHCPDeactivated to DHCP_DEACTIVATED
+	nsxMode := strings.ToUpper(mode)
+	if len(nsxMode) <= 4 {
+		log.Error(nil, "Failed to parse DHCP mode", "mode", mode)
+		return ""
+	}
+	nsxMode = nsxMode[:4] + "_" + nsxMode[4:]
+	return nsxMode
+}
diff --git a/pkg/nsx/util/utils_test.go b/pkg/nsx/util/utils_test.go
@@ -655,3 +655,17 @@ 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 := ParseDHCPMode("DHCPDeactivated")
+	assert.Equal(t, "DHCP_DEACTIVATED", nsxMode)
+
+	nsxMode = ParseDHCPMode("DHCPServer")
+	assert.Equal(t, "DHCP_SERVER", nsxMode)
+
+	nsxMode = ParseDHCPMode("DHCPRelay")
+	assert.Equal(t, "DHCP_RELAY", nsxMode)
+
+	nsxMode = ParseDHCPMode("None")
+	assert.Equal(t, "", nsxMode)
+}