Skip to content

Commit

Permalink
Change DefaultSubnetAccessMode to DefaultAccessModeForPod
Browse files Browse the repository at this point in the history
As we discussed, we want to set default Pod Subnet type in Namespace settings, and
always keep VMs default to "Private" Subnets.
Also, besides "Public" and "Private" access mode, also add "Project" and "Isolated"
for user to choose.
  • Loading branch information
lxiaopei authored and wenqiq committed May 29, 2024
1 parent 0b74b34 commit 0f2c174
Show file tree
Hide file tree
Showing 20 changed files with 96 additions and 74 deletions.
4 changes: 3 additions & 1 deletion build/yaml/crd/nsx.vmware.com_ippools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,12 @@ spec:
type: object
type: array
type:
description: Type defines the type of this IPPool, Public or Private.
description: Type defines the type of this IPPool, Public, Private
or Project.
enum:
- Public
- Private
- Project
type: string
type: object
status:
Expand Down
1 change: 1 addition & 0 deletions build/yaml/crd/nsx.vmware.com_subnets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ spec:
enum:
- Private
- Public
- Project
type: string
advancedConfig:
description: Subnet advanced configuration.
Expand Down
1 change: 1 addition & 0 deletions build/yaml/crd/nsx.vmware.com_subnetsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ spec:
enum:
- Private
- Public
- Project
type: string
advancedConfig:
description: Subnet advanced configuration.
Expand Down
7 changes: 4 additions & 3 deletions build/yaml/crd/nsx.vmware.com_vpcnetworkconfigurations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,13 @@ spec:
description: Default size of Subnet based upon estimated workload
count. Defaults to 26.
type: integer
defaultSubnetAccessMode:
description: DefaultSubnetAccessMode defines the access mode of the
default SubnetSet for PodVM and VM. Must be Public or Private.
defaultPodSubnetAccessMode:
description: DefaultPodSubnetAccessMode defines the access mode of
the default SubnetSet for PodVM. Must be Public or Private.
enum:
- Public
- Private
- Project
type: string
edgeClusterPath:
description: Edge cluster path on which the networking elements will
Expand Down
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ replace (
github.com/vmware-tanzu/nsx-operator/pkg/client => ./pkg/client
)

replace (
github.com/vmware-tanzu/nsx-operator/pkg/apis => ./pkg/apis
github.com/vmware-tanzu/nsx-operator/pkg/client => ./pkg/client
)

