[PATCH v3 0/3] Acceptance test: Extension of migration tests

[PATCH v3 0/3] Acceptance test: Extension of migration tests

[Oksana Vohchana]

This series adds a new migration test through RDMA.
To correct uses of migration need to add a new function to work
with RDMA service.
And as a part of migration tests, the series makes small updates to EXEC
migration and to _get_free_port function

V2:
 - improves commit message in Acceptance test: adds param 'address'
   in _get_free_port
 - provides import check for netifaces library
 - makes fix to _get_ip_rdma function
 - adds skip to test if not upload python module

V3:
 - removes unrelated changes
 - updates functions with new avocado features

Oksana Vohchana (3):
  Acceptance test: adds param 'address' in _get_free_port
  Acceptance test: provides new functions
  Acceptance test: provides to use RDMA transport for migration test

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

-- 
2.21.1

[PATCH v3 1/3] Acceptance test: adds param 'address' in _get_free_port

[Oksana Vohchana]

In the migration test function _get_free_port works only for localhost,
but in the case to use migration through an RDMA we need to get a free port
on the configured network RDMA-interface.
This patch is the start for another migration option

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

[PATCH v3 2/3] Acceptance test: provides new functions

[Oksana Vohchana]

Provides new functions related to the rdma migration test
Adds functions to check if service RDMA is enabled and gets
the ip address on the interface where it was configured

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

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index e4c39b85a1..a783f3915b 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -11,12 +11,17 @@
 
 
 import tempfile
+import json
 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.network.interfaces import NetworkInterface
+from avocado.utils.network.hosts import LocalHost
+from avocado.utils import service
+from avocado.utils import process
 
 
 class Migration(Test):
@@ -58,6 +63,31 @@ 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_interface_rdma(self):
+        cmd = 'rdma link show -j'
+        out = json.loads(process.getoutput(cmd))
+        try:
+            for i in out:
+                if i['state'] == 'ACTIVE':
+                    return i['netdev']
+        except KeyError:
+            return None
+
+    def _get_ip_rdma(self, interface):
+        local = LocalHost()
+        network_in = NetworkInterface(interface, local)
+        try:
+            ip = network_in._get_interface_details()
+            if ip:
+                return ip[0]['addr_info'][0]['local']
+        except:
+            self.cancel("Incorrect interface configuration or device name")
+
 
     def test_migration_with_tcp_localhost(self):
         dest_uri = 'tcp:localhost:%u' % self._get_free_port()
-- 
2.21.1

[PATCH v3 3/3] Acceptance test: provides to use RDMA transport for migration test

[Oksana Vohchana]

Adds test for RDMA migration check

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

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index a783f3915b..c8673114a9 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -105,3 +105,15 @@ class Migration(Test):
         """
         free_port = self._get_free_port()
         dest_uri = 'exec:nc -l localhost %u' % free_port
+
+    @skipUnless(_if_rdma_enable(None), "Unit rdma.service could not be found")
+    @skipUnless(_get_interface_rdma(None), 'RDMA service or interface not configured')
+    def test_migration_with_rdma_localhost(self):
+        iface = self._get_interface_rdma()
+        ip = self._get_ip_rdma(iface)
+        if ip:
+            free_port = self._get_free_port(address=ip)
+        else:
+            self.cancel("Ip address isn't configured")
+        dest_uri = 'rdma:%s:%u' % (ip, free_port)
+        self.do_migrate(dest_uri)
-- 
2.21.1

Re: [PATCH v3 1/3] Acceptance test: adds param 'address' in _get_free_port

[Willian Rampazzo]

On Fri, Mar 20, 2020 at 12:15 PM Oksana Vohchana <ovoshcha@redhat.com> wrote:
>
> In the migration test function _get_free_port works only for localhost,
> but in the case to use migration through an RDMA we need to get a free port
> on the configured network RDMA-interface.
> This patch is the start for another migration option
>
> 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
>
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH v3 2/3] Acceptance test: provides new functions

[Willian Rampazzo]

Hi Oksana,

On Fri, Mar 20, 2020 at 12:15 PM Oksana Vohchana <ovoshcha@redhat.com> wrote:
>
> Provides new functions related to the rdma migration test
> Adds functions to check if service RDMA is enabled and gets
> the ip address on the interface where it was configured
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> ---
>  tests/acceptance/migration.py | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index e4c39b85a1..a783f3915b 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -11,12 +11,17 @@
>
>
>  import tempfile
> +import json
>  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.network.interfaces import NetworkInterface
> +from avocado.utils.network.hosts import LocalHost
> +from avocado.utils import service
> +from avocado.utils import process
>
>
>  class Migration(Test):
> @@ -58,6 +63,31 @@ 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

You can just `return rdma_stat.status('rdma')` here! Also, as you are
not using any of the class attributes or methods, if you make this
method static, you don't need to call it with `None` as you did on
patch 3 of this series.

> +
> +    def _get_interface_rdma(self):
> +        cmd = 'rdma link show -j'
> +        out = json.loads(process.getoutput(cmd))
> +        try:
> +            for i in out:
> +                if i['state'] == 'ACTIVE':
> +                    return i['netdev']
> +        except KeyError:
> +            return None

Same comment about making this method static.

Actually, if you are not using any of the attributes or methods from
the Migration class on these two methods, I think you can define them
as functions, outside of the Class. Does it make sense?

> +
> +    def _get_ip_rdma(self, interface):
> +        local = LocalHost()
> +        network_in = NetworkInterface(interface, local)
> +        try:
> +            ip = network_in._get_interface_details()
> +            if ip:
> +                return ip[0]['addr_info'][0]['local']
> +        except:
> +            self.cancel("Incorrect interface configuration or device name")
> +

If you change the logic a bit and raise an exception here, instead of
doing a `self.cancel`, you can also make this method static, or move
it outside of the class. The cancel can be handled in the test, with
the exception raised here.

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

Let me know if the comments do not make sense.

Willian

Re: [PATCH v3 3/3] Acceptance test: provides to use RDMA transport for migration test

[Willian Rampazzo]

Hi Oksana,

On Fri, Mar 20, 2020 at 12:16 PM Oksana Vohchana <ovoshcha@redhat.com> wrote:
>
> Adds test for RDMA migration check
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> ---
>  tests/acceptance/migration.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index a783f3915b..c8673114a9 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -105,3 +105,15 @@ class Migration(Test):
>          """
>          free_port = self._get_free_port()
>          dest_uri = 'exec:nc -l localhost %u' % free_port
> +
> +    @skipUnless(_if_rdma_enable(None), "Unit rdma.service could not be found")
> +    @skipUnless(_get_interface_rdma(None), 'RDMA service or interface not configured')

