Backport Fixes-race-condition-in-test_add_remove_fixed_ip 47/69947/3
authorCédric Ollivier <cedric.ollivier@orange.com>
Tue, 14 Apr 2020 09:34:01 +0000 (11:34 +0200)
committerCédric Ollivier <cedric.ollivier@orange.com>
Tue, 14 Apr 2020 11:04:25 +0000 (13:04 +0200)
It has sometimes failed in gates and Orange reported the issue.
Backporting is the best approach as for swift testing.

Change-Id: I7c66faf77cdee3f52a2299d64dc4140de22f4cad
Signed-off-by: Cédric Ollivier <cedric.ollivier@orange.com>
(cherry picked from commit ab26ce1cc28c590115ac07ea4a51b9f75f902c9a)
(cherry picked from commit a710d74306c0a49c19c63676e0aab2f4e093aa26)

docker/tempest/Dockerfile
docker/tempest/Fixes-race-condition-in-test_add_remove_fixed_ip.patch [new file with mode: 0644]

index 0f78227..134a7f1 100644 (file)
@@ -8,6 +8,7 @@ ARG RALLY_OPENSTACK_TAG=1.5.0
 ARG UJSON_TAG=d25e024f481c5571d15f3c0c406a498ca0467cfd
 
 COPY Accept-custom-registered-endpoints.patch /tmp/Accept-custom-registered-endpoints.patch
+COPY Fixes-race-condition-in-test_add_remove_fixed_ip.patch /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch
 RUN apk --no-cache add --virtual .build-deps --update \
         python-dev build-base linux-headers libffi-dev \
         openssl-dev libjpeg-turbo-dev && \
@@ -39,7 +40,9 @@ RUN apk --no-cache add --virtual .build-deps --update \
         git config --global user.email "opnfv-tech-discuss@lists.opnfv.org" && \
         git config --global user.name "Functest" && \
         patch -p1 < /tmp/Accept-custom-registered-endpoints.patch && \
-        git commit -a -m "Apply Accept-custom-registered-endpoints" && \
+        patch -p1 < /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch && \
+        git commit -a -m "Backport critical bugfixes" && \
         rm ~/.gitconfig) && \
     rm /tmp/Accept-custom-registered-endpoints.patch && \
+    rm /tmp/Fixes-race-condition-in-test_add_remove_fixed_ip.patch && \
     apk del .build-deps
