jerakia / jerakia-ansible-lookup-plugin

An Ansible lookup plugin for integrating with Jerakia
Other
11 stars 9 forks source link

Errors with undefined scope variables. #2

Open crayfishx opened 6 years ago

crayfishx commented 6 years ago

User (GitHub handle unknown) Wrote by email:

One final question.

In the jerakia.yaml file used for Ansible under the scope section is there a mechanism to attempt relating multiple variables. What we ended up having in our configuration file was:

scope:
  hostname: ansible_nodename
  environment: ansible_local.custom.environment
  location: ansible_local.custom.location
  osfamily: ansible_os_family

What we found was that this ended up generating errors on systems that were missing local facts files that defined environment and location. Knowing this now we have adjusted the behavior of our playbooks but we were wondering if there was a supported mechanism to specifiy default values or other variables to use if one is not defined, i.e. something like the following:

scope:
  hostname: ansible_nodename
  environment: ansible_local.custom.environment|default('development')
  location: ansible_local.custom.location|default('deploying')
  osfamily: ansible_os_family

Or

scope:
  hostname: ansible_nodename
  environment: ansible_local.custom.environment, playbook_var_environment
  location: ansible_local.custom.location, playbook_var_location
  osfamily: ansible_os_family
crayfishx commented 6 years ago

This seems like a valid issue, I think we should address it. I think the OP has the right idea but I'd like to see it done with native YAML rather than inventing a new syntax like var|default('string')

Maybe making the value of the scope keys more flexible by allowing a hash or a string.

So either of these would work, the second uses a default.

# without default
scope:
  environment: ansible_local.custom.environment

# With default
scope:
  environment:
    var: ansible_local.custom.environment
    default: development

The above may be too complicated, I'm not sure, I've played around with the idea of an array because its shorter to express but how would we distinguish between a variable and a string? eg:

scope:
  environment: [ ansible_local.custom.environment, development ]

The above is shorter to express and assumes the first element of the array is a variable and the second is a hard coded string - this can be documented but is it obvious enough?

@beddari I think you're using this lookup plugin, any thoughts?

beddari commented 6 years ago

I think keeping scope configuation just key:value has value due to its simplicity.

beddari commented 6 years ago

Ok, the error you'll get if you use undefined variables in the scope config looks like this

fatal: [localhost]: FAILED! => {"msg": "An unhandled exception occurred while running the lookup plugin 'jerakia'. Error was a <class 'ansible.errors.AnsibleError'>, original message: Cannot find key ansible_does_not_exist "}

Not too friendly :) However, instead of trying to implement a mechanism providing defaults, does it not make more sense to just handle this case as it was an unset scope var/empty string?

scope:
  certname: ansible_does_not_exist
  environment: ansible_local.environment

- hosts: all
  tasks:
    - debug: msg="We're currently in the {{ ansible_local.environment }} env"
    - debug: msg="This node has certname {{ ansible_does_not_exist }}"
    - debug: msg="Do a lookup {{ lookup('jerakia', 'httpd/port') }}!"

If we run this specifying an empty value for ansible_does_not_exist it works as I'd expect

ansible-playbook playbook1.yaml -e ansible_does_not_exist=

ok: [localhost] => {
    "msg": "This node has certname "
}

and the lookup returns correct data.

Haven't looked at the full logic yet as the above ideally shouldn't even set that empty string in the API request. Will try to verify. And this shows that we need tests hehe.

beddari commented 6 years ago

I think we should classify this as a bug. The plan to fix it is currently to add debug output of all defined scope vars and values, and handle undefined vars the same as an empty string, if possible