Add more port_forwarding tests

Extend set of tests for the port_forwarding feature to automate coverage
of specific cases:

  - Port forwaring on neutron ports with multiple fixed ips
  - Out of range values for port
  - Forward communication to multiple fixed IPs of a particular Neutron port
  - Editing and Deleting UDP port forwarding rule

Related-Bug: #1897753
Change-Id: I0fbf0a12c050a5a7184c96b62eee32139bc820b4
diff --git a/neutron_tempest_plugin/api/base.py b/neutron_tempest_plugin/api/base.py
index 7cf8dd4..d63dec8 100644
--- a/neutron_tempest_plugin/api/base.py
+++ b/neutron_tempest_plugin/api/base.py
@@ -702,6 +702,12 @@
         return pf
 
     @classmethod
+    def update_port_forwarding(cls, fip_id, pf_id, client=None, **kwargs):
+        """Wrapper utility for update_port_forwarding."""
+        client = client or cls.client
+        return client.update_port_forwarding(fip_id, pf_id, **kwargs)
+
+    @classmethod
     def delete_port_forwarding(cls, pf, client=None):
         """Delete port forwarding
 
diff --git a/neutron_tempest_plugin/api/test_port_forwarding_negative.py b/neutron_tempest_plugin/api/test_port_forwarding_negative.py
index 76dd6ee..386cbed 100644
--- a/neutron_tempest_plugin/api/test_port_forwarding_negative.py
+++ b/neutron_tempest_plugin/api/test_port_forwarding_negative.py
@@ -12,6 +12,7 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
 from tempest.lib import exceptions
@@ -81,3 +82,43 @@
             internal_ip_address=port['fixed_ips'][0]['ip_address'],
             internal_port=1111, external_port=5555,
             protocol="tcp")
+
+    @decorators.attr(type='negative')
+    @decorators.idempotent_id('e9d3ffb6-e5bf-421d-acaa-ee6010dfbf14')
+    def test_out_of_range_ports(self):
+        port = self.create_port(self.network)
+        fip_for_pf = self.create_floatingip()
+
+        pf_params = {
+            'internal_port_id': port['id'],
+            'internal_ip_address': port['fixed_ips'][0]['ip_address'],
+            'internal_port': 1111,
+            'external_port': 3333,
+            'protocol': "tcp"}
+        pf = self.create_port_forwarding(fip_for_pf['id'], **pf_params)
+
+        # Check: Invalid input for external_port update
+        self.assertRaises(
+            exceptions.BadRequest,
+            self.update_port_forwarding,
+            fip_for_pf['id'], pf['id'], external_port=108343)
+
+        # Check: Invalid input for internal_port update
+        self.assertRaises(
+            exceptions.BadRequest,
+            self.update_port_forwarding,
+            fip_for_pf['id'], pf['id'], internal_port=108343)
+
+        # Check: Invalid input for external_port create
+        pf_params['internal_port'] = 4444
+        pf_params['external_port'] = 333333
+        self.assertRaises(
+            exceptions.BadRequest,
+            self.create_port_forwarding, fip_for_pf['id'], **pf_params)
+
+        # Check: Invalid input for internal_port create
+        pf_params['internal_port'] = 444444
+        pf_params['external_port'] = 3333
+        self.assertRaises(
+            exceptions.BadRequest,
+            self.create_port_forwarding, fip_for_pf['id'], **pf_params)
diff --git a/neutron_tempest_plugin/api/test_port_forwardings.py b/neutron_tempest_plugin/api/test_port_forwardings.py
index 82c3a34..79cce39 100644
--- a/neutron_tempest_plugin/api/test_port_forwardings.py
+++ b/neutron_tempest_plugin/api/test_port_forwardings.py
@@ -124,31 +124,10 @@
         fip = self.client.show_floatingip(fip['id'])['floatingip']
         self.assertEqual(0, len(fip['port_forwardings']))
 
-    @decorators.idempotent_id('8202cded-7e82-4420-9585-c091105404f6')
-    def test_associate_2_port_forwardings_to_floating_ip(self):
-        fip = self.create_floatingip()
-        forwardings_data = [(1111, 2222), (3333, 4444)]
-        created_pfs = []
-        for data in forwardings_data:
-            internal_port = data[0]
-            external_port = data[1]
-            port = self.create_port(self.network)
-            created_pf = self.create_port_forwarding(
-                fip['id'],
-                internal_port_id=port['id'],
-                internal_ip_address=port['fixed_ips'][0]['ip_address'],
-                internal_port=internal_port, external_port=external_port,
-                protocol="tcp")
-            self.assertEqual(internal_port, created_pf['internal_port'])
-            self.assertEqual(external_port, created_pf['external_port'])
-            self.assertEqual('tcp', created_pf['protocol'])
-            self.assertEqual(port['fixed_ips'][0]['ip_address'],
-                             created_pf['internal_ip_address'])
-            created_pfs.append(created_pf)
-
+    def _verify_created_pfs(self, fip_id, created_pfs):
         # Check that all PFs are visible in Floating IP details
-        fip = self.client.show_floatingip(fip['id'])['floatingip']
-        self.assertEqual(len(forwardings_data), len(fip['port_forwardings']))
+        fip = self.client.show_floatingip(fip_id)['floatingip']
+        self.assertEqual(len(created_pfs), len(fip['port_forwardings']))
         for pf in created_pfs:
             expected_pf = {
                 'external_port': pf['external_port'],
@@ -160,13 +139,73 @@
         # Test list of port forwardings
         port_forwardings = self.client.list_port_forwardings(
             fip['id'])['port_forwardings']
-        self.assertEqual(len(forwardings_data), len(port_forwardings))
+        self.assertEqual(len(created_pfs), len(port_forwardings))
         for pf in created_pfs:
             expected_pf = pf.copy()
             expected_pf.pop('client')
             expected_pf.pop('floatingip_id')
             self.assertIn(expected_pf, port_forwardings)
 
+    def _create_and_validate_pf(self, fip_id, internal_port_id,
+                                internal_ip_address, internal_port,
+                                external_port, protocol):
+        created_pf = self.create_port_forwarding(
+            fip_id,
+            internal_port_id=internal_port_id,
+            internal_ip_address=internal_ip_address,
+            internal_port=internal_port,
+            external_port=external_port,
+            protocol=protocol)
+        self.assertEqual(internal_port, created_pf['internal_port'])
+        self.assertEqual(external_port, created_pf['external_port'])
+        self.assertEqual(protocol, created_pf['protocol'])
+        self.assertEqual(internal_ip_address,
+                         created_pf['internal_ip_address'])
+        return created_pf
+
+    @decorators.idempotent_id('8202cded-7e82-4420-9585-c091105404f6')
+    def test_associate_2_port_forwardings_to_floating_ip(self):
+        fip = self.create_floatingip()
+        forwardings_data = [(1111, 2222), (3333, 4444)]
+        created_pfs = []
+        for data in forwardings_data:
+            internal_port = data[0]
+            external_port = data[1]
+            port = self.create_port(self.network)
+            created_pf = self._create_and_validate_pf(
+                fip_id=fip['id'],
+                internal_port_id=port['id'],
+                internal_ip_address=port['fixed_ips'][0]['ip_address'],
+                internal_port=internal_port, external_port=external_port,
+                protocol="tcp")
+            created_pfs.append(created_pf)
+        self._verify_created_pfs(fip['id'], created_pfs)
+
+    @decorators.idempotent_id('a7e6cc48-8a9b-49be-82fb-cef6f5c29381')
+    def test_associate_port_forwarding_to_2_fixed_ips(self):
+        fip = self.create_floatingip()
+        port = self.create_port(self.network)
+        internal_subnet_id = port['fixed_ips'][0]['subnet_id']
+        # Add a second fixed_ip address to port (same subnet)
+        port['fixed_ips'].append({'subnet_id': internal_subnet_id})
+        port = self.update_port(port, fixed_ips=port['fixed_ips'])
+        internal_ip_address1 = port['fixed_ips'][0]['ip_address']
+        internal_ip_address2 = port['fixed_ips'][1]['ip_address']
+        forwardings_data = [(4001, internal_ip_address1),
+                            (4002, internal_ip_address2)]
+        created_pfs = []
+        for data in forwardings_data:
+            external_port = data[0]
+            internal_ip_address = data[1]
+            created_pf = self._create_and_validate_pf(
+                fip_id=fip['id'],
+                internal_port_id=port['id'],
+                internal_ip_address=internal_ip_address,
+                internal_port=123, external_port=external_port,
+                protocol="tcp")
+            created_pfs.append(created_pf)
+        self._verify_created_pfs(fip['id'], created_pfs)
+
     @decorators.idempotent_id('6a34e811-66d1-4f63-aa4d-9013f15deb62')
     def test_associate_port_forwarding_to_used_floating_ip(self):
         port_for_fip = self.create_port(self.network)
diff --git a/neutron_tempest_plugin/scenario/base.py b/neutron_tempest_plugin/scenario/base.py
index 78b766b..402a901 100644
--- a/neutron_tempest_plugin/scenario/base.py
+++ b/neutron_tempest_plugin/scenario/base.py
@@ -452,7 +452,8 @@
         self.wait_for_server_status(
             server, constants.SERVER_STATUS_ACTIVE, client)
 
-    def check_servers_hostnames(self, servers, timeout=None, log_errors=True):
+    def check_servers_hostnames(self, servers, timeout=None, log_errors=True,
+                                external_port=None):
         """Compare hostnames of given servers with their names."""
         try:
             for server in servers:
@@ -460,7 +461,7 @@
                 if timeout:
                     kwargs['timeout'] = timeout
                 try:
-                    kwargs['port'] = (
+                    kwargs['port'] = external_port or (
                         server['port_forwarding_tcp']['external_port'])
                 except KeyError:
                     pass
diff --git a/neutron_tempest_plugin/scenario/test_port_forwardings.py b/neutron_tempest_plugin/scenario/test_port_forwardings.py
index 3158ea0..4080bca 100644
--- a/neutron_tempest_plugin/scenario/test_port_forwardings.py
+++ b/neutron_tempest_plugin/scenario/test_port_forwardings.py
@@ -45,9 +45,14 @@
         cls.secgroup = cls.create_security_group(
             name=data_utils.rand_name("test_port_secgroup"))
         cls.create_loginable_secgroup_rule(secgroup_id=cls.secgroup['id'])
+        udp_sg_rule = {'protocol': constants.PROTO_NAME_UDP,
+                       'direction': constants.INGRESS_DIRECTION,
+                       'remote_ip_prefix': '0.0.0.0/0'}
+        cls.create_secgroup_rules(
+            [udp_sg_rule], secgroup_id=cls.secgroup['id'])
         cls.keypair = cls.create_keypair()
 
-    def _prepare_resources(self, num_servers, internal_tcp_port, protocol,
+    def _prepare_resources(self, num_servers, internal_tcp_port=22,
                            external_port_base=1025):
         servers = []
         for i in range(1, num_servers + 1):
@@ -82,7 +87,7 @@
             servers.append(server)
         return servers
 
-    def _test_udp_port_forwarding(self, servers):
+    def _test_udp_port_forwarding(self, servers, timeout=None):
 
         def _message_received(server, ssh_client, expected_msg):
             self.nc_listen(ssh_client,
@@ -103,23 +108,22 @@
                 CONF.validation.image_ssh_user,
                 pkey=self.keypair['private_key'],
                 port=server['port_forwarding_tcp']['external_port'])
+            wait_params = {
+                'exception': RuntimeError(
+                    "Timed out waiting for message from server {!r} ".format(
+                        server['id']))
+            }
+            if timeout:
+                wait_params['timeout'] = timeout
             utils.wait_until_true(
                 lambda: _message_received(server, ssh_client, expected_msg),
-                exception=RuntimeError(
-                    "Timed out waiting for message from server {!r} ".format(
-                        server['id'])))
+                **wait_params)
 
     @test.unstable_test("bug 1896735")
     @decorators.idempotent_id('ab40fc48-ca8d-41a0-b2a3-f6679c847bfe')
     def test_port_forwarding_to_2_servers(self):
-        udp_sg_rule = {'protocol': constants.PROTO_NAME_UDP,
-                       'direction': constants.INGRESS_DIRECTION,
-                       'remote_ip_prefix': '0.0.0.0/0'}
-        self.create_secgroup_rules(
-            [udp_sg_rule], secgroup_id=self.secgroup['id'])
-        servers = self._prepare_resources(
-            num_servers=2, internal_tcp_port=22,
-            protocol=constants.PROTO_NAME_TCP)
+        servers = self._prepare_resources(num_servers=2,
+                                          external_port_base=1035)
         # Test TCP port forwarding by SSH to each server
         self.check_servers_hostnames(servers)
         # And now test UDP port forwarding using nc
@@ -128,25 +132,16 @@
     @test.unstable_test("bug 1896735")
     @decorators.idempotent_id('aa19d46c-a4a6-11ea-bb37-0242ac130002')
     def test_port_forwarding_editing_and_deleting_tcp_rule(self):
-        server = self._prepare_resources(
-            num_servers=1, internal_tcp_port=22,
-            protocol=constants.PROTO_NAME_TCP,
-            external_port_base=1035)
+        test_ext_port = 3333
+        server = self._prepare_resources(num_servers=1,
+                                         external_port_base=1045)
         fip_id = server[0]['port_forwarding_tcp']['floatingip_id']
         pf_id = server[0]['port_forwarding_tcp']['id']
 
         # Check connectivity with the original parameters
         self.check_servers_hostnames(server)
 
-        # Use a reasonable timeout to verify that connections will not
-        # happen. Default would be 196 seconds, which is an overkill.
-        test_ssh_connect_timeout = 6
-
-        # Update external port and check connectivity with original parameters
-        # Port under server[0]['port_forwarding_tcp']['external_port'] should
-        # not answer at this point.
-
-        def fip_pf_connectivity():
+        def fip_pf_connectivity(test_ssh_connect_timeout=60):
             try:
                 self.check_servers_hostnames(
                     server, timeout=test_ssh_connect_timeout)
@@ -155,9 +150,13 @@
                 return False
 
         def no_fip_pf_connectivity():
-            return not fip_pf_connectivity()
+            return not fip_pf_connectivity(6)
 
-        self.client.update_port_forwarding(fip_id, pf_id, external_port=3333)
+        # Update external port and check connectivity with original parameters
+        # Port under server[0]['port_forwarding_tcp']['external_port'] should
+        # not answer at this point.
+        self.client.update_port_forwarding(fip_id, pf_id,
+                                           external_port=test_ext_port)
         utils.wait_until_true(
             no_fip_pf_connectivity,
             exception=RuntimeError(
@@ -167,7 +166,7 @@
                     server[0]['port_forwarding_tcp']['external_port'])))
 
         # Check connectivity with the new parameters
-        server[0]['port_forwarding_tcp']['external_port'] = 3333
+        server[0]['port_forwarding_tcp']['external_port'] = test_ext_port
         utils.wait_until_true(
             fip_pf_connectivity,
             exception=RuntimeError(
@@ -187,3 +186,105 @@
                 "port {!r} is still possible.".format(
                     server[0]['id'],
                     server[0]['port_forwarding_tcp']['external_port'])))
+
+    @test.unstable_test("bug 1896735")
+    @decorators.idempotent_id('6d05b1b2-6109-4c30-b402-1503f4634acb')
+    def test_port_forwarding_editing_and_deleting_udp_rule(self):
+        test_ext_port = 3344
+        server = self._prepare_resources(num_servers=1,
+                                         external_port_base=1055)
+        fip_id = server[0]['port_forwarding_udp']['floatingip_id']
+        pf_id = server[0]['port_forwarding_udp']['id']
+
+        # Check connectivity with the original parameters
+        self.check_servers_hostnames(server)
+
+        def fip_pf_udp_connectivity(test_udp_timeout=60):
+            try:
+                self._test_udp_port_forwarding(server, test_udp_timeout)
+                return True
+            except (AssertionError, RuntimeError):
+                return False
+
+        def no_fip_pf_udp_connectivity():
+            return not fip_pf_udp_connectivity(6)
+
+        # Update external port and check connectivity with original parameters
+        # Port under server[0]['port_forwarding_udp']['external_port'] should
+        # not answer at this point.
+        self.client.update_port_forwarding(fip_id, pf_id,
+                                           external_port=test_ext_port)
+        utils.wait_until_true(
+            no_fip_pf_udp_connectivity,
+            exception=RuntimeError(
+                "Connection to the server {!r} through "
+                "port {!r} is still possible.".format(
+                    server[0]['id'],
+                    server[0]['port_forwarding_udp']['external_port'])))
+
+        # Check connectivity with the new parameters
+        server[0]['port_forwarding_udp']['external_port'] = test_ext_port
+        utils.wait_until_true(
+            fip_pf_udp_connectivity,
+            exception=RuntimeError(
+                "Connection to the server {!r} through "
+                "port {!r} is not possible.".format(
+                    server[0]['id'],
+                    server[0]['port_forwarding_udp']['external_port'])))
+
+        # Remove port forwarding and ensure connection stops working.
+        self.client.delete_port_forwarding(fip_id, pf_id)
+        self.assertRaises(lib_exc.NotFound, self.client.get_port_forwarding,
+                          fip_id, pf_id)
+        utils.wait_until_true(
+            no_fip_pf_udp_connectivity,
+            exception=RuntimeError(
+                "Connection to the server {!r} through "
+                "port {!r} is still possible.".format(
+                    server[0]['id'],
+                    server[0]['port_forwarding_udp']['external_port'])))
+
+    @test.unstable_test("bug 1896735")
+    @decorators.idempotent_id('5971881d-06a0-459e-b636-ce5d1929e2d4')
+    def test_port_forwarding_to_2_fixed_ips(self):
+        port = self.create_port(self.network,
+            security_groups=[self.secgroup['id']])
+        name = data_utils.rand_name("server-0")
+        server = self.create_server(flavor_ref=CONF.compute.flavor_ref,
+            image_ref=CONF.compute.image_ref, key_name=self.keypair['name'],
+            name=name, networks=[{'port': port['id']}])['server']
+        server['name'] = name
+        self.wait_for_server_active(server)
+
+        # Add a second fixed_ip address to port (same subnet)
+        internal_subnet_id = port['fixed_ips'][0]['subnet_id']
+        port['fixed_ips'].append({'subnet_id': internal_subnet_id})
+        port = self.update_port(port, fixed_ips=port['fixed_ips'])
+        internal_ip_address1 = port['fixed_ips'][0]['ip_address']
+        internal_ip_address2 = port['fixed_ips'][1]['ip_address']
+        pfs = []
+        for ip_address, external_port in [(internal_ip_address1, 1066),
+                                          (internal_ip_address2, 1067)]:
+            pf = self.create_port_forwarding(
+                self.fip['id'], internal_port_id=port['id'],
+                internal_ip_address=ip_address,
+                internal_port=22, external_port=external_port,
+                protocol=constants.PROTO_NAME_TCP)
+            pfs.append(pf)
+
+        test_ssh_connect_timeout = 32
+        number_of_connects = 0
+        for pf in pfs:
+            try:
+                self.check_servers_hostnames(
+                    [server], timeout=test_ssh_connect_timeout,
+                    external_port=pf['external_port'])
+                number_of_connects += 1
+            except (AssertionError, lib_exc.SSHTimeout):
+                pass
+
+        # TODO(flaviof): Quite possibly, the server is using only one of the
+        # fixed ips associated with the neutron port. Being so, we should not
+        # fail the test, as long as at least one connection was successful.
+        self.assertGreaterEqual(
+            number_of_connects, 1, "Did not connect via FIP port forwarding")