Merge pull request #520 from heypnus/subnetport/attachmentref_anno

Move SubnetPort.spec.AttachmentRef to Annotations

build/yaml/crd/nsx.vmware.com_subnetports.yaml

@@ -48,44 +48,6 @@ spec:
           spec:
             description: SubnetPortSpec defines the desired state of SubnetPort.
             properties:
-              attachmentRef:
-                description: AttachmentRef refers to the virtual machine which the
-                  SubnetPort is attached.
-                properties:
-                  apiVersion:
-                    description: API version of the referent.
-                    type: string
-                  fieldPath:
-                    description: 'If referring to a piece of an object instead of
-                      an entire object, this string should contain a valid JSON/Go
-                      field access statement, such as desiredState.manifest.containers[2].
-                      For example, if the object reference is to a container within
-                      a pod, this would take on a value like: "spec.containers{name}"
-                      (where "name" refers to the name of the container that triggered
-                      the event) or if no container name is specified "spec.containers[2]"
-                      (container with index 2 in this pod). This syntax is chosen
-                      only to have some well-defined way of referencing a part of
-                      an object. TODO: this design is not final and this field is
-                      subject to change in the future.'
-                    type: string
-                  kind:
-                    description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
-                    type: string
-                  name:
-                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
-                    type: string
-                  namespace:
-                    description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
-                    type: string
-                  resourceVersion:
-                    description: 'Specific resourceVersion to which this reference
-                      is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
-                    type: string
-                  uid:
-                    description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
-                    type: string
-                type: object
-                x-kubernetes-map-type: atomic
               subnet:
                 description: Subnet defines the parent Subnet name of the SubnetPort.
                 type: string

pkg/apis/nsx.vmware.com/v1alpha1/subnetport_types.go

@@ -4,7 +4,6 @@
 package v1alpha1
 
 import (
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -14,8 +13,6 @@ type SubnetPortSpec struct {
 	Subnet string `json:"subnet,omitempty"`
 	// SubnetSet defines the parent SubnetSet name of the SubnetPort.
 	SubnetSet string `json:"subnetSet,omitempty"`
-	// AttachmentRef refers to the virtual machine which the SubnetPort is attached.
-	AttachmentRef corev1.ObjectReference `json:"attachmentRef,omitempty"`
 }
 
 // SubnetPortStatus defines the observed state of SubnetPort.

pkg/apis/v1alpha1/subnetport_types.go

@@ -4,7 +4,6 @@
 package v1alpha1
 
 import (
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -14,8 +13,6 @@ type SubnetPortSpec struct {
 	Subnet string `json:"subnet,omitempty"`
 	// SubnetSet defines the parent SubnetSet name of the SubnetPort.
 	SubnetSet string `json:"subnetSet,omitempty"`
-	// AttachmentRef refers to the virtual machine which the SubnetPort is attached.
-	AttachmentRef corev1.ObjectReference `json:"attachmentRef,omitempty"`
 }
 
 // SubnetPortStatus defines the observed state of SubnetPort.

pkg/controllers/common/utils.go

@@ -125,3 +125,20 @@ func NodeIsMaster(node *v1.Node) bool {
 	}
 	return false
 }
+
+func GetVirtualMachineNameForSubnetPort(subnetPort *v1alpha1.SubnetPort) (string, error) {
+	annotations := subnetPort.GetAnnotations()
+	if annotations == nil {
+		return "", nil
+	}
+	attachmentRef, exist := annotations[servicecommon.AnnotationAttachmentRef]
+	if !exist {
+		return "", nil
+	}
+	array := strings.Split(attachmentRef, "/")
+	if len(array) != 2 || !strings.EqualFold(array[0], servicecommon.ResourceTypeVirtualMachine) {
+		err := fmt.Errorf("invalid annotation value of '%s': %s", servicecommon.AnnotationAttachmentRef, attachmentRef)
+		return "", err
+	}
+	return array[1], nil
+}

pkg/controllers/common/utils_test.go

@@ -0,0 +1,59 @@
+package common
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1"
+)
+
+func TestGetVirtualMachineNameForSubnetPort(t *testing.T) {
+	type args struct {
+		subnetPort *v1alpha1.SubnetPort
+	}
+	type want struct {
+		vm  string
+		err error
+	}
+	tests := []struct {
+		name string
+		args args
+		want want
+	}{
+		{
+			"port_with_annotation",
+			args{&v1alpha1.SubnetPort{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"nsx.vmware.com/attachment_ref": "virtualmachine/abc",
+					},
+				}}},
+			want{vm: "abc", err: nil},
+		},
+		{
+			"port_without_annotation",
+			args{&v1alpha1.SubnetPort{}},
+			want{vm: "", err: nil},
+		},
+		{
+			"port_with_invalid_annotation",
+			args{&v1alpha1.SubnetPort{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"nsx.vmware.com/attachment_ref": "invalid/abc",
+					},
+				}}},
+			want{vm: "", err: fmt.Errorf("invalid annotation value of 'nsx.vmware.com/attachment_ref': invalid/abc")},
+		},
+	}
+	for _, tt := range tests {
+		got, err := GetVirtualMachineNameForSubnetPort(tt.args.subnetPort)
+		assert.Equal(t, err, tt.want.err)
+		if got != tt.want.vm {
+			t.Errorf("%s failed: got %s, want %s", tt.name, got, tt.want.vm)
+		}
+	}
+}