If you change these two methods to be static, you will not need to use
the `None` parameter, as I mentioned in patch 2 of this series.

> +    def test_migration_with_rdma_localhost(self):
> +        iface = self._get_interface_rdma()
> +        ip = self._get_ip_rdma(iface)
> +        if ip:
> +            free_port = self._get_free_port(address=ip)
> +        else:
> +            self.cancel("Ip address isn't configured")
> +        dest_uri = 'rdma:%s:%u' % (ip, free_port)
> +        self.do_migrate(dest_uri)
> --
> 2.21.1
>
>

Re: [PATCH v3 2/3] Acceptance test: provides new functions

[Oksana Voshchana]

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

Hi Willian
I totally agree with your comments
I'll move those functions outside the class
Let me update and repost the patch
Thanks

On Fri, Mar 20, 2020 at 6:22 PM Willian Rampazzo <wrampazz@redhat.com>
wrote:

> Hi Oksana,
>
> On Fri, Mar 20, 2020 at 12:15 PM Oksana Vohchana <ovoshcha@redhat.com>
> wrote:
> >
> > Provides new functions related to the rdma migration test
> > Adds functions to check if service RDMA is enabled and gets
> > the ip address on the interface where it was configured
> >
> > Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
> > ---
> >  tests/acceptance/migration.py | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/tests/acceptance/migration.py
> b/tests/acceptance/migration.py
> > index e4c39b85a1..a783f3915b 100644
> > --- a/tests/acceptance/migration.py
> > +++ b/tests/acceptance/migration.py
> > @@ -11,12 +11,17 @@
> >
> >
> >  import tempfile
> > +import json
> >  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.network.interfaces import NetworkInterface
> > +from avocado.utils.network.hosts import LocalHost
> > +from avocado.utils import service
> > +from avocado.utils import process
> >
> >
> >  class Migration(Test):
> > @@ -58,6 +63,31 @@ 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
>
> You can just `return rdma_stat.status('rdma')` here! Also, as you are
> not using any of the class attributes or methods, if you make this
> method static, you don't need to call it with `None` as you did on
> patch 3 of this series.
>
> > +
> > +    def _get_interface_rdma(self):
> > +        cmd = 'rdma link show -j'
> > +        out = json.loads(process.getoutput(cmd))
> > +        try:
> > +            for i in out:
> > +                if i['state'] == 'ACTIVE':
> > +                    return i['netdev']
> > +        except KeyError:
> > +            return None
>
> Same comment about making this method static.
>
> Actually, if you are not using any of the attributes or methods from
> the Migration class on these two methods, I think you can define them
> as functions, outside of the Class. Does it make sense?
>
> > +
> > +    def _get_ip_rdma(self, interface):
> > +        local = LocalHost()
> > +        network_in = NetworkInterface(interface, local)
> > +        try:
> > +            ip = network_in._get_interface_details()
> > +            if ip:
> > +                return ip[0]['addr_info'][0]['local']
> > +        except:
> > +            self.cancel("Incorrect interface configuration or device
> name")
> > +
>
> If you change the logic a bit and raise an exception here, instead of
> doing a `self.cancel`, you can also make this method static, or move
> it outside of the class. The cancel can be handled in the test, with
> the exception raised here.
>
> >
> >      def test_migration_with_tcp_localhost(self):
> >          dest_uri = 'tcp:localhost:%u' % self._get_free_port()
> > --
> > 2.21.1
> >
> >
>
> Let me know if the comments do not make sense.
>
> Willian
>
>

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