Need to add argument specs for all roles

[sean-m-sullivan]

As in discussion for ansible inclusion, we need to add meta/argument_spec.yml to all roles

Detailed here
https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_reuse_roles.html#role-argument-validation

discussion here:
ansible-community/community-topics#197 (comment)

[Tompage1994]

I've made a start on this. Will continue this afternoon and put a PR in

[sean-m-sullivan]

Awesome, thanks Tom!

[Tompage1994]

@sean-m-sullivan I'm realising now there's a problem with how I'm doing all this... I'll try to explain as best I can:

So I'm trying to do it all properly with describing all sub options of the variables (e.g. with ah_hoc_command_cancel below):

controller_ad_hoc_commands_cancel:
        description: Data structure describing your ad hoc jobs to cancel
        type: list
        elements: dict
        options:
          id:
            required: true
            type: int
            description: ID of the command to cancel
          fail_if_not_running:
            default: false
            required: false
            type: bool
            description: Fail loudly if the command can not be canceled.
          interval:
            default: 0
            required: false
            type: int
            description: The interval in seconds, to request an update from.
          timeout:
            default: 0
            required: false
            type: int
            description: Maximum time in seconds to wait for a job to finish, 0 means wait until it is finished regardless.

The problem here is that if you try to use the role with additional keys on any element of controller_ad_hoc_commands_cancel you get a failure of the argument spec check when the role is called.

This is apparent in the CI where we are getting a failure because we are collecting information from a previous task and using that to cancel the ad_hoc command. That's a pretty normal workflow I would say, so I think we need to allow throughout the usage of additional keys which are arbitrary in sub-options. As far as I can see there is no way to allow this though so I may have to revert to the below:

controller_ad_hoc_commands_cancel:
        description: Data structure describing your ad hoc jobs to cancel
        type: list
        elements: dict

and not describe any sub-options which I think is a shame as it would have been a great way to be documented as code.

So they options here are the following:

1. be strict with what options are allowed. We would then need to fix our CI to not contain any additional keys in the variables and this may be a bit of work
2. Keep the top level variables generic and just keep the documentation for the allowed options in the readme

I'll pause working on this for now until we get further.

Oh how I wish I knew this would have been an issue before I spent so long transcribing all these options

[branic]

I think option 1 would be the way to go. The variables passed in should only have the options that are supported. It's a bit more work to get the CI correct, but it's worth it.

[sean-m-sullivan]

if it is just in our CI, we can adapt to it, the whole reason to get the id from previous was to do a functional test, can we use another test for doing the arg specs

[Tompage1994]

Only issue with that is its technically a breaking change as if someone does have extra options in their dict (i.e. because they use it to do some other task as well, or they generate the object from some other source, then it will break their code because when the role is called it does an argument spec check. I'm not entirely sure that is the way to go since I can see a flow of grabbing IDs or suchlike from other sources as we do in the CI being a reasonably common use case

[sean-m-sullivan]

Not to add on things to your workfload Tom, I meant to get to this, but this arg spec is accross ALL of the collections, all 4, just for note, Its just we have discussion for the collection group in this repo.

[Tompage1994]

I know. Just wanted to focus on this one first so we had a pattern to follow