Bug 1819310

Felix,

Please forgive my ignorance, because I have not used Ansible ever, I'm dealing with the MIQ side of things and working my way back.  In your two pictures, was this input to Ansible without any integration with CloudForms?  And is the first picture the input to Ansible Tower, while the second is the output? Interpretted output?  Inquiring minds....

that's Tower 3.6.1 with Ansible 2.9.1

the problem is that this introduces a behavioural difference between tower and tower through miq.

And by the way - just because it happens to work - doesn't mean its *required*.  I'm looking at the Ansible documentation, and they refer to the YAML spec for what is valid.  The YAML spec says '\/' is not valid.  If it works on Ansible Tower, should we break the spec in MIQ so that they're the same?  I have my doubts.  Still waiting for your response.  Thanks much!

Hi Jerry,

thanks for your time dealing with this issue. I don't care what the yaml spec says. The yaml interpreter that is called inside of cloudforms/manageiq throws an error. This is absolutely ok and I don't expect you to change the yaml spec or anything like that.

The current situation is that the yaml interpreter that is used inside the cloudforms sync job throws an error and the ansible tower sync has a *hard* break. It never finishes. And every element that *could* have been imported because it conatins no wrong formatting at all is not imported because of this. The ansible sync is never finished. I expect you to acknowledge the fact that ansible seems to interpret yaml differently then cloudforms does. And I expect you, as a software developer, to properly catch the error and accomplish the task of syncing ansible tower with cloudforms in a best effort manner. This means that cloudforms catches the yaml error, finds out which part of ansible tower is broken (In my case it is a *single* job template of hundreds), write an appropriate warning message in the evm.log or whichever you like. Then you skip the broken part and carry on with the sync instead of just letting it crash.

Kind Regards,
Philipp

I progressed a little and https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html indicates that it isn't how you're supposed to escape /. you should be using '.

which leaves one aspect : is it feasable to have the refresh worker continue to the next playbook and continue sync, skipping import of the playbooks that do raise errors (keeping those errors in the logs for good measure)

Hi Drew,

Can you look into this issue?

Thanks,
Tina

Looks like dup of https://bugzilla.redhat.com/show_bug.cgi?id=1824846

(In reply to drew uhlmann from comment #22)
> Looks like dup of https://bugzilla.redhat.com/show_bug.cgi?id=1824846

that's the parent ticket but because it was headed into the handling of parsing instead of handling of replication I split it to this one for clarity...

Hi Felix,

I agree that it makes sense to keep both tickets because they describe different scenarios with the same root cause.

This merged back ported PR should resolve both issues:
https://github.com/ManageIQ/manageiq-providers-ansible_tower/pull/218

Thanks,
Tina

For verification it has to be noted that prior the fixed-in-version I have seen the provider not refreshed (no objects in CFME), though it was reported as refreshed. I don't like this behavior. I think what the CFME should do is to do some kind of "best effort" -- discover as many objects as possible but not report provider as refreshed when there was a problem.

Thanks Tina - and I agree with Jaroslav. sync what can be synced, show that the sync completed with error at the very least. I believe this is already how the rhev provider is being handled.

I spoke with the developers: Tina, Adam Grare and dmetzger. Tina said this in deed is a dup. So I am going to close this as a DUP. It should be fixed in same PR anyway.

The fact that providers may be not really be fully refreshed but report "Last refresh: Success Less Then A Minute Ago" could be resolved by some third state: "Completed with warnings" or so. I will fill a RFE for this on Github.

@Felix Dewaleyne Please clarify what did you mean by the "replication" in https://bugzilla.redhat.com/show_bug.cgi?id=1819310#c23. This is the only remaining piece which I don't understand.

replication was a term used by the customer I believe. My understanding is that they meant to have the playbook syncronized into cloudforms. 

does that help?

if you end up raising that with github, let me know the url of the post so I can share it with the customer.

Verified on 5.11.6

Reproducer
1)Add Tower to CFME
2)Make sure the Tower set up has a job template to which a variable with '\\' is being passed.

On 5.11.6 (with the fix) , I have verified the following after the Tower provider was added:
1)UI shows provider refresh has completed
2)There are no errors in evm.log
3)I'm able to see all the objects under the provider. I also see the job template in question being listed in the UI. The job template shouldn't get listed in the UI since it's not usable. Clicking on the template doesn't take you to the template details page. I'll report a BZ to address this issue.

I also see this warning logged in evm.log. This change was incorporated through https://bugzilla.redhat.com/show_bug.cgi?id=1824846
----] W, [2020-06-03T12:27:42.321964 #1073065:2b0930dc05b4] WARN -- : MIQ(ManageIQ::Providers::AnsibleTower::Inventory::Parser::AutomationManager#configuration_scripts) Failed to parse job_template ID [7]: <unknown>): found unknown escape character while parsing a quoted scalar at line 2 column 10

*** This bug has been marked as a duplicate of bug 1824846 ***