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
Stop(deallocate) and Start vm actions #45119
Conversation
Adding missing functionality for starting and stopping vm
Hi @sumeetisp! Thank you for this addition. Can you fix the lint errors https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/17562/violations/file/salt/cloud/clouds/azurearm.py/? |
Done |
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.
I had a couple of questions and suggestions here. Otherwise this looks good.
salt/cloud/clouds/azurearm.py
Outdated
def stop(name, resource_group=None, call=None): | ||
''' | ||
Stop a VM | ||
|
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.
Can you add a .. versionadded:: Fluorine
here?
salt/cloud/clouds/azurearm.py
Outdated
def start(name, resource_group=None, call=None): | ||
''' | ||
Start a VM | ||
|
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 also need the same version added tag.
salt/cloud/clouds/azurearm.py
Outdated
try: | ||
instance_stop = compconn.virtual_machines.deallocate(group, name) | ||
instance_stop.wait() | ||
resource_group = group |
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.
What is the purpose of setting this resource_group variable? It looks like it's not being used anywhere later.
salt/cloud/clouds/azurearm.py
Outdated
try: | ||
instance_start = compconn.virtual_machines.start(group, name) | ||
instance_start.wait() | ||
resource_group = group |
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.
Same question about setting the resource group here as above.
Hi Nicole,
I will be reviewing the code after getting back from vacation on 1st Jan.
Thanks,
Sumeet Salvankar
On 22-Dec-2017 23:17, "Nicole Thomas" <notifications@github.com> wrote:
*@rallytime* requested changes on this pull request.
I had a couple of questions and suggestions here. Otherwise this looks good.
------------------------------
In salt/cloud/clouds/azurearm.py
<#45119 (comment)>:
@@ -1834,3 +1834,63 @@ def delete_blob(call=None, kwargs=None): # pylint: disable=unused-argument
storageservice.delete_blob(kwargs['container'], kwargs['blob'])
return True
+
+
+def stop(name, resource_group=None, call=None):
+ '''
+ Stop a VM
+
Can you add a .. versionadded:: Fluorine here?
------------------------------
In salt/cloud/clouds/azurearm.py
<#45119 (comment)>:
+ try:
+ instance_stop =
compconn.virtual_machines.deallocate(group, name)
+ instance_stop.wait()
+ resource_group = group
+ except CloudError:
+ continue
+ else:
+ instance_stop = compconn.virtual_machines.stop(resource_group, name)
+ instance_stop.wait()
+ return instance_stop
+
+
+def start(name, resource_group=None, call=None):
+ '''
+ Start a VM
+
This also need the same version added tag.
------------------------------
In salt/cloud/clouds/azurearm.py
<#45119 (comment)>:
+ .. code-block:: bash
+
+ salt-cloud -a stop myminion
+ '''
+ if call == 'function':
+ raise SaltCloudSystemExit(
+ 'The stop action must be called with -a or --action.'
+ )
+ if not compconn:
+ compconn = get_conn()
+ if resource_group is None:
+ for group in list_resource_groups():
+ try:
+ instance_stop =
compconn.virtual_machines.deallocate(group, name)
+ instance_stop.wait()
+ resource_group = group
What is the purpose of setting this resource_group variable? It looks like
it's not being used anywhere later.
------------------------------
In salt/cloud/clouds/azurearm.py
<#45119 (comment)>:
+ .. code-block:: bash
+
+ salt-cloud -a start myminion
+ '''
+ if call == 'function':
+ raise SaltCloudSystemExit(
+ 'The start action must be called with -a or --action.'
+ )
+ if not compconn:
+ compconn = get_conn()
+ if resource_group is None:
+ for group in list_resource_groups():
+ try:
+ instance_start = compconn.virtual_machines.start(group, name)
+ instance_start.wait()
+ resource_group = group
Same question about setting the resource group here as above.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#45119 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHg-ExF8X9n3i0lW4m_MG3iy54x_YxzDks5tC-s4gaJpZM4RJTpt>
.
|
I have made the suggested changes |
@sumeetisp This looks good, but there's a lint failure: https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/17815/violations/file/salt/cloud/clouds/azurearm.py/ Once that is fixed up we can get this in. :) |
Go Go Jenkins! |
Adding missing functionality for starting and stopping vm
What does this PR do?
This PR adds functionality for stopping(deallocate) and starting azure virtual machines.
What issues does this PR fix or reference?
Previous Behavior
Remove this section if not relevant
New Behavior
Remove this section if not relevant
Tests written?
Yes/No
Commits signed with GPG?
Yes/No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.