[VPC] Support NSXLB for VPC #618

gran-vmv · 2024-06-27T02:07:45Z

When NSXLB is enabled, create NSX LBS with VPC and skip AviLB code, and store NSX LBS path to VPCNetworkConfuguration status.

dantingl · 2024-07-01T09:32:53Z

pkg/controllers/networkinfo/networkinfo_controller.go

@@ -139,7 +139,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			LoadBalancerIPAddresses: cidr,
 			PrivateIPv4CIDRs:        nc.PrivateIPv4CIDRs,
 		}
-		updateSuccess(r, &ctx, obj, r.Client, state, nc.Name, path)
+		updateSuccess(r, &ctx, obj, r.Client, state, nc.Name, path, r.Service.GetNSXLBSPath(nc.Org, nc.NsxtProject, *createdVpc.Id, obj.Namespace))


Is it possible GetNSXLBSPath() return error, or notFound?

It's possible but I think no need to do this, since LBS and VPC is created in a batch request.
GetNSXLBSPath returns empty string means no LBS created for this VPC.

dantingl · 2024-07-01T09:37:07Z

pkg/nsx/services/common/wrap.go

+	return &infraObj, nil
+}
+
+func (service *Service) WrapVPC(vpc *model.Vpc) ([]*data.StructValue, error) {


Why not put WrapVPC and WrapLBS in pkg/nsx/services/vpc/wrap.go?

Although these 2 functions are used by VPC only, they can be used by other packages in future.
In future, I think we might refactor these functions to one function which can wrap multiple types to Children resource.

gran-vmv · 2024-07-08T05:59:25Z

pkg/nsx/services/vpc/vpc.go

@@ -602,6 +640,7 @@ func (s *VPCService) CreateOrUpdateVPC(obj *v1alpha1.NetworkInfo) (*model.Vpc, *
 		if realizestate.IsRealizeStateError(err) {
 			log.Error(err, "the created VPC is in error realization state, cleaning the resource", "VPC", *createdVpc.Id)
 			// delete the nsx vpc object and re-created in next loop
+			// TODO(gran) DeleteVPC will check VpcStore but new Vpc is not in store at this moment. Is it correct?


@dantingl @lxiaopei @seanpang-vmware Could you check if we have bug here?

gran-vmv · 2024-07-08T06:57:06Z

Verified with latest NSXT. However, the realization time is too long to DefaultRetry.

status:
{
    "consolidated_status": {
        "consolidated_status": "SUCCESS"
    },
    "consolidated_status_per_enforcement_point": [
        {
            "resource_type": "ConsolidatedStatusPerEnforcementPoint",
            "enforcement_point_id": "default",
            "consolidated_status": {
                "consolidated_status": "SUCCESS"
            }
        }
    ],
    "publish_status": "REALIZED",
    "intent_version": "0",
    "intent_path": "/orgs/default/projects/project-quality/vpcs/418f860d-a2fc-4dcb-bc77-c59eedf045b4/vpc-lbs/418f860d-a2fc-4dcb-bc77-c59eedf045b4"
}

realized-entities:
{
    "results": [
        {
            "extended_attributes": [
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "key": "service_error_message"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "values": [
                        "42eb6532-3a87-11ef-9f38-000c29c18e29"
                    ],
                    "key": "standby_transport_nodes"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "values": [
                        "1720421390701"
                    ],
                    "key": "last_update_timestamp"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "values": [
                        "UP"
                    ],
                    "key": "service_status"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "key": "memory_usage"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "values": [
                        "94c2a942-3a87-11ef-9343-000c29b21f97"
                    ],
                    "key": "active_transport_nodes"
                },
                {
                    "data_type": "STRING",
                    "multivalue": false,
                    "key": "cpu_usage"
                }
            ],
            "entity_type": "LbServiceDto",
            "intent_paths": [
                "/orgs/default/projects/project-quality/vpcs/418f860d-a2fc-4dcb-bc77-c59eedf045b4/vpc-lbs/418f860d-a2fc-4dcb-bc77-c59eedf045b4"
            ],
            "resource_type": "GenericPolicyRealizedResource",
            "id": "418f860d-a2fc-4dcb-bc77-c59eedf045b4",
            "display_name": "418f860d-a2fc-4dcb-bc77-c59eedf045b4",
            "path": "/orgs/default/projects/project-quality/vpcs/418f860d-a2fc-4dcb-bc77-c59eedf045b4/realized-state/enforcement-points/default/lb-services/418f860d-a2fc-4dcb-bc77-c59eedf045b4",
            "relative_path": "418f860d-a2fc-4dcb-bc77-c59eedf045b4",
            "parent_path": "/orgs/default/projects/project-quality/vpcs/418f860d-a2fc-4dcb-bc77-c59eedf045b4/realized-state/enforcement-points/default",
            "unique_id": "bf04c0b7-3837-4c86-8ccb-98e5a69dd3cd",
            "realization_id": "bf04c0b7-3837-4c86-8ccb-98e5a69dd3cd",
            "owner_id": "204df74a-5006-462b-8f34-7ee58a4eeb22",
            "intent_reference": [
                "/orgs/default/projects/project-quality/vpcs/418f860d-a2fc-4dcb-bc77-c59eedf045b4/vpc-lbs/418f860d-a2fc-4dcb-bc77-c59eedf045b4"
            ],
            "realization_specific_identifier": "ee84d8d3-ecab-44c3-9dac-6db636de465c",
            "realization_api": "/api/v1/loadbalancer/services/ee84d8d3-ecab-44c3-9dac-6db636de465c",
            "state": "REALIZED",
            "alarms": [
            ],
            "runtime_status": "UP",
            "publish_status": "UNINITIALIZED",
            "_system_owned": false,
            "_protection": "NOT_PROTECTED",
            "_create_time": 1720421181082,
            "_create_user": "system",
            "_last_modified_time": 1720421400132,
            "_last_modified_user": "system",
            "_revision": 2
        }
    ],
    "result_count": 1
}

gran-vmv · 2024-07-08T07:50:20Z

pkg/nsx/util/utils.go

@@ -561,6 +561,8 @@ func CasttoPointer(obj interface{}) interface{} {
 		return &v
 	case model.Vpc:
 		return &v
+	case model.LBService:
+		return &v


Is there any comment to inform developers to change here for each new resource type? @dantingl @lxiaopei

If type is missing, operator will crash when populate non-empty cache.

gran-vmv · 2024-07-12T02:39:05Z

pkg/nsx/services/vpc/builder.go

+	// LBS id should equal VPC id
+	lbs.Id = common.String(string(nsObj.GetUID()))
+	lbs.DisplayName = &lbsName
+	// TODO(gran) do we need "created_for" and "lb_t1_link_ip" tag?


@dantingl Here is the missing tags. I think lb_t1_link_ip is not available for this case.

lb_t1_link_ip is not needed. created_for should be added.

No need to expose lb_t1_link_ip for now.

Fixed this part.

zhengxiexie · 2024-07-15T06:40:45Z

/e2e

zhengxiexie · 2024-07-15T06:52:59Z

pkg/controllers/networkinfo/networkinfo_controller.go

 		// if lb vpc enabled, read avi subnet path and cidr
 		// nsx bug, if set LoadBalancerVpcEndpoint.Enabled to false, when read this vpc back,
 		// LoadBalancerVpcEndpoint.Enabled will become a nil pointer.
-		if createdVpc.LoadBalancerVpcEndpoint.Enabled != nil && *createdVpc.LoadBalancerVpcEndpoint.Enabled {
+		if r.Service.NSXConfig.NsxConfig.UseAVILB && createdVpc.LoadBalancerVpcEndpoint.Enabled != nil && *createdVpc.LoadBalancerVpcEndpoint.Enabled {


why not keep consistent with https://github.com/vmware-tanzu/nsx-operator/blob/main/pkg/nsx/services/vpc/vpc.go#L587-L587

This is only for AviLB, and L587 is for NSXLB. Although we support only 2 LB types currently, this is different.

However, createdVpc.LoadBalancerVpcEndpoint.Enabled is only for AviLB, thus we do not need to check UseAVILB in this line. Is it correct?

zhengxiexie · 2024-07-15T07:05:55Z

pkg/nsx/services/vpc/vpc.go

+			if realizestate.IsRealizeStateError(err) {
+				log.Error(err, "the created LBS is in error realization state, cleaning the resource", "LBS", *createdLBS.Id)
+				// delete the nsx vpc object and re-created in next loop
+				if err := s.DeleteVPC(*newVpc.Path); err != nil {


So if only lb not realized, must delete the whole vpc, it a must? Can we optimize it by not deleting it, rather only try lbs realization again?

lbs realization timeout is longer than vpc, thus it is enough.
lbs is always created with vpc, thus we should use deleteVPC to remove them.

zhengxiexie · 2024-07-15T07:13:16Z

pkg/nsx/services/vpc/vpc.go

 	go VPCService.InitializeResourceStore(&wg, fatalErrors, common.ResourceTypeVpc, nil, VPCService.VpcStore)
+	wg.Add(1)


could you update line 158 and 161, not here?

No. We might need to add a switch here, thus I use a separate wg.Add(1)

zhengxiexie · 2024-07-15T07:16:02Z

BTW, could you check why e2e test not pass?

zhengxiexie · 2024-07-15T07:17:46Z

Pls also attach what tests you have done in comment. Refer #626

gran-vmv · 2024-07-16T02:47:39Z

Result:

apiVersion: nsx.vmware.com/v1alpha1
kind: VPCNetworkConfiguration
metadata:
  creationTimestamp: "2024-07-09T06:55:58Z"
  generation: 1
  name: gran-ns1-vpc
  resourceVersion: "3738252"
  uid: a7cb1fea-1a8a-402e-bb76-edaee29f0173
spec:
  defaultGatewayPath: /infra/tier-0s/ContainerT0
  defaultIPv4SubnetSize: 64
  defaultSubnetAccessMode: Public
  edgeClusterPath: /infra/sites/default/enforcement-points/default/edge-clusters/21cdfe18-3136-4027-94d4-f66bb975a50e
  externalIPv4Blocks:
  - /infra/ip-blocks/ipblock-192.168.0.0-netmask-16
  nsxtProject: /orgs/default/projects/project-quality
  privateIPv4CIDRs:
  - 172.26.0.0/16
status:
  vpcs:
  - name: vpc-c3a65fe0-42ce-4b8c-ac1a-191efaf44793--gran-ns1
    nsxlbspath: /orgs/default/projects/project-quality/vpcs/41b4e3cd-cd1f-423d-83b9-c8385c150d39/vpc-lbs/41b4e3cd-cd1f-423d-83b9-c8385c150d39

Signed-off-by: gran <gran@vmware.com>

gran-vmv · 2024-07-16T06:13:19Z

/e2e

gran-vmv self-assigned this Jun 27, 2024

vmwclabot added the cla-not-required label Jun 27, 2024

gran-vmv force-pushed the lbvpc-1 branch from 35c7523 to ca8f40b Compare July 1, 2024 09:27

dantingl reviewed Jul 1, 2024

View reviewed changes

gran-vmv requested review from lxiaopei and dantingl July 2, 2024 03:02

gran-vmv force-pushed the lbvpc-1 branch 3 times, most recently from 5040cc2 to 4c564f5 Compare July 3, 2024 05:59

gran-vmv requested review from heypnus and TaoZou1 July 4, 2024 03:00

gran-vmv force-pushed the lbvpc-1 branch 8 times, most recently from 660f1a2 to 9d8f41b Compare July 8, 2024 03:55

gran-vmv commented Jul 8, 2024

View reviewed changes

gran-vmv force-pushed the lbvpc-1 branch from 9d8f41b to 83e9776 Compare July 8, 2024 06:29

gran-vmv force-pushed the lbvpc-1 branch 2 times, most recently from 4712571 to 59bcda2 Compare July 8, 2024 07:39

gran-vmv commented Jul 8, 2024

View reviewed changes

gran-vmv force-pushed the lbvpc-1 branch 5 times, most recently from 1f37f48 to 4a1d178 Compare July 9, 2024 08:15

gran-vmv force-pushed the lbvpc-1 branch 2 times, most recently from b878b9d to 104a4e8 Compare July 10, 2024 03:15

gran-vmv commented Jul 12, 2024

View reviewed changes

gran-vmv force-pushed the lbvpc-1 branch from 104a4e8 to daae322 Compare July 12, 2024 03:28

zhengxiexie reviewed Jul 15, 2024

View reviewed changes

[VPC] Add NSX LBS path to VPCInfo and use LB options in NsxConfig

26efebc

Signed-off-by: gran <gran@vmware.com>

gran-vmv force-pushed the lbvpc-1 branch from daae322 to 196c39e Compare July 16, 2024 02:56

[VPC] Support NSXLB for VPC

17e46dc

Signed-off-by: gran <gran@vmware.com>

gran-vmv force-pushed the lbvpc-1 branch from 196c39e to 17e46dc Compare July 16, 2024 03:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VPC] Support NSXLB for VPC #618

[VPC] Support NSXLB for VPC #618

gran-vmv commented Jun 27, 2024 •

edited

Loading

dantingl Jul 1, 2024

gran-vmv Jul 2, 2024

dantingl Jul 1, 2024

gran-vmv Jul 2, 2024

gran-vmv Jul 8, 2024

gran-vmv commented Jul 8, 2024 •

edited

Loading

gran-vmv Jul 8, 2024

gran-vmv Jul 8, 2024

gran-vmv Jul 12, 2024

gran-vmv Jul 12, 2024

dantingl Jul 12, 2024

gran-vmv Jul 12, 2024

zhengxiexie commented Jul 15, 2024

zhengxiexie Jul 15, 2024

gran-vmv Jul 16, 2024

zhengxiexie Jul 15, 2024

gran-vmv Jul 16, 2024

zhengxiexie Jul 15, 2024

gran-vmv Jul 16, 2024

zhengxiexie commented Jul 15, 2024

zhengxiexie commented Jul 15, 2024

gran-vmv commented Jul 16, 2024

gran-vmv commented Jul 16, 2024

		go VPCService.InitializeResourceStore(&wg, fatalErrors, common.ResourceTypeVpc, nil, VPCService.VpcStore)
		wg.Add(1)

[VPC] Support NSXLB for VPC #618

Are you sure you want to change the base?

[VPC] Support NSXLB for VPC #618

Conversation

gran-vmv commented Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gran-vmv commented Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhengxiexie commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhengxiexie commented Jul 15, 2024

zhengxiexie commented Jul 15, 2024

gran-vmv commented Jul 16, 2024

gran-vmv commented Jul 16, 2024

gran-vmv commented Jun 27, 2024 •

edited

Loading

gran-vmv commented Jul 8, 2024 •

edited

Loading