Fixup code review comments

.golangci.yml

@@ -122,8 +122,11 @@ linters-settings:
         alias: vmwarev1
       # VMware Operator
       - pkg: "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
-        alias: vmoprv1 
-      # CABPK
+        alias: vmoprv1
+      # NSX Operator
+      - pkg: "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+        alias: nsxvpcv1
+    # CABPK
       - pkg: sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1
         alias: bootstrapv1
       # KCP

pkg/manager/manager.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/pkg/errors"
 	netopv1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1"
-	vpcapisv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+	nsxvpcv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
 	ncpv1 "github.com/vmware-tanzu/vm-operator/external/ncp/api/v1alpha1"
 	"gopkg.in/fsnotify.v1"
@@ -65,7 +65,7 @@ func New(ctx context.Context, opts Options) (Manager, error) {
 	_ = vmoprv1.AddToScheme(opts.Scheme)
 	_ = ncpv1.AddToScheme(opts.Scheme)
 	_ = netopv1.AddToScheme(opts.Scheme)
-	_ = vpcapisv1.AddToScheme(opts.Scheme)
+	_ = nsxvpcv1.AddToScheme(opts.Scheme)
 	_ = topologyv1.AddToScheme(opts.Scheme)
 	_ = ipamv1.AddToScheme(opts.Scheme)
 

pkg/services/network/constants.go

@@ -19,7 +19,7 @@ package network
 
 import (
 	netopv1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1"
-	vpcapisv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+	nsxvpcv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	ncpv1 "github.com/vmware-tanzu/vm-operator/external/ncp/api/v1alpha1"
 )
 
@@ -39,6 +39,11 @@ const (
 	//
 	// Deprecated: legacyDefaultNetworkLabel will be removed in a future release.
 	legacyDefaultNetworkLabel = "capw.vmware.com/is-default-network"
+
+	// AnnotationEnableEndpointHealthCheckKey is the key of the annotation that is used to enable health check on the
+	// Service endpoint port. vm-operator propagates annotations in VMService to Service and LB providers like NSX-T
+	// will enable health check on the endpoint target port when this annotation is present on the Service.
+	AnnotationEnableEndpointHealthCheckKey = "lb.iaas.vmware.com/enable-endpoint-health-check"
 )
 
 var (
@@ -49,5 +54,5 @@ var (
 	NetworkGVKNSXT = ncpv1.SchemeGroupVersion.WithKind("VirtualNetwork")
 
 	// NetworkGVKNSXTVPC is the GVK used for networks in NSX-T VPC mode.
-	NetworkGVKNSXTVPC = vpcapisv1.SchemeGroupVersion.WithKind("SubnetSet")
+	NetworkGVKNSXTVPC = nsxvpcv1.SchemeGroupVersion.WithKind("SubnetSet")
 )

pkg/services/network/network_test.go

@@ -24,7 +24,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	netopv1 "github.com/vmware-tanzu/net-operator-api/api/v1alpha1"
-	vpcapisv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+	nsxvpcv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
 	ncpv1 "github.com/vmware-tanzu/vm-operator/external/ncp/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
@@ -270,7 +270,7 @@ var _ = Describe("Network provider", func() {
 				Expect(vm.Spec.Network.Interfaces).To(HaveLen(1))
 				Expect(vm.Spec.Network.Interfaces[0].Network.Name).To(Equal(vSphereCluster.Name))
 				Expect(vm.Spec.Network.Interfaces[0].Network.TypeMeta.Kind).To(Equal("SubnetSet"))
-				Expect(vm.Spec.Network.Interfaces[0].Network.TypeMeta.APIVersion).To(Equal(vpcapisv1.SchemeGroupVersion.String()))
+				Expect(vm.Spec.Network.Interfaces[0].Network.TypeMeta.APIVersion).To(Equal(nsxvpcv1.SchemeGroupVersion.String()))
 			})
 		})
 	})