require (
github.com/agiledragon/gomonkey/v2 v2.9.0
github.com/apparentlymart/go-cidr v1.1.0
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/nsx.vmware.com/v1alpha1/subnet_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type SubnetSpec struct {
// +kubebuilder:validation:Minimum:=16
IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public
// +kubebuilder:validation:Enum=Private;Public;Project
AccessMode AccessMode `json:"accessMode,omitempty"`
// Subnet CIDRS.
// +kubebuilder:validation:MinItems=0
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/nsx.vmware.com/v1alpha1/subnetset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type SubnetSetSpec struct {
// +kubebuilder:validation:Minimum:=16
IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public
// +kubebuilder:validation:Enum=Private;Public;Project
AccessMode AccessMode `json:"accessMode,omitempty"`
// Subnet advanced configuration.
AdvancedConfig AdvancedConfig `json:"advancedConfig,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ type VPCNetworkConfigurationSpec struct {
// Defaults to 26.
// +kubebuilder:default=26
DefaultIPv4SubnetSize int `json:"defaultIPv4SubnetSize,omitempty"`
// DefaultSubnetAccessMode defines the access mode of the default SubnetSet for PodVM and VM.
// DefaultPodSubnetAccessMode defines the access mode of the default SubnetSet for PodVM.
// Must be Public or Private.
// +kubebuilder:validation:Enum=Public;Private
DefaultSubnetAccessMode string `json:"defaultSubnetAccessMode,omitempty"`
// +kubebuilder:validation:Enum=Public;Private;Project
DefaultPodSubnetAccessMode string `json:"defaultPodSubnetAccessMode,omitempty"`
// ShortID specifies Identifier to use when displaying VPC context in logs.
// Less than or equal to 8 characters.
// Less than equal to 8 characters.
// +kubebuilder:validation:MaxLength=8
// +optional
ShortID string `json:"shortID,omitempty"`
Expand All @@ -64,9 +64,9 @@ type VPCInfo struct {

// +genclient
// +genclient:nonNamespaced
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
//+kubebuilder:storageversion
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:storageversion

// VPCNetworkConfiguration is the Schema for the vpcnetworkconfigurations API.
// +kubebuilder:resource:scope="Cluster"
Expand All @@ -81,7 +81,7 @@ type VPCNetworkConfiguration struct {
Status VPCNetworkConfigurationStatus `json:"status,omitempty"`
}

//+kubebuilder:object:root=true
// +kubebuilder:object:root=true

// VPCNetworkConfigurationList contains a list of VPCNetworkConfiguration.
type VPCNetworkConfigurationList struct {
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/nsx.vmware.com/v1alpha2/ippool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type IPPoolList struct {

// IPPoolSpec defines the desired state of IPPool.
type IPPoolSpec struct {
// Type defines the type of this IPPool, Public or Private.
// +kubebuilder:validation:Enum=Public;Private
// Type defines the type of this IPPool, Public, Private or Project.
// +kubebuilder:validation:Enum=Public;Private;Project
// +optional
Type string `json:"type,omitempty"`
// Subnets defines set of subnets need to be allocated.
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/v1alpha1/subnet_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type SubnetSpec struct {
// +kubebuilder:validation:Minimum:=16
IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public
// +kubebuilder:validation:Enum=Private;Public;Project
AccessMode AccessMode `json:"accessMode,omitempty"`
// Subnet CIDRS.
// +kubebuilder:validation:MinItems=0
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/v1alpha1/subnetset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type SubnetSetSpec struct {
// +kubebuilder:validation:Minimum:=16
IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public
// +kubebuilder:validation:Enum=Private;Public;Project
AccessMode AccessMode `json:"accessMode,omitempty"`
// Subnet advanced configuration.
AdvancedConfig AdvancedConfig `json:"advancedConfig,omitempty"`
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/v1alpha1/vpcnetworkconfiguration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ type VPCNetworkConfigurationSpec struct {
// Defaults to 26.
// +kubebuilder:default=26
DefaultIPv4SubnetSize int `json:"defaultIPv4SubnetSize,omitempty"`
// DefaultSubnetAccessMode defines the access mode of the default SubnetSet for PodVM and VM.
// DefaultPodSubnetAccessMode defines the access mode of the default SubnetSet for PodVM.
// Must be Public or Private.
// +kubebuilder:validation:Enum=Public;Private
DefaultSubnetAccessMode string `json:"defaultSubnetAccessMode,omitempty"`
// +kubebuilder:validation:Enum=Public;Private;Project
DefaultPodSubnetAccessMode string `json:"defaultPodSubnetAccessMode,omitempty"`
// ShortID specifies Identifier to use when displaying VPC context in logs.
// Less than or equal to 8 characters.
// +kubebuilder:validation:MaxLength=8
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/v1alpha2/ippool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type IPPoolList struct {

// IPPoolSpec defines the desired state of IPPool.
type IPPoolSpec struct {
// Type defines the type of this IPPool, Public or Private.
// +kubebuilder:validation:Enum=Public;Private
// Type defines the type of this IPPool, Public, Private or Project.
// +kubebuilder:validation:Enum=Public;Private;Project
// +optional
Type string `json:"type,omitempty"`
// Subnets defines set of subnets need to be allocated.
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/ippool/ippool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (r *IPPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
updateFail(r, &ctx, obj, &err)
return resultRequeue, err
}
obj.Spec.Type = vpcNetworkConfig.DefaultSubnetAccessMode
obj.Spec.Type = "Private"
}

if obj.ObjectMeta.DeletionTimestamp.IsZero() {
Expand Down
34 changes: 20 additions & 14 deletions pkg/controllers/namespace/namespace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,6 @@ func (r *NamespaceReconciler) createNetworkInfoCR(ctx *context.Context, obj clie
log.Info("networkInfo already exists", "networkInfo", networkInfos.Items[0].Name, "Namespace", ns)
return &networkInfos.Items[0], nil
}
nc, ncExist := r.VPCService.GetVPCNetworkConfig(ncName)
if !ncExist {
message := fmt.Sprintf("missing network config %s for namespace %s", ncName, ns)
r.namespaceError(ctx, obj, message, nil)
return nil, errors.New(message)
}
if !r.VPCService.ValidateNetworkConfig(nc) {
// if network config is not valid, no need to retry, skip processing
message := fmt.Sprintf("invalid network config %s for namespace %s, missing private cidr", ncName, ns)
r.namespaceError(ctx, obj, message, nil)
return nil, errors.New(message)
}

// create networkInfo cr with existing vpc network config
log.V(2).Info("building networkInfo", "ns", ns)
Expand Down Expand Up @@ -96,7 +84,7 @@ func (r *NamespaceReconciler) createNetworkInfoCR(ctx *context.Context, obj clie
return networkInfoCR, nil
}

func (r *NamespaceReconciler) createDefaultSubnetSet(ns string) error {
func (r *NamespaceReconciler) createDefaultSubnetSet(ns string, defaultPodAccessMode string) error {
defaultSubnetSets := map[string]string{
types.DefaultVMSubnetSet: types.LabelDefaultVMSubnetSet,
types.DefaultPodSubnetSet: types.LabelDefaultPodSubnetSet,
Expand Down Expand Up @@ -132,6 +120,12 @@ func (r *NamespaceReconciler) createDefaultSubnetSet(ns string) error {
},
},
}
if name == types.DefaultVMSubnetSet {
// use "Private" type for VM
obj.Spec.AccessMode = v1alpha1.AccessMode("Private")
} else if name == types.DefaultPodSubnetSet {
obj.Spec.AccessMode = v1alpha1.AccessMode(defaultPodAccessMode)
}
if err := r.Client.Create(context.Background(), obj); err != nil {
return err
}
Expand Down Expand Up @@ -228,11 +222,23 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return common.ResultRequeueAfter10sec, nil
}
}
nc, ncExist := r.VPCService.GetVPCNetworkConfig(ncName)
if !ncExist {
message := fmt.Sprintf("missing network config %s for namespace %s", ncName, ns)
r.namespaceError(&ctx, obj, message, nil)
return common.ResultRequeueAfter10sec, nil
}
if !r.VPCService.ValidateNetworkConfig(nc) {
// if network config is not valid, no need to retry, skip processing
message := fmt.Sprintf("invalid network config %s for namespace %s, missing private cidr", ncName, ns)
r.namespaceError(&ctx, obj, message, nil)
return common.ResultRequeueAfter10sec, nil
}

if _, err := r.createNetworkInfoCR(&ctx, obj, ns, ncName); err != nil {
return common.ResultRequeueAfter10sec, nil
}
if err := r.createDefaultSubnetSet(ns); err != nil {
if err := r.createDefaultSubnetSet(ns, nc.DefaultPodSubnetAccessMode); err != nil {
return common.ResultRequeueAfter10sec, nil
}
return common.ResultNormal, nil
Expand Down
30 changes: 15 additions & 15 deletions pkg/controllers/networkinfo/vpcnetworkconfig_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,22 @@ func TestBuildNetworkConfigInfo(t *testing.T) {
assert.NotNil(t, e)

spec1 := v1alpha1.VPCNetworkConfigurationSpec{
DefaultGatewayPath: "test-gw-path-1",
EdgeClusterPath: "test-edge-path-1",
ExternalIPv4Blocks: []string{"external-ipb-1", "external-ipb-2"},
PrivateIPv4CIDRs: []string{"private-ipb-1", "private-ipb-2"},
DefaultIPv4SubnetSize: 64,
DefaultSubnetAccessMode: "Public",
NSXTProject: "/orgs/default/projects/nsx_operator_e2e_test",
DefaultGatewayPath: "test-gw-path-1",
EdgeClusterPath: "test-edge-path-1",
ExternalIPv4Blocks: []string{"external-ipb-1", "external-ipb-2"},
PrivateIPv4CIDRs: []string{"private-ipb-1", "private-ipb-2"},
DefaultIPv4SubnetSize: 64,
DefaultPodSubnetAccessMode: "Public",
NSXTProject: "/orgs/default/projects/nsx_operator_e2e_test",
}
spec2 := v1alpha1.VPCNetworkConfigurationSpec{
DefaultGatewayPath: "test-gw-path-2",
EdgeClusterPath: "test-edge-path-2",
ExternalIPv4Blocks: []string{"external-ipb-1", "external-ipb-2"},
PrivateIPv4CIDRs: []string{"private-ipb-1", "private-ipb-2"},
DefaultIPv4SubnetSize: 32,
DefaultSubnetAccessMode: "Private",
NSXTProject: "/orgs/anotherOrg/projects/anotherProject",
DefaultGatewayPath: "test-gw-path-2",
EdgeClusterPath: "test-edge-path-2",
ExternalIPv4Blocks: []string{"external-ipb-1", "external-ipb-2"},
PrivateIPv4CIDRs: []string{"private-ipb-1", "private-ipb-2"},
DefaultIPv4SubnetSize: 32,
DefaultPodSubnetAccessMode: "Private",
NSXTProject: "/orgs/anotherOrg/projects/anotherProject",
}
testCRD1 := v1alpha1.VPCNetworkConfiguration{
Spec: spec1,
Expand Down Expand Up @@ -133,7 +133,7 @@ func TestBuildNetworkConfigInfo(t *testing.T) {
assert.Equal(t, tt.org, nc.Org)
assert.Equal(t, tt.project, nc.NsxtProject)
assert.Equal(t, tt.subnetSize, nc.DefaultIPv4SubnetSize)
assert.Equal(t, tt.accessMode, nc.DefaultSubnetAccessMode)
assert.Equal(t, tt.accessMode, nc.DefaultPodSubnetAccessMode)
assert.Equal(t, tt.isDefault, nc.IsDefault)
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/subnet/subnet_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ResultRequeue, err
}
if obj.Spec.AccessMode == "" {
obj.Spec.AccessMode = v1alpha1.AccessMode(vpcNetworkConfig.DefaultSubnetAccessMode)
obj.Spec.AccessMode = v1alpha1.AccessMode("Private")
}
if obj.Spec.IPv4SubnetSize == 0 {
obj.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultIPv4SubnetSize
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/subnetset/subnetset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ResultRequeue, err
}
if obj.Spec.AccessMode == "" {
obj.Spec.AccessMode = v1alpha1.AccessMode(vpcNetworkConfig.DefaultSubnetAccessMode)
obj.Spec.AccessMode = v1alpha1.AccessMode("Private")
}
if obj.Spec.IPv4SubnetSize == 0 {
obj.Spec.IPv4SubnetSize = vpcNetworkConfig.DefaultIPv4SubnetSize
Expand Down
22 changes: 11 additions & 11 deletions pkg/nsx/services/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,15 @@ type VPCResourceInfo struct {
}

type VPCNetworkConfigInfo struct {
IsDefault bool
Org string
Name string
DefaultGatewayPath string
EdgeClusterPath string
NsxtProject string
ExternalIPv4Blocks []string
PrivateIPv4CIDRs []string
DefaultIPv4SubnetSize int
DefaultSubnetAccessMode string
ShortID string
IsDefault bool
Org string
Name string
DefaultGatewayPath string
EdgeClusterPath string
NsxtProject string
ExternalIPv4Blocks []string
PrivateIPv4CIDRs []string
DefaultIPv4SubnetSize int
DefaultPodSubnetAccessMode string
ShortID string
}
22 changes: 14 additions & 8 deletions test/e2e/nsx_subnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
SubnetDeletionTimeout = 300 * time.Second
)

func verifySubnetSetCR(subnetSet string) bool {
func verifySubnetSetCR(subnetSet string, subnetType string) bool {
vpcNetworkConfig, err := testData.crdClientset.NsxV1alpha1().VPCNetworkConfigurations().Get(context.TODO(), VPCNetworkConfigCRName, v1.GetOptions{})
if err != nil {
log.Printf("Failed to get VPCNetworkConfiguration %s: %v", VPCNetworkConfigCRName, err)
Expand All @@ -41,8 +41,14 @@ func verifySubnetSetCR(subnetSet string) bool {
log.Printf("Failed to get %s/%s: %s", E2ENamespace, subnetSet, err)
return false
}
if string(subnetSetCR.Spec.AccessMode) != vpcNetworkConfig.Spec.DefaultSubnetAccessMode {
log.Printf("AccessMode is %s, while it's expected to be %s", subnetSetCR.Spec.AccessMode, vpcNetworkConfig.Spec.DefaultSubnetAccessMode)
var desiredSubnetType string
if subnetType == "" {
desiredSubnetType = vpcNetworkConfig.Spec.DefaultPodSubnetAccessMode
} else {
desiredSubnetType = subnetType
}
if string(subnetSetCR.Spec.AccessMode) != desiredSubnetType {
log.Printf("AccessMode is %s, while it's expected to be %s", subnetSetCR.Spec.AccessMode, desiredSubnetType)
return false
}
if subnetSetCR.Spec.IPv4SubnetSize != vpcNetworkConfig.Spec.DefaultIPv4SubnetSize {
Expand Down Expand Up @@ -78,8 +84,8 @@ func defaultSubnetSet(t *testing.T) {
assertNil(t, err)

// 2. Check `Ipv4SubnetSize` and `AccessMode` should be same with related fields in VPCNetworkConfig.
assertTrue(t, verifySubnetSetCR(common.DefaultVMSubnetSet))
assertTrue(t, verifySubnetSetCR(common.DefaultPodSubnetSet))
assertTrue(t, verifySubnetSetCR(common.DefaultVMSubnetSet, "Private"))
assertTrue(t, verifySubnetSetCR(common.DefaultPodSubnetSet, ""))

portPath, _ := filepath.Abs("./manifest/testSubnet/subnetport_1.yaml")
err = applyYAML(portPath, E2ENamespace)
Expand Down Expand Up @@ -181,7 +187,7 @@ func UserSubnetSet(t *testing.T) {
assertNil(t, err)

// 2. Check `Ipv4SubnetSize` and `AccessMode` should be same with related fields in VPCNetworkConfig.
assertTrue(t, verifySubnetSetCR(subnetSetName))
assertTrue(t, verifySubnetSetCR(UserSubnetSet, "Private"))

Check failure on line 190 in test/e2e/nsx_subnet_test.go

View workflow job for this annotation

GitHub Actions / build

cannot use UserSubnetSet (value of type func(t *"testing".T)) as string value in argument to verifySubnetSetCR (typecheck)

portPath, _ := filepath.Abs(portYAML)
err = applyYAML(portPath, E2ENamespace)
Expand Down Expand Up @@ -221,8 +227,8 @@ func sharedSubnetSet(t *testing.T) {
assertNil(t, err)

// 2. Check `Ipv4SubnetSize` and `AccessMode` should be same with related fields in VPCNetworkConfig.
assertTrue(t, verifySubnetSetCR(common.DefaultVMSubnetSet))
assertTrue(t, verifySubnetSetCR(common.DefaultPodSubnetSet))
assertTrue(t, verifySubnetSetCR(common.DefaultVMSubnetSet, "Private"))
assertTrue(t, verifySubnetSetCR(common.DefaultPodSubnetSet, ""))

portPath, _ := filepath.Abs("./manifest/testSubnet/subnetport_3.yaml")
err = applyYAML(portPath, E2ENamespaceShared)
Expand Down

0 comments on commit 0f2c174

Please sign in to comment.