Support restraint_id record in metadata and fix destroy action

[Tiboris]

Support restraint_id defined in metadata and later defined in inventory for the restraint jobs. For this we pass requirements all the way for later inventory generation.

Before:

#metadata 
domains:
- hosts:
  - group: ipaserver
    name: f32.mrack.test
    os: fedora-32
    role: master
  name: mrack.test

#mrack inventory
all:
  children:
    ipaserver:
      hosts:
        f32.mrack.test: {}
  hosts:
    f32.mrack.test:
       ansible_host: ec2.compute.amazonaws.com
      ansible_python_interpreter: /usr/bin/python3
      ansible_user: fedora
      meta_dc_record: DC=mrack,DC=test
      meta_domain: mrack.test
      meta_fqdn: f32.mrack.test
      meta_ip: 10.10.10.10
      meta_os: fedora-32
      meta_provider_id: i-0d92949afagag
      meta_role: master

After:

#metadata 
domains:
- hosts:
  - group: ipaserver
    name: f32.mrack.test
    os: fedora-32
    role: master
    restraint_id: 9 # <<<<<<<<<<<<<<
  name: mrack.test

#mrack inventory
all:
  children:
    ipaserver:
      hosts:
        f32.mrack.test: {}
  hosts:
    f32.mrack.test:
      ansible_host: ec2.compute.amazonaws.com
      ansible_python_interpreter: /usr/bin/python3
      ansible_user: fedora
      meta_dc_record: DC=mrack,DC=test
      meta_domain: mrack.test
      meta_fqdn: f32.mrack.test
      meta_ip: 10.10.10.10
      meta_os: fedora-32
      meta_provider_id: i-0d92949afagag
      meta_restraint_id: 9  # <<<<<<<<<<<<<<
      meta_role: master

Fix destroy action because delete host now needs only host.id

Because of delete_host method now needs only host.id,
the Destroy action was broken and it needed to use
the host.id variable not whole object as well.

before:

cannot pickle 'dict_values' object
Traceback (most recent call last):
...

after

Destroy done

Signed-off-by: Tibor Dudlák tdudlak@redhat.com

[Tiboris]

My concern is the fact this proposal returns reqs all around. Would it be better to add req params to methods ?

[netoarmando]

My concern is the fact this proposal returns reqs all around. Would it be better to add req params to methods ?

Do we need to store it in the database? Why not just access the value from metadata when generating the inventory? E.g: https://github.com/neoave/mrack/blob/c6f8394bdfe421f9275dfb4bd2b802e9edfc9b23/src/mrack/outputs/ansible_inventory.py#L116

[pvoborni]

It seems quite bad that we are forcing beaker specific thing (restraint_id) to all the other providers. This comes form the design of prov_result_to_host_data. As I was saying earlier in the refactoring PR. We should not have a method prov_result_to_host_data(prov_result_to_host_data) but only to_host(prov_result).

Atm it is not possible to generate outputs only from DB. All 3 sources are needed for that. So +1 that we should not put restraint_id to DB. DB should contain info from providers + info to be able to identify the host from metadata.

[Tiboris]

I believe the restraint_id is not beaker specific.

It is a mechanism to run restraint jobs on any of the provider we may support. In beaker job.xml definition of the recicipe the id value helps to assign host for the task. Or in other words if we would have more recipes in job definition we would be able to select via this id which restraint script will be run on the host. This is the document where the is mentioned https://restraint.readthedocs.io/en/latest/using.html hopefully i got it right.

I have implemented this because we found with @netoarmando that it is missing in specific jobs which fail. These jobs are using the restraint tests.

PS, i will update this to not store this in DB

[pvoborni]

I forgot, you are right, it is not beaker specific. But it's still good to not taint the providers and the DB with it.

What we can implement (later?) is a generic helper lib which can join the info from provisioning config, metadata and db. So that when we want some information, we don't need to implement this logic again.

[Tiboris]

I have added commit:

Fix destroy action because delete host now needs only host.id

Because of delete_host method now needs only host.id,
the Destroy action was broken and it needed to use
the host.id variable not whole object as well.