diff --git a/docker/tempest/Fixes-race-condition-in-test_add_remove_fixed_ip.patch b/docker/tempest/Fixes-race-condition-in-test_add_remove_fixed_ip.patch
new file mode 100644 (file)
index 0000000..a9f3c7e
--- /dev/null
@@ -0,0 +1,165 @@
+From 61a3c8efa4a5e41dc6b5fd2d7a28a25555ebb54b Mon Sep 17 00:00:00 2001
+From: David Sedlák <dsedlak@redhat.com>
+Date: Wed, 30 Oct 2019 15:38:21 +0100
+Subject: [PATCH] Fixes race condition in test_add_remove_fixed_ip
+
+Currently race condition can occure in
+tempest.api.compute.servers.test_attach_interfaces.
+AttachInterfacesUnderV243Test.test_add_remove_fixed_ip
+when floating IP added during resource preparation doesn't appear in
+the list of original IPs that is created at the beggining of the test,
+which then confuses the test
+and floating ip is later recognized as fixed IP added in the test.
+more details including log:
+https://bugzilla.redhat.com/show_bug.cgi?id=1752416
+
+This change ensures possible floating IP added during server
+creation is always present in the set of original IPs and also
+during every comparasion of IPs.
+
+Closes-Bug: #1866179
+
+Change-Id: Ic3a3e0708714b6d6c9c226e641e1c520e5ebde9d
+Signed-off-by: David Sedlák <dsedlak@redhat.com>
+---
+
+diff --git a/tempest/api/compute/servers/test_attach_interfaces.py b/tempest/api/compute/servers/test_attach_interfaces.py
+index df8da07..c1af6c7 100644
+--- a/tempest/api/compute/servers/test_attach_interfaces.py
++++ b/tempest/api/compute/servers/test_attach_interfaces.py
+@@ -86,12 +86,16 @@
+         # apparently not enough? Add cleanup here.
+         self.addCleanup(self.delete_server, server['id'])
+         self._wait_for_validation(server, validation_resources)
++        try:
++            fip = set([validation_resources['floating_ip']['ip']])
++        except KeyError:
++            fip = ()
+         ifs = (self.interfaces_client.list_interfaces(server['id'])
+                ['interfaceAttachments'])
+         body = waiters.wait_for_interface_status(
+             self.interfaces_client, server['id'], ifs[0]['port_id'], 'ACTIVE')
+         ifs[0]['port_state'] = body['port_state']
+-        return server, ifs
++        return server, ifs, fip
+ class AttachInterfacesTestJSON(AttachInterfacesTestBase):
+@@ -226,7 +230,7 @@
+     @decorators.idempotent_id('73fe8f02-590d-4bf1-b184-e9ca81065051')
+     @utils.services('network')
+     def test_create_list_show_delete_interfaces_by_network_port(self):
+-        server, ifs = self._create_server_get_interfaces()
++        server, ifs, _ = self._create_server_get_interfaces()
+         interface_count = len(ifs)
+         self.assertGreater(interface_count, 0)
+@@ -268,7 +272,7 @@
+             raise self.skipException("Only owner network supports "
+                                      "creating interface by fixed ip.")
+-        server, ifs = self._create_server_get_interfaces()
++        server, ifs, _ = self._create_server_get_interfaces()
+         interface_count = len(ifs)
+         self.assertGreater(interface_count, 0)
+@@ -354,9 +358,8 @@
+                 not CONF.network.shared_physical_network):
+             raise self.skipException("Only owner network supports "
+                                      "creating interface by fixed ip.")
+-
+         # Add and Remove the fixed IP to server.
+-        server, ifs = self._create_server_get_interfaces()
++        server, ifs, fip = self._create_server_get_interfaces()
+         original_interface_count = len(ifs)  # This is the number of ports.
+         self.assertGreater(original_interface_count, 0)
+         # Get the starting list of IPs on the server.
+@@ -369,6 +372,9 @@
+         self.assertEqual(1, len(addresses), addresses)  # number of networks
+         # Keep track of the original addresses so we can know which IP is new.
+         original_ips = [addr['addr'] for addr in list(addresses.values())[0]]
++        # Make sure the floating IP possibly assigned during
++        # server creation is always present in the set of original ips.
++        original_ips = set(original_ips).union(fip)
+         original_ip_count = len(original_ips)
+         self.assertGreater(original_ip_count, 0, addresses)  # at least 1
+         network_id = ifs[0]['net_id']
+@@ -376,40 +382,22 @@
+         # fixed IP on the same network (and same port since we only have one
+         # port).
+         self.servers_client.add_fixed_ip(server['id'], networkId=network_id)
+-        # Wait for the ips count to increase by one.
+-        def _get_server_floating_ips():
+-            _floating_ips = []
+-            _server = self.os_primary.servers_client.show_server(
+-                server['id'])['server']
+-            for _ip_set in _server['addresses']:
+-                for _ip in _server['addresses'][_ip_set]:
+-                    if _ip['OS-EXT-IPS:type'] == 'floating':
+-                        _floating_ips.append(_ip['addr'])
+-            return _floating_ips
+-
+-        def _wait_for_ip_increase():
++        def _wait_for_ip_change(expected_count):
+             _addresses = self.os_primary.servers_client.list_addresses(
+                 server['id'])['addresses']
+-            _ips = [addr['addr'] for addr in list(_addresses.values())[0]]
+-            LOG.debug("Wait for IP increase. All IPs still associated to "
++            _ips = set([addr['addr'] for addr in list(_addresses.values())[0]])
++            # Make sure possible floating ip is always present in the set.
++            _ips = _ips.union(fip)
++            LOG.debug("Wait for change of IPs. All IPs still associated to "
+                       "the server %(id)s: %(ips)s",
+                       {'id': server['id'], 'ips': _ips})
+-            if len(_ips) == original_ip_count + 1:
+-                return True
+-            elif len(_ips) == original_ip_count:
+-                return False
+-            # If not, lets remove any floating IP from the list and check again
+-            _fips = _get_server_floating_ips()
+-            _ips = [_ip for _ip in _ips if _ip not in _fips]
+-            LOG.debug("Wait for IP increase. Fixed IPs still associated to "
+-                      "the server %(id)s: %(ips)s",
+-                      {'id': server['id'], 'ips': _ips})
+-            return len(_ips) == original_ip_count + 1
++            return len(_ips) == expected_count
++        # Wait for the ips count to increase by one.
+         if not test_utils.call_until_true(
+-                _wait_for_ip_increase, CONF.compute.build_timeout,
+-                CONF.compute.build_interval):
++                _wait_for_ip_change, CONF.compute.build_timeout,
++                CONF.compute.build_interval, original_ip_count + 1):
+             raise lib_exc.TimeoutException(
+                 'Timed out while waiting for IP count to increase.')
+@@ -428,26 +416,8 @@
+                 break
+         self.servers_client.remove_fixed_ip(server['id'], address=fixed_ip)
+         # Wait for the interface count to decrease by one.
+-
+-        def _wait_for_ip_decrease():
+-            _addresses = self.os_primary.servers_client.list_addresses(
+-                server['id'])['addresses']
+-            _ips = [addr['addr'] for addr in list(_addresses.values())[0]]
+-            LOG.debug("Wait for IP decrease. All IPs still associated to "
+-                      "the server %(id)s: %(ips)s",
+-                      {'id': server['id'], 'ips': _ips})
+-            if len(_ips) == original_ip_count:
+-                return True
+-            # If not, lets remove any floating IP from the list and check again
+-            _fips = _get_server_floating_ips()
+-            _ips = [_ip for _ip in _ips if _ip not in _fips]
+-            LOG.debug("Wait for IP decrease. Fixed IPs still associated to "
+-                      "the server %(id)s: %(ips)s",
+-                      {'id': server['id'], 'ips': _ips})
+-            return len(_ips) == original_ip_count
+-
+         if not test_utils.call_until_true(
+-                _wait_for_ip_decrease, CONF.compute.build_timeout,
+-                CONF.compute.build_interval):
++                _wait_for_ip_change, CONF.compute.build_timeout,
++                CONF.compute.build_interval, original_ip_count):
+             raise lib_exc.TimeoutException(
+                 'Timed out while waiting for IP count to decrease.')