Skip to content

Added base Azure backend#105

Open
ader1990 wants to merge 1 commit into
cloudbase:masterfrom
ader1990:argus-azure-master
Open

Added base Azure backend#105
ader1990 wants to merge 1 commit into
cloudbase:masterfrom
ader1990:argus-azure-master

Conversation

@ader1990

Copy link
Copy Markdown
Member

No description provided.

@ader1990
ader1990 force-pushed the argus-azure-master branch 2 times, most recently from 546f981 to 5db80a5 Compare March 14, 2017 15:42

@stefan-caraiman stefan-caraiman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides pep8 issues and a few suggestions, the patch looks alright.

Comment thread argus/config/azure.py
help="The subscription id for the Azure account"
"used for tests."),
cfg.StrOpt(
"username", default=None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can fit on the one above.

Comment thread argus/backends/azure/client.py Outdated
}]
},}

@util.retry_on_error(max_attempts=20, sleep_seconds=5)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use CONF values regarding the retry count

Comment thread argus/backends/azure/client.py Outdated

@util.retry_on_error(max_attempts=20, sleep_seconds=5)
def get_floating_ip(self):
floating_ip = self._network_client.public_ip_addresses.get(self._resource_group_name, self._vip_name).ip_address

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not pep8 compliant

Comment thread argus/backends/azure/client.py Outdated
self._resource_group_name, self._sec_group_name, {'location':self._availability_zone})
secgroup_info = async_secgroup_creation.result()

self._network_client.security_rules.create_or_update(self._resource_group_name, self._sec_group_name, 'secgr1', {'access':'allow', 'protocol':'Tcp', 'direction':'inbound', 'source_address_prefix':'*','destination_address_prefix':'*', 'source_port_range':'*', 'destination_port_range':'3389', 'priority':100}).wait()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use a for loop and only change the required values from the dict instead of repeating these code blocks, also not pep8 compliant


def create_nic(self):
"""Create a Network Interface for a VM.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for new line

Comment thread argus/backends/azure/client.py Outdated
from azure.mgmt.compute import ComputeManagementClient
from azure.storage import CloudStorageAccount

VM_REFERENCE = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be global?

Comment thread argus/backends/azure/azure_backend.py Outdated
return { 'image': {'OS-EXT-IMG-SIZE:size': 50000}}

def get_mtu(self):
return 1450

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there already a function used for getting the mtu size?

Comment thread argus/backends/azure/azure_backend.py Outdated

def get_image_by_ref(self):
"""Get the image object by its reference id."""
return { 'image': {'OS-EXT-IMG-SIZE:size': 50000}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will all images be 50000 in size? or is there another reason for this

status = execute_function(
'Test-Path "{}\\Cloudbase` Solutions"'.format(
_location), command_type=util.POWERSHELL).strip().lower()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to remove this line, since there are separated logic code blocks.


def _create_vm_parameters(self, vm_name, vm_username, vm_password, os_disk_name, nic_id, vm_reference):
"""Create the VM parameters structure.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for new line

@ader1990
ader1990 force-pushed the argus-azure-master branch 8 times, most recently from 1e57378 to 0da0377 Compare March 16, 2017 14:58
Adds support for Argus testing on Azure cloud.
This commit adds an Azure backend, an Azure recipe
and a scenario (ScenarioAzure) that verifies that no errors
are found in the cloudbase-init logs.
@ader1990
ader1990 force-pushed the argus-azure-master branch from 0da0377 to d7e5153 Compare March 16, 2017 15:18

CONFIG = argus_config.CONFIG

RETRY_COUNT = 50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to have these as configurable options in default? Also I don't find these variables being used anywhere

self._creds, subscription_id)
self._vm_client = ComputeManagementClient(
self._creds, subscription_id)
self.net_client = NetworkManagementClient(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be public?

self.vm_password = vm_password
self._creds = UserPassCredentials(username, password)
self.res_client = ResourceManagementClient(
self._creds, subscription_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, couldn't we save this to a variable since we are using it to initialize other objects too?


@staticmethod
def _get_random_id():
return str(random.randint(0, 10000))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a method for generating random names in util, util.rand_name.

storage_account_name, availability_zone, image_vhd_path,
vm_username, vm_password)

self.vm_name = "argusvm-" + AzureClient._get_random_id()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use util.rand_name, also we could save this to a variable instead of calling _get_random_id each time.

self._vm_client.virtual_machines.start(
self._resource_group_name, self.vm_name)

def destroy_vm(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public?

def _get_random_id():
return str(random.randint(0, 10000))

def create_vm(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants