theforeman / foreman-ansible-modules

Ansible modules for interacting with the Foreman API and various plugin APIs such as Katello
GNU General Public License v3.0
147 stars 163 forks source link

RFC: "automated" module generation #324

Closed evgeni closed 4 years ago

evgeni commented 5 years ago
SUMMARY

While reviewing #318 and working on slides for an upcoming talk, I noticed that our modules mostly follow the same structure:

  1. write a name_map to map Ansible params to Foreman API params
  2. create a ForemanAnsibleModule instance that receives a argument_spec that duplicates some data from name_map
  3. create an entity_dict from the Ansible params
  4. module.connect()
  5. entity = module.find_resource_by_name(…)
  6. enrich entity_dict with data if we need to reference other entities
  7. changed = module.ensure_resource_state(…)
  8. module.exit_json(changed=changed)

My crazy idea was to merge the data of name_map and argument_spec into a single dict (that can be later processed by ForemanAnsibleModule) and make the module look like this:

foreman_spec = { name_map+argument_spec }
module = ForemanAnsibleModule(resource='resource', foreman_spec=foreman_spec)
module.run()

And module.run() would be a wrapper like this:

def run(self):
  entity_dict = self.clean_params()
  self.connect()
  entity = self.find_resource_by_name(self.resource, …)`
  for additional_resource in self.foreman_spec:
    entity_dict[additional_resource['name']] = find…
  changed = self.ensure_resource_state(…)
  self.exit_json(changed=changed)

This surely won't work for some more complicated modules we have, but for those that "just" ensure entities, this should work.

ISSUE TYPE
ekohl commented 5 years ago

I think the name_map can still be a separate parameter but I like the idea. We also have very common mappings like locations -> location and organizations -> organization. Perhaps it should automatically handle those? I also wonder why we have mappings like name -> name. That sounds like a noop to me.

How do you see modules which can't do this simple run? Would they override the run method?

mdellweg commented 5 years ago

@ekohl I know, but a mapping name: name still serves as a filter. Every other parameter will be removed. Also we had the discussion earlier, and my answer was: "Consistency." I think, we gain something, if those modules look as similar as possible. (At least something to refactor away from them.)

mdellweg commented 5 years ago

I started some work on the state and name_map parameters here: #337

evgeni commented 4 years ago

With #610 merged, I as the author of the original issue consider this done.