From 01dd9a1cced411804f53d57bbc23f4ea15ff3a38 Mon Sep 17 00:00:00 2001
From: asarfaty <asarfaty@vmware.com>
Date: Tue, 24 Mar 2020 12:33:40 +0200
Subject: [PATCH] Fix segment port attach & detach

1. Support tags in detach
2. Support setting vif_id in detach (reset other attachment
   attributes)
3. Use update instead of create for attach, to keep original
   port attributes

Change-Id: I7093fbf70a76a7560c9174b209259f167b21f74f
(cherry picked from commit 394dadd07fb019592d83f9f4f1264c3fbd39858b)
---
 .../tests/unit/v3/policy/test_resources.py    | 20 +++++++++++-
 vmware_nsxlib/v3/policy/core_resources.py     | 32 +++++++++++--------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
index 37986849..d070c605 100644
--- a/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
+++ b/vmware_nsxlib/tests/unit/v3/policy/test_resources.py
@@ -4228,7 +4228,25 @@ class TestPolicySegmentPort(NsxPolicyLibTestCase):
             api_get.assert_called_once()
             api_put.assert_called_once_with(
                 "%s/segments/%s/ports/%s" % (TEST_TENANT, segment_id, port_id),
-                {'attachment': None})
+                {'attachment': None, 'tags': tags})
+
+    def test_detach_with_vif(self):
+        segment_id = "segment"
+        port_id = "port"
+        vif_id = "abc"
+        tags = [{'scope': 'a', 'tag': 'b'}]
+        with mock.patch.object(self.policy_api.client,
+                               "get", return_value={}) as api_get,\
+            mock.patch.object(self.policy_api.client,
+                              "update") as api_put:
+            self.resourceApi.detach(
+                segment_id, port_id, tags=tags, vif_id=vif_id,
+                tenant=TEST_TENANT)
+
+            api_get.assert_called_once()
+            api_put.assert_called_once_with(
+                "%s/segments/%s/ports/%s" % (TEST_TENANT, segment_id, port_id),
+                {'attachment': {'id': vif_id}, 'tags': tags})
 
 
 class TestPolicySegmentProfileBase(NsxPolicyLibTestCase):
diff --git a/vmware_nsxlib/v3/policy/core_resources.py b/vmware_nsxlib/v3/policy/core_resources.py
index 6fb1d3aa..efbd581e 100644
--- a/vmware_nsxlib/v3/policy/core_resources.py
+++ b/vmware_nsxlib/v3/policy/core_resources.py
@@ -2071,8 +2071,9 @@ class NsxPolicySegmentPortApi(NsxPolicyResourceBase):
                      tags=tags,
                      tenant=tenant)
 
-    def detach(self, segment_id, port_id, tags=IGNORE,
+    def detach(self, segment_id, port_id, vif_id=None, tags=IGNORE,
                tenant=constants.POLICY_INFRA_TENANT):
+        """Reset the attachment with or without a vif_id"""
         # Due to platform limitation, PUT should be used here and not PATCH
         port_def = self.entry_def(
             segment_id=segment_id,
@@ -2085,7 +2086,12 @@ class NsxPolicySegmentPortApi(NsxPolicyResourceBase):
             max_attempts=self.policy_api.client.max_attempts)
         def _detach():
             port = self.policy_api.get(port_def)
-            port['attachment'] = None
+            if vif_id:
+                port['attachment'] = {'id': vif_id}
+            else:
+                port['attachment'] = None
+            if tags != IGNORE:
+                port['tags'] = tags
             self.policy_api.client.update(path, port)
 
         _detach()
@@ -2100,18 +2106,16 @@ class NsxPolicySegmentPortApi(NsxPolicyResourceBase):
                tags=IGNORE,
                tenant=constants.POLICY_INFRA_TENANT):
 
-        port_def = self.entry_def(segment_id=segment_id,
-                                  port_id=port_id,
-                                  attachment_type=attachment_type,
-                                  allocate_addresses=allocate_addresses,
-                                  vif_id=vif_id,
-                                  app_id=app_id,
-                                  context_id=context_id,
-                                  traffic_tag=traffic_tag,
-                                  tags=tags,
-                                  tenant=tenant)
-
-        self.policy_api.create_or_update(port_def)
+        self._update(segment_id=segment_id,
+                     port_id=port_id,
+                     attachment_type=attachment_type,
+                     allocate_addresses=allocate_addresses,
+                     vif_id=vif_id,
+                     app_id=app_id,
+                     context_id=context_id,
+                     traffic_tag=traffic_tag,
+                     tags=tags,
+                     tenant=tenant)
 
     def get_realized_state(self, segment_id, port_id, entity_type=None,
                            tenant=constants.POLICY_INFRA_TENANT,