Adding import/export modules around AWX Kit features #7799
Conversation
|
Build failed.
|
ryanpetrello
requested review from
AlanCoding,
beeankha,
squidboylan,
chrismeyersfsu and
jbradberry
and removed request for
AlanCoding
|
Build failed.
|
| # Here we are going to setup a dict of values to export | ||
| export_args = {} | ||
| for resource in EXPORTABLE_RESOURCES: | ||
| if module.params.get('all') or module.params.get(resource) == 'all': |
There was a problem hiding this comment.
Seems like we should add some really simple validation for the case that
module.params.get('all') and module.params.get(resource) not in (None, 'all')In that case, you're getting conflicting instructions from the user. The "all" parameter dictates that you should export all credentials, but the credential param says to export just "my-cred", for example.
There was a problem hiding this comment.
Should we consider moving away from the use of 'all' flags? The actual semantics of the import/export code that I wrote is unambiguous, the case that @AlanCoding is talking about here cannot arise.
There was a problem hiding this comment.
I'm open to input but I think I am partial to the explicit all parameter and keywords. We could make it do something like:
- name: Export all resources
tower_export:
register: all_exported_data
And then have something like your code:
# If no resource parameters are explicitly used, export everything.
all_resources = all(module.params.get(resource) is None for resource in EXPORTABLE_RESOURCES)
But then we are back to our existing loop, just changing module.params.get('all') to all_resources.
I think I am also partial to the explicit:
- name: Export all credentials
tower_export:
credentials: 'all'
as opposed to :
- name: Export all credentials
tower_export:
credentials: ''
The second syntax looks like it could be confused with "nothing" vs an all. Anyone else have thoughts on this?
There was a problem hiding this comment.
Hmm, as a gut reaction I personally like the aesthetics of this
- name: Export all resources
tower_export:
register: all_exported_dataBut the behavior is "jumpy". If you have credentials: "foo" then it exports 1 thing, but take it away and it exports 1,000 things. If this syntax returned nothing that would give a better impression of behavioral stability. I'm just saying it's arguable so I don't want to take a position.
I don't have a problem with the current "all" param. The validation statement I'm talking about would just be 1 conditional.
I also don't want to care about giving blank values. Ansible has an omit syntax. You could throw an error for blank values for clarity, but it doesn't bother me either way, it shouldn't be your problem to deal with.
| @@ -89,7 +89,7 @@ def main(): | |||
| ) | |||
|
|
|||
| # Create a module for ourselves | |||
| module = TowerModule(argument_spec=argument_spec) | |||
| module = TowerAPIModule(argument_spec=argument_spec) | |||
There was a problem hiding this comment.
The new import/export code does not support hosts or groups, though there is an open issue to add them in.
awx_collection/tests/integration/targets/tower_import/tasks/main.yml
Outdated
Show resolved
Hide resolved
|
Build failed.
|
|
Build failed.
|
|
Build succeeded.
|
|
👍 looking so far. I'm kicking off some tests, once those are passing, then I'm going to be pushing to get this ready for merge. |
| log_contents = log_capture_string.getvalue() | ||
| log_capture_string.close() | ||
| if log_contents != '': | ||
| module.fail_json(msg=log_contents) |
There was a problem hiding this comment.
this seems a little extreme... wouldn't you just want to warn with the log contents? Wouldn't the CLI code raise an exception if something was actually a problem? Asking @jbradberry here too
There was a problem hiding this comment.
I think I agree, this seems a bit much.
john-westcott-iv
force-pushed
the
import-export-collecion-modules
branch
from
fcf9974 to
1fb0e86
Compare
|
Build succeeded.
|
|
Build succeeded.
|
Changed library structure Origional TowerModule becomes TowerLegacyModule TowerModule from tower_api becomes TowerAPIModule A real base TowerModule is created in tower_module.py A new TowerAWXKitModule is created in tower_awxkit TowerAWXKitModule and TowerAPIModule are child classes of TowerModule
john-westcott-iv
force-pushed
the
import-export-collecion-modules
branch
from
d510d8b to
a5afe02
Compare
|
Build failed.
|
|
recheck |
|
Build failed.
|
|
recheck |
|
Build succeeded.
|
|
Build succeeded (gate pipeline).
|
SUMMARY
This PR adds a tower_import and tower_export modules to wrap the awxkit features.
It includes changes to the module_utils library structure:
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION