[PATCH v1 0/4] Extension of migration tests

[PATCH v1 0/4] Extension of migration tests

[Oksana Vohchana]

This series adds a new migration test through RDMA and provides new
functions to it.
The last update by mistake was not provided a full scenario to the EXEC
migration test. One of patch fixed it.

Oksana Vohchana (4):
  Acceptance test: add address as param
  Acceptance test: EXEC migration
  Acceptance test: provides new functions
  Acceptance test: provides to use RDMA transport for migration

 tests/acceptance/migration.py | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

-- 
2.21.1

[PATCH v1 1/4] Acceptance test: add address as param

[Oksana Vohchana]

Provides param address in _get_free_port()
because by default it takes free port only on the localhost

Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
---
 tests/acceptance/migration.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index a8367ca023..e4c39b85a1 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -52,8 +52,8 @@ class Migration(Test):
         source_vm.qmp('migrate', uri=src_uri)
         self.assert_migration(source_vm, dest_vm)
 
-    def _get_free_port(self):
-        port = network.find_free_port()
+    def _get_free_port(self, address='localhost'):
+        port = network.find_free_port(address=address)
         if port is None:
             self.cancel('Failed to find a free port')
         return port
-- 
2.21.1

Re: [PATCH v1 1/4] Acceptance test: add address as param

[Wainer dos Santos Moschetta]

Hi Oksana,

On 2/14/20 12:52 PM, Oksana Vohchana wrote:
> Provides param address in _get_free_port()
> because by default it takes free port only on the localhost
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> ---
>   tests/acceptance/migration.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index a8367ca023..e4c39b85a1 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -52,8 +52,8 @@ class Migration(Test):
>           source_vm.qmp('migrate', uri=src_uri)
>           self.assert_migration(source_vm, dest_vm)
>   
> -    def _get_free_port(self):
> -        port = network.find_free_port()
> +    def _get_free_port(self, address='localhost'):
> +        port = network.find_free_port(address=address)

Looks good to me, but:

- This patch is out of order, it should be followed by patches 03 and 04.

- You could have described in the cover letter (or in the commit) your 
intention with this change.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

Thanks!

- Wainer

>           if port is None:
>               self.cancel('Failed to find a free port')
>           return port

[PATCH v1 2/4] Acceptance test: EXEC migration

[Oksana Vohchana]

Improves EXEC migration to run whole test stage

Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
---
 tests/acceptance/migration.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index e4c39b85a1..8209dcf71d 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -75,3 +75,5 @@ class Migration(Test):
         """
         free_port = self._get_free_port()
         dest_uri = 'exec:nc -l localhost %u' % free_port
+        src_uri = 'exec:nc localhost %u' % free_port
+        self.do_migrate(dest_uri, src_uri)
-- 
2.21.1

Re: [PATCH v1 2/4] Acceptance test: EXEC migration

[Philippe Mathieu-Daudé]

On 2/14/20 3:52 PM, Oksana Vohchana wrote:
> Improves EXEC migration to run whole test stage
> 
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>

Fixes: 2e768cb682bf

> ---
>   tests/acceptance/migration.py | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index e4c39b85a1..8209dcf71d 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -75,3 +75,5 @@ class Migration(Test):
>           """
>           free_port = self._get_free_port()
>           dest_uri = 'exec:nc -l localhost %u' % free_port
> +        src_uri = 'exec:nc localhost %u' % free_port
> +        self.do_migrate(dest_uri, src_uri)
> 

I was very surprised because commit 2e768cb682bf is not the content I 
reviewed. Apparently what happened here is you sent 2 different series 
with the same subject... I got confused while applying I selected the 
incorrect id:

$ pwclient list 'Acceptance test: provides to use different transport 
for migration'
ID      State        Name
--      -----        ----
11348377 New          Acceptance test: provides to use different 
transport for migration
11348719 New          Acceptance test: provides to use different 
transport for migration
11355797 New          [v2] Acceptance test: provides to use different 
transport for migration
11358447 New          [v2,REPOST] Acceptance test: provides to use 
different transport for migration
11362397 New          [v3,1/2] Acceptance test: provides to use 
different transport for migration
11362399 New          [v3,2/2] Acceptance test: provides to use 
different transport for migration
11362479 New          [v3,1/2] Acceptance test: provides to use 
different transport for migration
11362485 New          [v3,2/2] Acceptance test: provides to use 
different transport for migration

