sensu / sensu-ansible

An Ansible role to deploy a fully dynamic Sensu stack!
https://ansible-sensu.readthedocs.io
MIT License
126 stars 96 forks source link

Fix minor issues with plugins list, ssl cert copy and allow no auth for API and Uchiwa #136

Closed danragnar closed 6 years ago

danragnar commented 6 years ago

When running ansible 2.4, I got a value error for sensu_remote_plugins for comparing a list with an int, which to me indicates that this behaviour has changed in newer ansible versions. Not sure if this breaks earlier versions, but seems the length function isn't something new...

I also generate my certs on the servers via our FreeIPA CA, which worked fine for the client certs that uses the "sensu_ssl_deploy_remote_src" var, would be nice if the same behaviour was shown for the rabbitmq/server certs.

I run an Apache reverse proxy with kerberos auth in front of the API and Uchiwa, and rely totally on that and setting empty user/password does not work (still prompts for username/password), so I threw in a simple check for length check to remove those directives in the config files.

jaredledvina commented 6 years ago

Hey @danragnar - Thanks so much for the PR! It's very strange that you're hitting this with Ansible 2.4 as I've been unable to repro the issue. Clearly, I'm missing something here. Could you comment with what you have configured for:

sensu_remote_plugins

Also, could you update the CHANGELOG with an entry for the corresponding changes for me? Docs on how we changelog are here: https://github.com/sensu-plugins/community/blob/master/HOW_WE_CHANGELOG.md

danragnar commented 6 years ago

Ah, lack of testning on my part there. Ok, so probably have to change default to an empty list instead of '~'. I will have a look at it tomorrow morning.

Den 8 feb. 2018 23:53 skrev "Jared" notifications@github.com:

@jaredledvina commented on this pull request.

In tasks/plugins.yml https://github.com/sensu/sensu-ansible/pull/136#discussion_r167093120:

@@ -25,7 +25,7 @@ shell: umask 0022; sensu-install -p {{ item }} with_items: "{{ sensu_remote_plugins }}" changed_when: false

  • when: sensu_remote_plugins > 0
  • when: sensu_remote_plugins | length > 0

Womp, so the integration tests fail with this:

fatal: [localhost]: FAILED! => {"msg": "The conditional check 'sensu_remote_plugins | length > 0' failed. The error was: Unexpected templating type error occurred on ({% if sensu_remote_plugins | length > 0 %} True {% else %} False {% endif %}): object of type 'NoneType' has no len()

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sensu/sensu-ansible/pull/136#pullrequestreview-95263242, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad9O5TX9dq-C7-u09Kyqazmrg-yjP-uWks5tS3rsgaJpZM4R_H_s .

jaredledvina commented 6 years ago

@danragnar - Yeah, that might work as well, we can iterate on it and see what's the best setup. I'm going to check out locally if I can repro this as well, I feel like I'm just crazy and missing something.

danragnar commented 6 years ago

I'll provide some more info on it tomorrow. Thanks for the quick response , anyhow!

Den 9 feb. 2018 00:08 skrev "Jared" notifications@github.com:

@danragnar https://github.com/danragnar - Yeah, that might work as well, we can iterate on it and see what's the best setup. I'm going to check out locally if I can repro this as well, I feel like I'm just crazy and missing something.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sensu/sensu-ansible/pull/136#issuecomment-364280126, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad9O5bZ5UlI2jsXokENahls_r3g1ajb2ks5tS36BgaJpZM4R_H_s .

danragnar commented 6 years ago

Here's the error I'm receiving:

fatal: [REDACTED]: FAILED! => {
    "failed": true,
    "msg": "The conditional check 'sensu_remote_plugins > 0' failed. The error was: Unexpected templating type error occurred on ({% if sensu_remote_plugins > 0 %} True {% else %} False {% endif %}): '>' not supported between instances of 'list' and 'int'\n\nThe error appears to have been in '/home/dan/git/profiler/playbooks/galaxy_roles/cmacrae.sensu/tasks/plugins.yml': line 24, column 5, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n  - name: Ensure any remote plugins defined are present\n    ^ here\n"
}

Output of "ansible --version"

ansible 2.4.0.0
  config file = /home/dan/git/profiler/playbooks/ansible.cfg
  configured module search path = ['/home/dan/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/dan/.pyenv/versions/3.6.2/envs/profiler@3.6.2/lib/python3.6/site-packages/ansible
  executable location = /home/dan/.pyenv/versions/profiler@3.6.2/bin/ansible
  python version = 3.6.2 (default, Dec 12 2017, 18:18:13) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

Maybe this is a problem only when running 3.6, I'll try with a 2.X version and see if I get the same result.

jaredledvina commented 6 years ago

Ohhhh interesting, you might be the first Python 3.X Ansible user, I'm for sure using 2.X in the integration tests (and locally as well).

danragnar commented 6 years ago

Python 3.X (3.5+ specifically) is in "tech preview" for Ansible, and I presume it will be recommended sooner rather than later. We have run all our playbooks on 3.6.2 with no issues, and the only issue I had with this role was this list comparison. I have tested on 2.7 and that works as is. But the length comparison should work on both 3.X and 2.X. I'll push some changes soon and look at the changelog shortly.

jaredledvina commented 6 years ago

👍 Thanks @danragnar, this looks legit!

jaredledvina commented 6 years ago

Released in 2.1.0 - https://github.com/sensu/sensu-ansible/releases/tag/2.1.0 Thanks again!

danragnar commented 6 years ago

Thanks for your time and help!