-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add generator subcommand to ansible-dk CLI #65
base: master
Are you sure you want to change the base?
Conversation
…ke it a dependency of ansible-dk-cli
@click.option('--generator', '-g', default=get_config()['generator'], help='Path to custom generator repo') | ||
@click.option('-e', 'extra_vars', multiple=True, callback=parse_extra_vars, help='extra vars to pass to ansible, var=value') | ||
@click.argument('name') | ||
def generated_method(debug, generator, name, extra_vars): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should comment this section to describe your intent as to what is going on here. Cleverness can be harmful, remember this code has to be maintained by the whole team.
When we add a CLI command, we should add real help. This should be describing what the generate command does, from a user perspective, and give a pointer to more detailed docs. [clinton@and-cheese ansible-dk]$ ansible-dk-cli/ansible-dk generate --help
Usage: ansible-dk generate [OPTIONS] COMMAND [ARGS]...
Options:
--help Show this message and exit.
Commands:
config
filter
inventory
playbook
repo
role |
|
||
################################################### | ||
# Defaults | ||
RUBY_ABI = "2.1.0" | ||
PYTHON_ABI = "2.7" | ||
HOME = os.getenv("HOME") | ||
|
||
def get_config(debug=False): | ||
#TODO: pass debug properly | ||
config_path=os.path.expanduser('~/.ansible-dk/config') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make this config.yaml instead of plain config, to make it obvious?
I'm thinking we'd like to be able to leave open the possibility of shipping multiple stock generators.
|
run_ansible(playbook_path, extra_vars) | ||
generated_method.__name__ = 'generate_%s' % thing | ||
|
||
generate_things = ['repo', 'role', 'filter', 'inventory', 'config', 'playbook'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong - repo should treat its 'name' parameter differently, and needs to create the directory. The implementation of 'repo' is different than the others.
variable_manager = VariableManager() | ||
inventory = Inventory(loader=loader, variable_manager=variable_manager, host_list='localhost') | ||
variable_manager.set_inventory(inventory) | ||
if vars: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will ALWAYS have vars to pass into the run. We need to determine what they are, but a good start would be:
adk_generator_command # 'role' in ansible-dk generate role pony
adk_generator_target_path # ./pony if we're in repo mode, . otherwise
adk_generator_target_name # 'pony' in ansible-dk generate role pony
adk_generator_source # /opt/ansible-dk/generators/default or whatever
The ansible-dk CLI will maintain a list of 'context vars' that it passes to the ansible run. We must document these in the README or somewhere else.
not ready - just starting the pull request
Update: I believe this is now ready to merge, have done a test build and all seems well.
We will still need to make sane default playbooks, the ones in place now are dummy placeholders.