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

Azure VM: Adds feature to choose Availability Zone for VM #46581

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
6 participants
@kamotos

kamotos commented Oct 6, 2018

SUMMARY

This change makes it possible for azure_rm_virtualmachine users to choose on which Availability Zone their Azure Virtual Machine will be created.

Fixes #40596

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

azure_rm_virtualmachine

ANSIBLE VERSION
ansible 2.8.0.dev0
ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Oct 6, 2018

Hi @kamotos,

Thank you for the pullrequest, just so you are aware we have a dedicated Working Group for azure.
You can find other people interested in this in #ansible-azure on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@kamotos

This comment has been minimized.

kamotos commented Oct 7, 2018

I'm getting an "Failed to determine PowerState of virtual machine vm17f53a" error message, as I'm a bit new, I was wondering if any of the module maintainers have an idea on how to fix it?

@@ -1224,6 +1237,7 @@ def exec_module(self, **kwargs):
network_profile=self.compute_models.NetworkProfile(
network_interfaces=nics
),
zones=zones

This comment has been minimized.

@kamotos

kamotos Oct 10, 2018

Good catch. Will update ASAP.

This comment has been minimized.

@kamotos

kamotos Oct 12, 2018

Updated. Could use some help/hints on how to make the integrations tests I wrote work based on the current setup.

@@ -694,6 +702,7 @@ class AzureRMVirtualMachine(AzureRMModuleBase):
def __init__(self):
self.module_arg_spec = dict(
availability_zone=dict(type='int', choices=[1, 2, 3]),

This comment has been minimized.

@yuwzho

yuwzho Oct 8, 2018

Contributor

should we accept list instead of a single one?

This comment has been minimized.

@kamotos

kamotos Oct 10, 2018

From my understanding, a virtual machine can be only created in one availability zone. From the user's perspective having to provide a list with a single element can be a bit misleading I think.

@kamotos kamotos force-pushed the kamotos:azure-vm-zones branch from f98070e to 07c323e Oct 12, 2018

@kamotos kamotos force-pushed the kamotos:azure-vm-zones branch from 07c323e to acfd0a0 Oct 12, 2018

vm_size: Standard_A0
availability_zone: 1
public_ip_allocation_method: Disabled
location: centralus

This comment has been minimized.

@kamotos

kamotos Oct 12, 2018

From my understanding, the current integration tests are being ran on eastus region which doesn't support availability zones yet.

One of the solutions I see:

  1. Create a new resource group in centralus region which happens to support availability zones and use it for the purpose of these tests
  2. Move the current resource group to centralus or any other region that supports Availability zone all along with other features being tested?

As I am new to this, I am open to suggestions regarding the best way to test this.

This comment has been minimized.

@yuwzho

yuwzho Oct 15, 2018

Contributor

I would like to move one of the vm1 or vm2 to centralus to avoid creating more VM, which requires many dependencies and costs a lot of time.
BTW, the resource group is created out side of the integration test, and we run the whole yaml file in the resource group. My concern is whether the subscription, the integration test pays for, can allow us to create vm in centralus.

This comment has been minimized.

@kamotos

kamotos Oct 28, 2018

Is there any way I can do that myself? or is it more in the hands of the people who are in charge of the Microsoft Azure account from which theses tests are ran (and which owns the resource group).

This comment has been minimized.

@zikalino

zikalino Oct 28, 2018

Contributor

@kamotos you should just add "location:" parameter in appropriate tasks and see if that works.

This comment has been minimized.

@kamotos

kamotos Nov 24, 2018

@zikalino Sorry for the late answer. I think I have already added the location paramater to the integration test I wrote but that didn't work because the resource group is not in that region (location).

Or did you mean some other tasks when you were talking about "appropriate tasks"?

@mattclay

This comment has been minimized.

Member

mattclay commented Oct 15, 2018

@Fred-sun

This comment has been minimized.

Contributor

Fred-sun commented Oct 24, 2018

@kamotos Thanks for the contribution, do you have idea according by @yuwzho's comments? Thanks!

@ansibot ansibot added the stale_ci label Oct 24, 2018

@kamotos

This comment has been minimized.

kamotos commented Oct 28, 2018

@Fred-sun From my understanding, the current resource group is created outside of the integration tests, but I'm not sure how I can get around this. Could use some guidance to know how to proceed next.

@Fred-sun

This comment has been minimized.

Contributor

Fred-sun commented Nov 13, 2018

@yuwzho Could you help to guidance this? Thanks!

@Fred-sun

This comment has been minimized.

Contributor

Fred-sun commented Nov 22, 2018

@zikalino @yuwzho Which one could help to guidance this? Thanks!

@Fred-sun

This comment has been minimized.

Contributor

Fred-sun commented Nov 30, 2018

@yuwzho @zikalino Could you help to guidance the contributor? Another, this PR seem duplicate of #49243 ,Could you help review and push to merged? Thanks!

@Fred-sun

This comment has been minimized.

Contributor

Fred-sun commented Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment