Successfully test collection of event_query.yml data
#15761
Conversation
awx/main/tasks/jobs.py
Outdated
| @@ -948,6 +945,10 @@ def build_env(self, job, private_data_dir, private_data_files=None): | |||
| paths = [os.path.join(CONTAINER_ROOT, folder)] + paths | |||
| env[env_key] = os.pathsep.join(paths) | |||
|
|
|||
| env['ANSIBLE_CALLBACKS_ENABLED'] = 'indirect_instance_count' | |||
| if 'callbacks_enabled' in config_values: | |||
| env['ANSIBLE_CALLBACKS_ENABLED'] += ':' + config_values['callbacks_enabled'] | |||
There was a problem hiding this comment.
@jessicamack around here, in particular, we would like to check the feature flag to tell if the indirect host counting is enabled. How do I do that?
There was a problem hiding this comment.
awx/main/tasks/jobs.py
Outdated
| with open(data_file_path) as f: | ||
| collected_data = json.loads(f.read()) | ||
|
|
||
| instance.installed_collections = collected_data['installed_collections'] |
There was a problem hiding this comment.
This could throw a ValueError if the plugin returns invalid or out-of-spec json.
|
|
||
| if os.path.exists(collections_info): | ||
| with open(collections_info) as ee_json_info: | ||
| ee_collections_info = json.loads(ee_json_info.read()) |
There was a problem hiding this comment.
So this mechanism already exists? Or was it unused so far?
There was a problem hiding this comment.
It existed, but was broken with a particular ansible-runner version.
The design problem is that we can't honestly collect the event_query data without also indirectly collecting the collection list (which is what this did, but is broken). So it would be awfully neglectful to throw in the new loading logic right next to the old, broken, artifact file loading which is the same in form.
We still have a software coordination problem, which is that supposed there is a shared problem which will lead to ansible-runner developing a new solution, and that the collection listing this uses is a non-public API... but categorically, if there is no public API for the collection list, this sub-collection detail of the event_query can't be gathered in a kosher manner.
Let me jump to the conclusion - we finish up this approach and call it the "beta" version. It will be fully functional, but use non-public APIs. Then, we will have a TODO when the public API (new callback methods) are delivered from ansible-runner. We may need to have an import test / fallback to switch between the "beta" behavior and the accepted solution.
| def v2_playbook_on_stats(self, stats): | ||
| artifact_dir = os.getenv('AWX_ISOLATED_DATA_DIR') | ||
| if not artifact_dir: | ||
| raise RuntimeError('Only suitable in AWX, did not find private_data_dir') |
There was a problem hiding this comment.
what does Only suitable in AWX mean here? Wouldn't this plugin be only suitable for AAP instead?
There was a problem hiding this comment.
We're writing a plugin that is usable generally in an ansible-playbook run. This error should be seen if someone ever does that (enable the plugin, use in ansible-playbook) outside of a AWX/controller job.
| from ansible.cli.galaxy import with_collection_artifacts_manager | ||
| from ansible.release import __version__ | ||
|
|
||
| from ansible.galaxy.collection import find_existing_collections |
There was a problem hiding this comment.
TODO: will have to wrap this in a try-except, not currently a public API. Do simple warning of some type if it doesn't work.
AlanCoding
changed the title
event_query.yml dataevent_query.yml data
Get tests passing Mild rebranding Put behind feature flag, flip true in dev Add noqa flag
AlanCoding
force-pushed
the
callback_hosts
branch
from
9ce0327 to
2df90c1
Compare
|
AlanCoding
merged commit 6fc3461
into
ansible:indirect-host-counting
* Callback plugin method from cmeyers adapted to global collection list Get tests passing Mild rebranding Put behind feature flag, flip true in dev Add noqa flag * Add missing wait_for_events
* Callback plugin method from cmeyers adapted to global collection list Get tests passing Mild rebranding Put behind feature flag, flip true in dev Add noqa flag * Add missing wait_for_events
* Callback plugin method from cmeyers adapted to global collection list Get tests passing Mild rebranding Put behind feature flag, flip true in dev Add noqa flag * Add missing wait_for_events
SUMMARY
This accomplishes passing the test:
The scaffolding of the test was mostly set up in prior merge #15754 and this fills in some of the TODO there. Basically, this ships the data in the collection
event_query.ymldata from ansible-runner back to AWX. Qualifier: this uses internal ansible-core methods, and this is ultimately not acceptable, so we'll table that.After this, what is needed:
installed_collections, and from that, gets all collectionevent_query.ymldata. Then it loops over all job event data and applies JQ logic to get the data to save into the new model, and bulk inserts it.I'm still a little concerned about performance on a number of items here.
This is somewhat competitive with @pb82's draft #15750, and it is derived from @chrismeyersfsu still-earlier PoC https://github.com/chrismeyersfsu/indirect-instance-count. But importantly, and strategically, it merges the solution to this problem with the solution to ansible/ansible-runner#1273
This PR also willfully disregards the decision to put this logic in ansible-runner's (stdout) callback plugin. There are 2 reasons for that. Firstly, this is way way easier to fully test right now, as it plugs into the
file://based testing system in the live tests, so we can go full cycle. Secondly, there may be resistance to making this an ansible-runner or ansible-core "thing" and rightfully deserves to live on the AWX software layer, which is an argument I am re-stating from @sivel. It's not a bad argument, and my intent with this PoC is to clearly show an interface where the AWX task system sets up collection, and then receives collected data in the artifacts folder. It's clear what we're collecting, and we can adhere to whatever the updated contract will be.ISSUE TYPE
COMPONENT NAME