v2 API compatibility

[AlexanderZagaynov]

This PR allows to use Ansible Tower's v2 API, e.g. https://example.com/api/v2
It simply mimics old API's object fields to make new v2 objects compatible with v1.
It doesn't cover all the changes, only part of them, but it's enough to make https://github.com/ManageIQ/manageiq-providers-ansible_tower work.

[jameswnl]

@Fryguy @bdunne take a look at this. This will allow existing MIQ and the https://github.com/ManageIQ/manageiq-providers-ansible_tower to continue to work with Tower's v2 API

[jrafanie]

Interesting. I'll have to review more but we should probably gets the bug fixes #126 and #127 merged and released and then we can see about getting this merged. In terms of risk, I'd rather we get the less risky changes in first, then give us time to test this PR and master before needing to release.

It doesn't cover all the changes, only part of them,

We might even want to consider an api/v2 branch depending on how much more there is to do. Thoughts @AlexanderZagaynov @jameswnl

[AlexanderZagaynov]

@jrafanie here is some work in progress about pure new version from a scratch: https://github.com/AlexanderZagaynov/ansible_tower_client_ruby_v2
I've forwarded you emails with discussion about that.

[bronaghs]

hi @jrafanie
#126 and #127 have been merged and, I am told, released. Can this PR be merged?

[jrafanie]

Yes, those PRs were merged and released for ruby 2.5 compatibility. I'm not familiar with the v2 changes to feel confident either way. The only concern is if we need more changes on the last stable version and we need to backport a change, we might need to create a branch at a prior release to do that backport.

[mfeifer]

@jrafanie who can help with confidence level, here, so that this can move forward.

[jrafanie]

@mfeifer someone who's familiar with the ansible provider in both embedded and standalone form who knows how we represent the ansible objects via refresh and through operations... it's unclear if this works with all supported versions and what testing was done.

[mfeifer]

@jrafanie and do you have any people in mind? I don't.

[jrafanie]

Looking at the committers other than Brandon, maybe @syncrou or @jameswnl know more of the tower api side ... @agrare from a general providers perspective. It would also be hopeful to know what versions we need to support in master for ansible tower and if this PR was tested for these versions. Maybe @AlexanderZagaynov can update this PR with some information about that. Also, it's unclear if this is really important because v1 is going away or if it's a general enhancement, as there is no bugzilla or linked task to do this work. 🤷‍♂️

[AlexanderZagaynov]

Hi Joe, apologies, I didn't put a links in description.
Here is the BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1677553

We started that with @jameswnl, and he were curating my work there, he has all the knowledge, and I was expecting him to merge this PR.

Let me write some brief here:

• This change doesn't affects v1 interactions, everything will work as before (both embedded and tower).
• As I know, Ansible Tower going to drop support for v1 very soon.
• This change tries to get v1 attributes, and if they missing - takes their values from v2 objects if they available.
• User should point to v2 endpoint specially if wants to use it. Otherwise it works as before: if URL has host only - it uses v1 path (as far as I know, I didn't touch that, just saw somewhere in the code). If user enters URL with v2 path - we use v2.

[AlexanderZagaynov]

I tested everything manually, and @jameswnl is a witness here. Don't ask me to do it again please, it took a bunch of time already :)

[jameswnl]

hey, yeah, so like my previous comments, this is a pretty smart tweak to make the gem to work with v2. But my concern is that this may not be an obvious approach (or fragile). So I would like to hear from @Fryguy and @bdunne

Especially when @Fryguy is on his exploration of a longer term approach for Ansible integration

BTW, I don't have merge right for this repo.

[jrafanie]

If this just works, I wouldn't be opposed to getting it in but I'm not sure what a longer term goals are here and if the changes here would be ok for now.

[AlexanderZagaynov]

@jrafanie sorry, missed your reply. As I see it - long term goal is to rewrite this gem and refactor related MiQ code, I posted link above.

[AlexanderZagaynov]

@mfeifer looks like things haven't budged an inch? 😃

[mfeifer]

@Fryguy any thoughts on how to proceed with this one?

just in case you want to follow it: https://projects.engineering.redhat.com/browse/CFMEDEV-30

[bdunne]

Sorry for the delay, I was away for a while.

[Fryguy]

Code-wise LGTM...Not sure about sematics, so I'll defer to @bdunne or others.

[miq-bot]

Some comments on commits AlexanderZagaynov/ansible_tower_client_ruby@4e18ad1~...bd12dab

lib/ansible_tower_client/base_models/credential.rb

• 💣 💥 🔥 🚒 - 18 - Detected cloudforms

[miq-bot]

Checked commits AlexanderZagaynov/ansible_tower_client_ruby@4e18ad1~...bd12dab with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
7 files checked, 1 offense detected

lib/ansible_tower_client/base_models/project.rb

• ❗ - Line 26, Col 7 - Style/NegatedIf - Favor unless over if for negative conditions.

[bdunne]

Version 0.20.0 has been released https://rubygems.org/gems/ansible_tower_client/versions/0.20.0

[simaishi]

Is this safe to go to hammer branch?

We need #131 to be included in hammer and I was reviewing changes that went into this gem after 1.19.1 release (the version used in the last build). This PR talks about concerns around future backport, so I wanted to check.

cc @dmetzger57 @smallamp

[AlexanderZagaynov]

@simaishi yes, those changes are backwards compatible