pkg/controllers/subnetport/subnetport_controller.go

@@ -94,10 +94,9 @@ func (r *SubnetPortReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 		labels, err := r.getLabelsFromVirtualMachine(ctx, subnetPort)
 		if err != nil {
-			log.Error(err, "failed to get labels from virtualmachine", "subnetPort.Name", subnetPort.Name, "subnetPort.UID", subnetPort.UID, "subnetPort.Spec.AttachmentRef", subnetPort.Spec.AttachmentRef)
+			log.Error(err, "failed to get labels from virtualmachine", "subnetPort.Name", subnetPort.Name, "subnetPort.UID", subnetPort.UID)
 			return common.ResultRequeue, err
 		}
-		log.Info("got labels from virtualmachine for subnetport", "subnetPort.UID", subnetPort.UID, "virtualmachine name", subnetPort.Spec.AttachmentRef.Name, "labels", labels)
 		nsxSubnetPortState, err := r.SubnetPortService.CreateOrUpdateSubnetPort(subnetPort, nsxSubnetPath, "", labels)
 		if err != nil {
 			log.Error(err, "failed to create or update NSX subnet port, would retry exponentially", "subnetport", req.NamespacedName)
@@ -180,8 +179,13 @@ func (r *SubnetPortReconciler) vmMapFunc(_ context.Context, vm client.Object) []
 		return requests
 	}
 	for _, subnetPort := range subnetPortList.Items {
-		if subnetPort.Spec.AttachmentRef.Name == vm.GetName() && (subnetPort.Spec.AttachmentRef.Namespace == vm.GetNamespace() ||
-			(subnetPort.Spec.AttachmentRef.Namespace == "" && subnetPort.Namespace == vm.GetNamespace())) {
+		port := subnetPort
+		vmName, err := common.GetVirtualMachineNameForSubnetPort(&port)
+		if err != nil {
+			// not block the subnetport visiting because of invalid annotations
+			log.Error(err, "failed to get virtualmachine name from subnetport", "subnetPort.UID", subnetPort.UID)
+		}
+		if vmName == vm.GetName() && subnetPort.Namespace == vm.GetNamespace() {
 			requests = append(requests, reconcile.Request{
 				NamespacedName: types.NamespacedName{
 					Name:      subnetPort.Name,
@@ -424,20 +428,18 @@ func (r *SubnetPortReconciler) updateSubnetStatusOnSubnetPort(subnetPort *v1alph
 }
 
 func (r *SubnetPortReconciler) getLabelsFromVirtualMachine(ctx context.Context, subnetPort *v1alpha1.SubnetPort) (*map[string]string, error) {
-	if subnetPort.Spec.AttachmentRef.Name == "" {
-		return nil, nil
+	vmName, err := common.GetVirtualMachineNameForSubnetPort(subnetPort)
+	if vmName == "" {
+		return nil, err
 	}
 	vm := &vmv1alpha1.VirtualMachine{}
-	namespace := subnetPort.Spec.AttachmentRef.Namespace
-	if len(namespace) == 0 {
-		namespace = subnetPort.Namespace
-	}
 	namespacedName := types.NamespacedName{
-		Name:      subnetPort.Spec.AttachmentRef.Name,
-		Namespace: namespace,
+		Name:      vmName,
+		Namespace: subnetPort.Namespace,
 	}
 	if err := r.Client.Get(ctx, namespacedName, vm); err != nil {
 		return nil, err
 	}
+	log.Info("got labels from virtualmachine for subnetport", "subnetPort.UID", subnetPort.UID, "vmName", vmName, "labels", vm.ObjectMeta.Labels)
 	return &vm.ObjectMeta.Labels, nil
 }

pkg/nsx/services/common/types.go

@@ -73,6 +73,7 @@ const (
 	AnnotationVPCNetworkConfig         string = "nsx.vmware.com/vpc_network_config"
 	AnnotationVPCName                  string = "nsx.vmware.com/vpc_name"
 	AnnotationDefaultNetworkConfig     string = "nsx.vmware.com/default"
+	AnnotationAttachmentRef            string = "nsx.vmware.com/attachment_ref"
 	AnnotationPodMAC                   string = "nsx.vmware.com/mac"
 	AnnotationPodAttachment            string = "nsx.vmware.com/attachment"
 	TagScopePodName                    string = "nsx-op/pod_name"

test/e2e/manifest/testSubnetPort/port_with_attachment_ref.yaml

@@ -0,0 +1,9 @@
+apiVersion: nsx.vmware.com/v1alpha1
+kind: SubnetPort
+metadata:
+  name: vm-port
+  namespace: subnetport-e2e
+  annotations:
+    nsx.vmware.com/attachment_ref: virtualmachine/vm1
+spec:
+  subnetSet: vm-default

test/e2e/manifest/testSubnetPort/vm_without_labels.yaml

@@ -0,0 +1,6 @@
+apiVersion: vmoperator.vmware.com/v1alpha1
+kind: VirtualMachine
+metadata:
+  name: vm1
+  namespace: subnetport-e2e
+spec: