ahelal / kitchen-ansiblepush

test-kitchen plugin to use ansible in push mode
41 stars 22 forks source link

Incompatible with Ansible 2.x #12

Closed arielsalvo closed 1 year ago

arielsalvo commented 8 years ago

Hi,

I noticed that kitchen-ansiblepush will always state that idempotency tests are passed even when changes take place. This is because the API for callback plugins has changed and the current plugin no is longer loaded.

I managed to get it working in my fork (git@github.com:arielsalvo/kitchen-ansiblepush.git) branch Concurrency which also includes the changes in the pull request I opened earlier. Also needs to add "callback_whitelist = changes" to the "[default]" section of ansible.cfg.

I didn't open another PR for this because the solution breaks compatibility with Ansible 1.x.

Please, let me know if you need any help with this one.

Regards! --Ariel

ahelal commented 8 years ago

Ariel,

Thank you for your PR #11 I will review it ASAP.

I have been working on other projects and had no time to update for V2. But it s on my todo list. I did a trick with action plugins to make them include the correct file for the Ansible version hence compatibility for V1 and V2. I think it should also work for callbacks.

Please do open a PR for V2 callback and I would try to make it work for V1 and V2

arielsalvo commented 8 years ago

Hi!

I finally got it working with both versions and opened PR #14 (Ansible v2 compatible).

Let me know what you think

arielsalvo commented 8 years ago

Hi!

One final note before closing this issue:

I noticed you added an entry in your TODO list

Enable envirionment var ANSIBLE_CALLBACK_WHITELIST="changes" before call

I did consider it when I was working on PR #14 but decided against it. The reason was that, even though it is quite easy to do, the way Ansible handles the environment variable would override any setting in the ansible.cfg file.

lib/ansible/constants.py:236:DEFAULT_CALLBACK_WHITELIST     = get_config(p, DEFAULTS, 'callback_whitelist', 'ANSIBLE_CALLBACK_WHITELIST', [], islist=True)

That's why I decided to leave it to the user to add "changes" to either their environment variable or the ansible.cfg file. The only scenario where this would not affect other settings would be to ensure "changes" is part of an already existing environment variable. I wouldn't create the variable if it doesn't exist.

Hope it helps ;)

Beyond that, I think this issue can be closed.

Regards! --Ariel

ahelal commented 8 years ago

Ariel,

Thats clear to me. But the effect of overriding the callback during the kitchen run you will not have other callbacks. I am not sure what use case would require having another callback during test run ?

It can also be an optional thing. whats your thoughts ?

arielsalvo commented 8 years ago

Well.... I hadn't thought of making it optional but.... how about moving the setting to .kitchen.yml? I can't say no other callback will be needed in a test run and moving it to .kitchen.yml would provide the same flexibility.

Say, like this:

if ANSIBLE_CALLBACK_WHITELIST in env:
    check if "changes" is present and add if necessary
else if callback_whitelist in .kitchen.yml:
    set ANSIBLE_CALLBACK_WHITELIST = callback_whitelist
    check if "changes" is present and add if necessary
else
    set ANSIBLE_CALLBACK_WHITELIST = "changes"
end

This would ignore ansible.cfg completely because we could never know from kitchen if a value is set but would still allow setting the whitelist with the same flexibility. If you want to let the user handle it on his own via ansible.cfg, I would remove the catch-all "else" clause.

What do you think?

arielsalvo commented 1 year ago

No longer relevant