-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
allow smartos vmadm to create docker vms #32819
Conversation
@fishman Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information. Here are the items we could not find in your description:
Please set the description of this pullrequest with this template: |
The test
|
docker: | ||
required: false | ||
description: | ||
- Enable Docker mode |
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.
Is it obvious what this means, could any extra information be given?
also missing full stop.
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.
the whole docker mode isn't exactly obvious to use, but I changed the description
@gundalow anything else that you need to get this merged? |
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.
Looks fine, just a few nitpicks to address please :)
docker: | ||
required: false | ||
description: | ||
- Docker images need this flag enabled along with the brand set to lx. |
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 use please use proper formatting for brand
and lx
to make it clear these refer to parameters? e.g. I(brand) set to C(lx)
.
@@ -608,7 +613,8 @@ def main(): | |||
'bool': [ | |||
'archive_on_delete', 'autoboot', 'debug', 'delegate_dataset', | |||
'firewall_enabled', 'force', 'indestructible_delegated', | |||
'indestructible_zoneroot', 'maintain_resolvers', 'nowait' | |||
'indestructible_zoneroot', 'maintain_resolvers', 'nowait', | |||
'docker' |
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.
Please keep this list sorted.
@jasperla done thanks, guess I should have seen the module documentation guide first |
@jasperla can we get this merged ? |
Merging into Will be released in Ansible 2.5.0 |
SUMMARY
vmadm allows creating vms with docker images. the current vmadm.py will fail saying that docker in not an allowed option
ISSUE TYPE
COMPONENT NAME
modules/cloud/smartos/vmadm.py
ANSIBLE VERSION
ansible 2.4.1.0
devel