-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] ovirtlago: add generic waiters class #483
base: master
Are you sure you want to change the base?
Conversation
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 - Sure. I think the equivalent method is here:
Here, the
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): - 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 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: 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please split this pull request into two parts ?
- Adding "lago ovirt stop/start"
- Adding waiters.
[1] is more urgent for us, and I think that we need to discuss about [2], and check how we can utilize ovirt ansible module.
@gbenhaim - sure will do. |
9ad23fd
to
22b1a07
Compare
This new class creates a common infrastructure for tasks that require polling results periodically from the SDK. The methods in the 'waiters' class are generated from the data/waiters.yaml file. Each method defined in the YAML is in charge of polling the Engine for a status of a request. In this commit only 3 methods were configured in the YAML: waiters.vm_up(api, id) waiters.vm_down(api, id) waiters.host_up(api, id) Invoking each of these methods will check if the VM with the given id reached an 'accepting' state as defined in the YAML 'acceptors' section. It will run for 'max_attempts', with a 'delay' between and block until. The polling might end earlier, if the VM reached any of the 'rejectors' states as defined in the YAML, and then a 'WaiterError' exception will be thrown. Adding more 'waiters' requires only updating the YAML. Signed-off-by: Nadav Goldin <[email protected]>
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 thedata/waiters.yaml
file. This can minimize the repetitive task of (here and in OST):Down to:
And additionally, as these tasks are quite repetitive and similar: allow adding new 'waiters' by patching the YAML instead of the code.