lago icon indicating copy to clipboard operation
lago copied to clipboard

[WIP] ovirtlago: add generic waiters class

Open nvgoldin opened this issue 7 years ago • 3 comments

The first commit adds a waiters.py class. The class goal is to set common infrastructure for tasks that require to poll the SDK periodically for a status of a request. It generates the 'waiters' methods from the data/waiters.yaml file. This can minimize the repetitive task of (here and in OST):

def some_test(...):
    ...
    def _is_host_up():
      .....
    testlib.assert_true_within(_is_host_up, timeout=...)

Down to:

   def some_test(...):
       ...
       waiters.host_up(id=id)

And additionally, as these tasks are quite repetitive and similar: allow adding new 'waiters' by patching the YAML instead of the code.

nvgoldin avatar Mar 20 '17 00:03 nvgoldin

Can you check how our Ansible module is doing it? Let's try to share code. See https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/ovirt.py for example.

mykaul avatar Mar 20 '17 08:03 mykaul

@mykaul - Sure.

I think the equivalent method is here: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/ovirt.py#L298-L330 few differences I see: In the Ansible module the wait method is invoked directly with the target parameters, as done in: https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/ovirt/ovirt_hosts.py#L299-L303

...
        wait(
            service=host_service,
            condition=lambda host: host.status == hoststate.UP,
            fail_condition=failed_state,
        )
...

Here, the waiters class is a little more 'high level', as the methods are auto-generated with the parameters, which is less flexible when invoking, but simpler:

   waiters.host_up(id)

The Ansible code is written to integrate with the ansible tasks, so eventually at the higher level adding a host and pooling bundled together looks like this(taken from OST): https://github.com/oVirt/ovirt-system-tests/blob/master/ansible-suite-master/ovirt-deploy/tasks/bootstrap.yml#L48-L60

- name: Add hosts to oVirt engine
  ovirt_hosts:
    auth: "{{ ovirt_auth }}"
    name: "{{ item.0 }}"
    address: "{{ item.1 }}"
    password: "{{ host_password }}"
    cluster: "{{ cluster_name }}"
    override_iptables: true
    timeout: 300
    poll_interval: 10
  with_together:
    - "{{ host_names}}"
    - "{{ host_ips }}"

Which is quite nice and explains why the wait method was built like that.

What I'm not sure at the moment is how we can share the code between the modules, without using Ansible directly. Maybe we should just go there?

If using Ansible directly, the difficulty might be in OST.. As new tests could require more fine-grained control than what the Ansible module provides.

I should say that the inspiration for 'waiters' I took from 'boto'(AWS official SDK), for example: http://boto3.readthedocs.io/en/latest/reference/services/ec2.html?highlight=waiters#EC2.Waiter.InstanceRunning

In parallel to each 'Action', like we have in the SDK, they provide out-of-the-box 'waiters' methods, which are implemented in the client side.

nvgoldin avatar Mar 20 '17 09:03 nvgoldin

@gbenhaim - sure will do.

nvgoldin avatar Mar 20 '17 09:03 nvgoldin