Skip to content
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

[WIP] initial cutover to API profiles - DO NOT MERGE #35538

Merged
merged 1 commit into from
Feb 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
157 changes: 111 additions & 46 deletions lib/ansible/module_utils/azure_rm_common.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,11 @@
# Copyright (c) 2016 Matt Davis, <mdavis@ansible.com>
# Chris Houseknecht, <house@redhat.com>
#
# This file is part of Ansible
#
# Ansible is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# Ansible is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Ansible. If not, see <http://www.gnu.org/licenses/>.
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)

import json
import os
import re
import sys
import types
import copy
import inspect
import traceback
Expand Down Expand Up @@ -47,7 +33,8 @@
ad_user=dict(type='str', no_log=True),
password=dict(type='str', no_log=True),
cloud_environment=dict(type='str'),
cert_validation_mode=dict(type='str', choices=['validate', 'ignore'])
cert_validation_mode=dict(type='str', choices=['validate', 'ignore']),
api_profile=dict(type='str', default='latest')
# debug=dict(type='bool', default=False),
Copy link
Contributor

Choose a reason for hiding this comment

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

why should api_profile be module parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent is to allow it to be set/overridden the same way as all other "auth-y/connection-y" stuff, so we'd also add it to env and disk cred profiles once it's fully-supported (also important for Tower). I'm not sure if the CLI stuff exposes the selected api_profile the same way as current cloud_env/current subscription/etc, but we'd need a way to get that when using CLI auth (eg against an Azure Stack instance).

)

Expand All @@ -63,6 +50,31 @@
cert_validation_mode='AZURE_CERT_VALIDATION_MODE',
)

# FUTURE: this should come from the SDK or an external location.
# For now, we have to copy from azure-cli
AZURE_API_PROFILES = {
'latest': {
'ContainerInstanceManagementClient': '2018-02-01-preview',
'ComputeManagementClient': dict(
default_api_version='2017-12-01',
resource_skus='2017-09-01',
disks='2017-03-30',
snapshots='2017-03-30',
virtual_machine_run_commands='2017-03-30'
),
'NetworkManagementClient': '2017-11-01',
'ResourceManagementClient': '2017-05-10',
'StorageManagementClient': '2017-10-01'
},

'2017-03-09-profile': {
'ComputeManagementClient': '2016-03-30',
'NetworkManagementClient': '2015-06-15',
'ResourceManagementClient': '2016-02-01',
'StorageManagementClient': '2016-01-01'
}
}

AZURE_TAG_ARGS = dict(
tags=dict(type='dict'),
append_tags=dict(type='bool', default=True),
Expand Down Expand Up @@ -162,36 +174,36 @@ def format_resource_id(val, subscription_id, namespace, types, resource_group):
type=types,
subscription=subscription_id) if not is_valid_resource_id(val) else val