@@ -336,7 +336,7 @@ var _ = Describe("Network provider", func() {
 			Expect(ncpv1.AddToScheme(scheme)).To(Succeed())
 			Expect(corev1.AddToScheme(scheme)).To(Succeed())
 			Expect(vmwarev1.AddToScheme(scheme)).To(Succeed())
-			Expect(vpcapisv1.AddToScheme(scheme)).To(Succeed())
+			Expect(nsxvpcv1.AddToScheme(scheme)).To(Succeed())
 		})
 
 		Context("with dummy network provider", func() {
@@ -599,14 +599,14 @@ var _ = Describe("Network provider", func() {
 
 			It("should not update subnetset", func() {
 				// Fetch the SubnetSet before the operation
-				initialSubnetSet := &vpcapisv1.SubnetSet{}
+				initialSubnetSet := &nsxvpcv1.SubnetSet{}
 				err = client.Get(ctx, apitypes.NamespacedName{
 					Name:      dummyCluster,
 					Namespace: dummyNs,
 				}, initialSubnetSet)
 				Expect(err).NotTo(HaveOccurred())
-				status := vpcapisv1.SubnetSetStatus{
-					Conditions: []vpcapisv1.Condition{
+				status := nsxvpcv1.SubnetSetStatus{
+					Conditions: []nsxvpcv1.Condition{
 						{
 							Type:   "Ready",
 							Status: "True",
@@ -620,7 +620,7 @@ var _ = Describe("Network provider", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(subnetset).To(Equal(clusterCtx.VSphereCluster.Name))
 
-				createdSubnetSet := &vpcapisv1.SubnetSet{}
+				createdSubnetSet := &nsxvpcv1.SubnetSet{}
 				err = client.Get(ctx, apitypes.NamespacedName{
 					Name:      dummyCluster,
 					Namespace: dummyNs,
@@ -633,7 +633,7 @@ var _ = Describe("Network provider", func() {
 			It("should successfully retrieve VM service annotations, including the annotation to enable LB healthcheck", func() {
 				annotations, err := np.GetVMServiceAnnotations(ctx, clusterCtx)
 				Expect(err).ToNot(HaveOccurred())
-				Expect(annotations).To(HaveKey("lb.iaas.vmware.com/enable-endpoint-health-check"))
+				Expect(annotations).To(HaveKey(AnnotationEnableEndpointHealthCheckKey))
 			})
 
 		})
@@ -658,7 +658,7 @@ var _ = Describe("Network provider", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(subnetset).To(Equal(clusterCtx.VSphereCluster.Name))
 
-				createdSubnetSet := &vpcapisv1.SubnetSet{}
+				createdSubnetSet := &nsxvpcv1.SubnetSet{}
 				err = client.Get(ctx, apitypes.NamespacedName{
 					Name:      dummyCluster,
 					Namespace: dummyNs,
@@ -700,14 +700,14 @@ var _ = Describe("Network provider", func() {
 
 			BeforeEach(func() {
 				scheme = runtime.NewScheme()
-				Expect(vpcapisv1.AddToScheme(scheme)).To(Succeed())
+				Expect(nsxvpcv1.AddToScheme(scheme)).To(Succeed())
 				nsxvpcNp, _ = NSXTVpcNetworkProvider(client).(*nsxtVPCNetworkProvider)
 				np = nsxvpcNp
 			})
 
 			It("should return error when subnetset ready status is false", func() {
-				status := vpcapisv1.SubnetSetStatus{
-					Conditions: []vpcapisv1.Condition{
+				status := nsxvpcv1.SubnetSetStatus{
+					Conditions: []nsxvpcv1.Condition{
 						{
 							Type:    "Ready",
 							Status:  "False",
@@ -716,7 +716,7 @@ var _ = Describe("Network provider", func() {
 						},
 					},
 				}
-				subnetsetObj = &vpcapisv1.SubnetSet{
+				subnetsetObj = &nsxvpcv1.SubnetSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: cluster.Namespace,
 						Name:      cluster.Name,
@@ -732,10 +732,10 @@ var _ = Describe("Network provider", func() {
 			})
 
 			It("should return error when subnetset ready status is not set", func() {
-				status := vpcapisv1.SubnetSetStatus{
-					Conditions: []vpcapisv1.Condition{},
+				status := nsxvpcv1.SubnetSetStatus{
+					Conditions: []nsxvpcv1.Condition{},
 				}
-				subnetsetObj = &vpcapisv1.SubnetSet{
+				subnetsetObj = &nsxvpcv1.SubnetSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: cluster.Namespace,
 						Name:      cluster.Name,

pkg/services/network/nsxt_vpc_provider.go

@@ -21,7 +21,7 @@ import (
 	"fmt"
 
 	"github.com/pkg/errors"
-	vpcapisv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+	nsxvpcv1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 	vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2"
 	vmoprv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common"
 	corev1 "k8s.io/api/core/v1"
@@ -40,11 +40,6 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services"
 )
 
-const (
-	// AnnotationEnableEndpointHealthCheckKey is the key of the annotation that is used to enable health check on the VMService endpoint port.
-	AnnotationEnableEndpointHealthCheckKey = "lb.iaas.vmware.com/enable-endpoint-health-check"
-)
-
 // nsxtVPCNetworkProvider provisions nsx-vpc type cluster network.
 type nsxtVPCNetworkProvider struct {
 	client client.Client
@@ -71,13 +66,13 @@ func (vp *nsxtVPCNetworkProvider) SupportsVMReadinessProbe() bool {
 // verifyNsxtVpcSubnetSetStatus checks the status conditions of a given SubnetSet within a cluster context.
 // If the subnet isn't ready, it is marked as false, and the function returns an error.
 // If the subnet is ready, the function updates the VSphereCluster with a "true" status and returns nil.
-func (vp *nsxtVPCNetworkProvider) verifyNsxtVpcSubnetSetStatus(vspherecluster *vmwarev1.VSphereCluster, subnetset *vpcapisv1.SubnetSet) error {
+func (vp *nsxtVPCNetworkProvider) verifyNsxtVpcSubnetSetStatus(vspherecluster *vmwarev1.VSphereCluster, subnetset *nsxvpcv1.SubnetSet) error {
 	clusterName := vspherecluster.Name
 	namespace := vspherecluster.Namespace
 	hasReadyCondition := false
 
 	for _, condition := range subnetset.Status.Conditions {
-		if condition.Type != vpcapisv1.Ready {
+		if condition.Type != nsxvpcv1.Ready {
 			continue
 		}
 		hasReadyCondition = true
@@ -101,7 +96,7 @@ func (vp *nsxtVPCNetworkProvider) verifyNsxtVpcSubnetSetStatus(vspherecluster *v
 // If it is, then it calls verifyNsxVpcSubnetSetStatus with the SubnetSet to verify its status.
 // If it's not, it returns an error.
 func (vp *nsxtVPCNetworkProvider) VerifyNetworkStatus(_ context.Context, clusterCtx *vmware.ClusterContext, obj runtime.Object) error {
-	subnetset, ok := obj.(*vpcapisv1.SubnetSet)
+	subnetset, ok := obj.(*nsxvpcv1.SubnetSet)
 	if !ok {
 		return fmt.Errorf("expected NSX VPC SubnetSet but got %T", obj)
 	}
@@ -124,14 +119,14 @@ func (vp *nsxtVPCNetworkProvider) ProvisionClusterNetwork(ctx context.Context, c
 	log.Info("Provisioning ")
 	defer log.Info("Finished provisioning")
 
-	subnetset := &vpcapisv1.SubnetSet{
+	subnetset := &nsxvpcv1.SubnetSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: networkNamespace,
 			Name:      networkName,
 		},
-		Spec: vpcapisv1.SubnetSetSpec{
-			AdvancedConfig: vpcapisv1.AdvancedConfig{
-				StaticIPAllocation: vpcapisv1.StaticIPAllocation{
+		Spec: nsxvpcv1.SubnetSetSpec{
+			AdvancedConfig: nsxvpcv1.AdvancedConfig{
+				StaticIPAllocation: nsxvpcv1.StaticIPAllocation{
 					Enable: true,
 				},
 			},
@@ -159,7 +154,7 @@ func (vp *nsxtVPCNetworkProvider) ProvisionClusterNetwork(ctx context.Context, c
 
 // GetClusterNetworkName returns the name of a valid cluster network if one exists.
 func (vp *nsxtVPCNetworkProvider) GetClusterNetworkName(ctx context.Context, clusterCtx *vmware.ClusterContext) (string, error) {
-	subnetset := &vpcapisv1.SubnetSet{}
+	subnetset := &nsxvpcv1.SubnetSet{}
 	cluster := clusterCtx.VSphereCluster
 	namespacedName := types.NamespacedName{
 		Namespace: cluster.Namespace,
@@ -173,6 +168,7 @@ func (vp *nsxtVPCNetworkProvider) GetClusterNetworkName(ctx context.Context, clu
 
 // The GetVMServiceAnnotations method always returns an empty map representing annotations.
 func (vp *nsxtVPCNetworkProvider) GetVMServiceAnnotations(_ context.Context, _ *vmware.ClusterContext) (map[string]string, error) {
+	// The value of the annotation lb.iaas.vmware.com/enable-endpoint-health-check is expected to be an empty string.
 	return map[string]string{AnnotationEnableEndpointHealthCheckKey: ""}, nil
 }