I am sorry I missed that.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v1 2/4] Acceptance test: EXEC migration

[Wainer dos Santos Moschetta]

On 2/14/20 12:52 PM, Oksana Vohchana wrote:
> Improves EXEC migration to run whole test stage
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> ---
>   tests/acceptance/migration.py | 2 ++
>   1 file changed, 2 insertions(+)

Indeed, with this changes the migration is triggered.

Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index e4c39b85a1..8209dcf71d 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -75,3 +75,5 @@ class Migration(Test):
>           """
>           free_port = self._get_free_port()
>           dest_uri = 'exec:nc -l localhost %u' % free_port
> +        src_uri = 'exec:nc localhost %u' % free_port
> +        self.do_migrate(dest_uri, src_uri)

[PATCH v1 3/4] Acceptance test: provides new functions

[Oksana Vohchana]

Adds functions to check if service RDMA is enabled and gets the interface
where it was configured

Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
---
 tests/acceptance/migration.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 8209dcf71d..bbd88f8dda 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -11,12 +11,16 @@
 
 
 import tempfile
+import re
+import netifaces
 from avocado_qemu import Test
 from avocado import skipUnless
 
 from avocado.utils import network
 from avocado.utils import wait
 from avocado.utils.path import find_command
+from avocado.utils import service
+from avocado.utils import process
 
 
 class Migration(Test):
@@ -58,6 +62,19 @@ class Migration(Test):
             self.cancel('Failed to find a free port')
         return port
 
+    def _if_rdma_enable(self):
+        rdma_stat = service.ServiceManager()
+        rdma = rdma_stat.status('rdma')
+        return rdma
+
+    def _get_ip_rdma(self):
+        get_ip_rdma = process.run('rdma link show').stdout.decode()
+        for line in get_ip_rdma.split('\n'):
+            if re.search(r"ACTIVE", line):
+                interface = line.split(" ")[-2]
+                ip = netifaces.ifaddresses(interface)[netifaces.AF_INET][0]['addr']
+                return ip
+
 
     def test_migration_with_tcp_localhost(self):
         dest_uri = 'tcp:localhost:%u' % self._get_free_port()
-- 
2.21.1

Re: [PATCH v1 3/4] Acceptance test: provides new functions

[Wainer dos Santos Moschetta]

Hi Oksana,

On 2/14/20 12:52 PM, Oksana Vohchana wrote:
> Adds functions to check if service RDMA is enabled and gets the interface
> where it was configured
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> ---
>   tests/acceptance/migration.py | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 8209dcf71d..bbd88f8dda 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -11,12 +11,16 @@
>   
>   
>   import tempfile
> +import re
> +import netifaces

Since netifaces isn't a standard Python library that import might fail.

The tests dependencies are listed in tests/requirements.txt, and 
installed in the environment created by `make check-acceptance`. If you 
want to ensure the test behaves well even when executed manually (i.e. 
not via `make check-acceptance`), you can add runtime checks as can be 
seen in tests/acceptance/machine_m68k_nextcube.py

>   from avocado_qemu import Test
>   from avocado import skipUnless
>   
>   from avocado.utils import network
>   from avocado.utils import wait
>   from avocado.utils.path import find_command
> +from avocado.utils import service
> +from avocado.utils import process
>   
>   
>   class Migration(Test):
> @@ -58,6 +62,19 @@ class Migration(Test):
>               self.cancel('Failed to find a free port')
>           return port
>   
> +    def _if_rdma_enable(self):
> +        rdma_stat = service.ServiceManager()
> +        rdma = rdma_stat.status('rdma')
> +        return rdma


Above function is used on patch04, but actually I don't think it needs 
to check this service for RoCE. It would be needed if it was using the 
rxe_cfg to configure the rdma link. Or am I missing something?


> +
> +    def _get_ip_rdma(self):
> +        get_ip_rdma = process.run('rdma link show').stdout.decode()
> +        for line in get_ip_rdma.split('\n'):
> +            if re.search(r"ACTIVE", line):
> +                interface = line.split(" ")[-2]
> +                ip = netifaces.ifaddresses(interface)[netifaces.AF_INET][0]['addr']
> +                return ip
> +

I suggest that it explicitly returns None if none is found.

Thanks!

- Wainer

>   
>       def test_migration_with_tcp_localhost(self):
>           dest_uri = 'tcp:localhost:%u' % self._get_free_port()

Re: [PATCH v1 3/4] Acceptance test: provides new functions

[Oksana Voshchana]

[-- Attachment #1: Type: text/plain, Size: 3011 bytes --]

Hi Wainer,
Thanks for review

On Fri, Feb 21, 2020 at 8:31 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

> Hi Oksana,
>
> On 2/14/20 12:52 PM, Oksana Vohchana wrote:
> > Adds functions to check if service RDMA is enabled and gets the interface
> > where it was configured
> >
> > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> > ---
> >   tests/acceptance/migration.py | 17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/tests/acceptance/migration.py
> b/tests/acceptance/migration.py
> > index 8209dcf71d..bbd88f8dda 100644
> > --- a/tests/acceptance/migration.py
> > +++ b/tests/acceptance/migration.py
> > @@ -11,12 +11,16 @@
> >
> >
> >   import tempfile
> > +import re
> > +import netifaces
>
> Since netifaces isn't a standard Python library that import might fail.
>
> The tests dependencies are listed in tests/requirements.txt, and
> installed in the environment created by `make check-acceptance`. If you
> want to ensure the test behaves well even when executed manually (i.e.
> not via `make check-acceptance`), you can add runtime checks as can be
> seen in tests/acceptance/machine_m68k_nextcube.py
>

Thanks, a "runtime checks" is a good approach
I'll improve it


>
> >   from avocado_qemu import Test
> >   from avocado import skipUnless
> >
> >   from avocado.utils import network
> >   from avocado.utils import wait
> >   from avocado.utils.path import find_command
> > +from avocado.utils import service
> > +from avocado.utils import process
> >
> >
> >   class Migration(Test):
> > @@ -58,6 +62,19 @@ class Migration(Test):
> >               self.cancel('Failed to find a free port')
> >           return port
> >
> > +    def _if_rdma_enable(self):
> > +        rdma_stat = service.ServiceManager()
> > +        rdma = rdma_stat.status('rdma')
> > +        return rdma
>
>
> Above function is used on patch04, but actually I don't think it needs
> to check this service for RoCE. It would be needed if it was using the
> rxe_cfg to configure the rdma link. Or am I missing something?
>
>  The function _if_rdma_enable() checks if RDMA service enabled in the
system it does not depend on wich utils we will use to check if some
network configuration present (rdma or rxe_cfg)


> > +
> > +    def _get_ip_rdma(self):
> > +        get_ip_rdma = process.run('rdma link show').stdout.decode()
> > +        for line in get_ip_rdma.split('\n'):
> > +            if re.search(r"ACTIVE", line):
> > +                interface = line.split(" ")[-2]
> > +                ip =
> netifaces.ifaddresses(interface)[netifaces.AF_INET][0]['addr']
> > +                return ip
> > +
>
> I suggest that it explicitly returns None if none is found.
>

Ok, I agree
I'll improve it


> Thanks!
>
> - Wainer
>
> >
> >       def test_migration_with_tcp_localhost(self):
> >           dest_uri = 'tcp:localhost:%u' % self._get_free_port()
>
>

Thanks!

[-- Attachment #2: Type: text/html, Size: 4473 bytes --]

[PATCH v1 4/4] Acceptance test: provides to use RDMA transport for migration

[Oksana Vohchana]

Adds test for RDMA migration check

Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
---
 tests/acceptance/migration.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index bbd88f8dda..c0a3031e67 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -94,3 +94,11 @@ class Migration(Test):
         dest_uri = 'exec:nc -l localhost %u' % free_port
         src_uri = 'exec:nc localhost %u' % free_port
         self.do_migrate(dest_uri, src_uri)
+
+    @skipUnless(_if_rdma_enable(None), "Unit rdma.service could not be found")
+    @skipUnless(_get_ip_rdma(None), 'RoCE(RDMA) service or interface not configured')
+    def test_migration_with_rdma_localhost(self):
+        ip = self._get_ip_rdma()
+        free_port = self._get_free_port(address=ip)
+        dest_uri = 'rdma:%s:%u' % (ip, free_port)
+        self.do_migrate(dest_uri)
-- 
2.21.1