# FUTURE: either get this from the requirements file (if we can be sure it's always available at runtime)
# or generate the requirements files from this so we only have one source of truth to maintain...
AZURE_PKG_VERSIONS = {
StorageManagementClient.__name__: {
'StorageManagementClient': {
'package_name': 'storage',
'expected_version': '1.5.0',
'installed_version': storage_client_version
'expected_version': '1.5.0'
},
ComputeManagementClient.__name__: {
'ComputeManagementClient': {
'package_name': 'compute',
'expected_version': '2.0.0',
'installed_version': compute_client_version
'expected_version': '2.0.0'
},
'ContainerInstanceManagementClient': {
'package_name': 'containerinstance',
'expected_version': '0.3.1'
},
NetworkManagementClient.__name__: {
'NetworkManagementClient': {
'package_name': 'network',
'expected_version': '1.3.0',
'installed_version': network_client_version
'expected_version': '1.3.0'
},
ResourceManagementClient.__name__: {
'ResourceManagementClient': {
'package_name': 'resource',
'expected_version': '1.1.0',
'installed_version': resource_client_version
'expected_version': '1.1.0'
},
DnsManagementClient.__name__: {
'DnsManagementClient': {
'package_name': 'dns',
'expected_version': '1.0.1',
'installed_version': dns_client_version
'expected_version': '1.0.1'
},
WebSiteManagementClient.__name__: {
'WebSiteManagementClient': {
'package_name': 'web',
'expected_version': '0.32.0',
'installed_version': web_client_version
'expected_version': '0.32.0'
},
} if HAS_AZURE else {}

Expand Down Expand Up @@ -250,6 +262,7 @@ def __init__(self, derived_arg_spec, bypass_checks=False, no_log=False,
self._containerservice_client = None

self.check_mode = self.module.check_mode
self.api_profile = self.module.params.get('api_profile')
self.facts_module = facts_module
# self.debug = self.module.params.get('debug')

Expand Down Expand Up @@ -337,10 +350,15 @@ def check_client_version(self, client_type):
package_version = AZURE_PKG_VERSIONS.get(client_type.__name__, None)
if package_version is not None:
client_name = package_version.get('package_name')
client_version = package_version.get('installed_version')
try:
client_module = importlib.import_module(client_type.__module__)
client_version = client_module.VERSION
except RuntimeError:
# can't get at the module version for some reason, just fail silently...
return
expected_version = package_version.get('expected_version')
if Version(client_version) < Version(expected_version):
self.fail("Installed {0} client version is {1}. The supported version is {2}. Try "
self.fail("Installed azure-mgmt-{0} client version is {1}. The supported version is {2}. Try "
"`pip install ansible[azure]`".format(client_name, client_version, expected_version))

def exec_module(self, **kwargs):
Expand Down Expand Up @@ -767,18 +785,65 @@ def create_default_securitygroup(self, resource_group, location, security_group_
def _validation_ignore_callback(session, global_config, local_config, **kwargs):
session.verify = False

def get_api_profile(self, client_type_name, api_profile_name):
profile_all_clients = AZURE_API_PROFILES.get(api_profile_name)

if not profile_all_clients:
raise KeyError("unknown Azure API profile: {0}".format(api_profile_name))

profile_raw = profile_all_clients.get(client_type_name, None)

if not profile_raw:
self.module.warn("Azure API profile {0} does not define an entry for {1}".format(api_profile_name, client_type_name))

if isinstance(profile_raw, dict):
if not profile_raw.get('default_api_version'):
raise KeyError("Azure API profile {0} does not define 'default_api_version'".format(api_profile_name))
return profile_raw

# wrap basic strings in a dict that just defines the default
return dict(default_api_version=profile_raw)

def get_mgmt_svc_client(self, client_type, base_url=None, api_version=None):
self.log('Getting management service client {0}'.format(client_type.__name__))
self.check_client_version(client_type)
if api_version:
client = client_type(self.azure_credentials,
self.subscription_id,
api_version=api_version,
base_url=base_url)
else:
client = client_type(self.azure_credentials,
self.subscription_id,
base_url=base_url)

client_argspec = inspect.getargspec(client_type.__init__)

client_kwargs = dict(credentials=self.azure_credentials, subscription_id=self.subscription_id, base_url=base_url)

api_profile_dict = {}

if self.api_profile:
api_profile_dict = self.get_api_profile(client_type.__name__, self.api_profile)

if not base_url:
# most things are resource_manager, don't make everyone specify
base_url = self._cloud_environment.endpoints.resource_manager

# unversioned clients won't accept profile; only send it if necessary
# clients without a version specified in the profile will use the default
if api_profile_dict and 'profile' in client_argspec.args:
Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid inspect with

if hasattr(client, 'DEFAULT_PROFILE')

client_kwargs['profile'] = api_profile_dict

# If the client doesn't accept api_version, it's unversioned.
# If it does, favor explicitly-specified api_version, fall back to api_profile
if 'api_version' in client_argspec.args:
Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid inspect with

if hasattr(client, 'DEFAULT_API_VERSION')

Will not work for azure-mgmt-resource before 1.3.0 if you want multi-api support there as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, thanks- since this is in the common code, maybe we'll try that approach later in 2.6 once that's available everywhere.

profile_default_version = api_profile_dict.get('default_api_version', None)
if api_version or profile_default_version:
client_kwargs['api_version'] = api_version or profile_default_version

client = client_type(**client_kwargs)

# FUTURE: remove this once everything exposes models directly (eg, containerinstance)
Copy link
Contributor

Choose a reason for hiding this comment

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

ContainerInstance doesn't support profile, it's why it doesn't have a models method

Copy link
Contributor

Choose a reason for hiding this comment

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

so again, as with api_version, is there a reason that it shouldn't have models method just for consistency?

Copy link
Contributor

@lmazuel lmazuel Feb 9, 2018

Choose a reason for hiding this comment

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

models in multi-api packages takes an api-version in parameter and return the models for this ApiVersion.
On a single api package, this doesn't make sense to have an api-version parameter. If I do models, the most logical would be to have no parameter. Technically you could do:

try:
   # raise NotImplementedError if api_version not supported
   models = client.models(api_version)
except TypeError: # Does not support api_version parameter
   models = client.models()

@zikalino @nitzmahone Would this help?

try:
getattr(client, "models")
except AttributeError:
def _ansible_get_models(self, *arg, **kwarg):
return self._ansible_models

setattr(client, '_ansible_models', importlib.import_module(client_type.__module__).models)
client.models = types.MethodType(_ansible_get_models, client)

# Add user agent for Ansible
client.config.add_user_agent(ANSIBLE_USER_AGENT)
Expand Down
67 changes: 33 additions & 34 deletions lib/ansible/modules/cloud/azure/azure_rm_containerinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,6 @@

try:
from msrestazure.azure_exceptions import CloudError
from azure.mgmt.containerinstance.models import (ContainerGroup,
Container,
ResourceRequirements,
ResourceRequests,
ImageRegistryCredential,
IpAddress,
Port,
ContainerPort)
from azure.mgmt.containerinstance import ContainerInstanceManagementClient
except ImportError:
# This is handled in azure_rm_common
Expand Down Expand Up @@ -264,7 +256,8 @@ def __init__(self):
self.containers = None

self.results = dict(changed=False, state=dict())
self.mgmt_client = None
self.client = None
self.cgmodels = None

super(AzureRMContainerInstance, self).__init__(derived_arg_spec=self.module_arg_spec,
supports_check_mode=True,
Expand All @@ -280,8 +273,10 @@ def exec_module(self, **kwargs):
response = None
results = dict()

self.mgmt_client = self.get_mgmt_svc_client(ContainerInstanceManagementClient,
base_url=self._cloud_environment.endpoints.resource_manager)
self.client = self.get_mgmt_svc_client(ContainerInstanceManagementClient)

# since this client hasn't been upgraded to expose models directly off the OperationClass, fish them out
self.cgmodels = self.client.container_groups.models

resource_group = self.get_resource_group(self.resource_group)

Expand Down Expand Up @@ -341,9 +336,9 @@ def create_update_containerinstance(self):
registry_credentials = None

if self.registry_login_server is not None:
registry_credentials = [ImageRegistryCredential(server=self.registry_login_server,
username=self.registry_username,
password=self.registry_password)]
registry_credentials = [self.cgmodels.ImageRegistryCredential(server=self.registry_login_server,
username=self.registry_username,
password=self.registry_password)]

ip_address = None

Expand All @@ -352,8 +347,8 @@ def create_update_containerinstance(self):
if self.ports:
ports = []
for port in self.ports:
ports.append(Port(port=port, protocol="TCP"))
ip_address = IpAddress(ports=ports, ip=self.ip_address)
ports.append(self.cgmodels.Port(port=port, protocol="TCP"))
ip_address = self.cgmodels.IpAddress(ports=ports, ip=self.ip_address)

containers = []

Expand All @@ -367,22 +362,26 @@ def create_update_containerinstance(self):
port_list = container_def.get("ports")
if port_list:
for port in port_list:
ports.append(ContainerPort(port))

containers.append(Container(name=name,
image=image,
resources=ResourceRequirements(ResourceRequests(memory_in_gb=memory, cpu=cpu)),
ports=ports))

parameters = ContainerGroup(location=self.location,
containers=containers,
image_registry_credentials=registry_credentials,
restart_policy=None,
ip_address=ip_address,
os_type=self.os_type,
volumes=None)

response = self.mgmt_client.container_groups.create_or_update(self.resource_group, self.name, parameters)
ports.append(self.cgmodels.ContainerPort(port=port))

containers.append(self.cgmodels.Container(name=name,
image=image,
resources=self.cgmodels.ResourceRequirements(
requests=self.cgmodels.ResourceRequests(memory_in_gb=memory, cpu=cpu)
),
ports=ports))

parameters = self.cgmodels.ContainerGroup(location=self.location,
containers=containers,
image_registry_credentials=registry_credentials,
restart_policy=None,
ip_address=ip_address,
os_type=self.os_type,
volumes=None)

response = self.client.container_groups.create_or_update(resource_group_name=self.resource_group,
container_group_name=self.name,
container_group=parameters)

return response.as_dict()

Expand All @@ -393,7 +392,7 @@ def delete_containerinstance(self):
:return: True
'''
self.log("Deleting the container instance {0}".format(self.name))
response = self.mgmt_client.container_groups.delete(self.resource_group, self.name)
response = self.client.container_groups.delete(resource_group_name=self.resource_group, container_group_name=self.name)
return True

def get_containerinstance(self):
Expand All @@ -405,7 +404,7 @@ def get_containerinstance(self):
self.log("Checking if the container instance {0} is present".format(self.name))
found = False
try:
response = self.mgmt_client.container_groups.get(self.resource_group, self.name)
response = self.client.container_groups.get(resource_group_name=self.resource_group, container_group_name=self.name)
found = True
self.log("Response : {0}".format(response))
self.log("Container instance : {0} found".format(response.name))
Expand Down