From 5414b31800b8dc077f123128940b2867b799f9d6 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Mon, 14 May 2018 15:41:33 -0400 Subject: [PATCH 1/2] salt-cloud: allow map nodes to override provider due to the way salt-cloud was previously pulling provider from only the profile, it made it impossible to use map node overrides to override a provider. This patch makes that possible. The main usecase for this is having profiles that model your node type (db, webapp, etc), without having to duplicate across providers. Your map can then specify provider specific overrides whilst keeping profiles unique and DRY: webapp: - node1: provider: ny:openstack - node2: provider: nj:openstack - node3: provider: toronto:openstack etc. --- salt/cloud/__init__.py | 30 ++++++++------- tests/unit/cloud/test_map_conf.py | 62 ++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/salt/cloud/__init__.py b/salt/cloud/__init__.py index e5cd235839..5e34930015 100644 --- a/salt/cloud/__init__.py +++ b/salt/cloud/__init__.py @@ -1934,21 +1934,23 @@ class Map(Cloud): profile_data = self.opts['profiles'].get(profile_name) - # Get associated provider data, in case something like size - # or image is specified in the provider file. See issue #32510. - alias, driver = profile_data.get('provider').split(':') - provider_details = self.opts['providers'][alias][driver].copy() - del provider_details['profiles'] - - # Update the provider details information with profile data - # Profile data should override provider data, if defined. - # This keeps map file data definitions consistent with -p usage. - provider_details.update(profile_data) - profile_data = provider_details - for nodename, overrides in six.iteritems(nodes): - # Get the VM name - nodedata = copy.deepcopy(profile_data) + # Get associated provider data, in case something like size + # or image is specified in the provider file. See issue #32510. + if 'provider' in overrides and overrides['provider'] != profile_data['provider']: + alias, driver = overrides.get('provider').split(':') + else: + alias, driver = profile_data.get('provider').split(':') + + provider_details = self.opts['providers'][alias][driver].copy() + del provider_details['profiles'] + + # Update the provider details information with profile data + # Profile data and node overrides should override provider data, if defined. + # This keeps map file data definitions consistent with -p usage. + provider_details.update(profile_data) + nodedata = copy.deepcopy(provider_details) + # Update profile data with the map overrides for setting in ('grains', 'master', 'minion', 'volumes', 'requires'): diff --git a/tests/unit/cloud/test_map_conf.py b/tests/unit/cloud/test_map_conf.py index 996e78f25f..82034a92a9 100644 --- a/tests/unit/cloud/test_map_conf.py +++ b/tests/unit/cloud/test_map_conf.py @@ -21,25 +21,15 @@ import salt.cloud EXAMPLE_PROVIDERS = { 'nyc_vcenter': {'vmware': {'driver': 'vmware', 'password': '123456', - 'profiles': {'nyc-vm': {'cluster': 'nycvirt', - 'datastore': 'datastore1', - 'devices': {'disk': {'Hard disk 1': {'controller': 'SCSI controller 1', - 'size': 20}}, - 'network': {'Network Adapter 1': {'mac': '44:44:44:44:44:42', - 'name': 'vlan50', - 'switch_type': 'standard'}}, - 'scsi': {'SCSI controller 1': {'type': 'paravirtual'}}}, - 'extra_config': {'mem.hotadd': 'yes'}, - 'folder': 'coreinfra', - 'image': 'rhel6_64Guest', - 'memory': '8GB', - 'num_cpus': 2, - 'power_on': True, - 'profile': 'nyc-vm', - 'provider': 'nyc_vcenter:vmware', - 'resourcepool': 'Resources'}}, 'url': 'vca1.saltstack.com', - 'user': 'root'}} + 'profiles': {}, + 'user': 'root'}}, + 'nj_vcenter': {'vmware': {'driver': 'vmware', + 'password': '333', + 'profiles': {}, + 'image': 'rhel6_64prod', + 'url': 'vca2.saltstack.com', + 'user': 'root'}} } EXAMPLE_PROFILES = { @@ -47,7 +37,7 @@ EXAMPLE_PROFILES = { 'datastore': 'datastore1', 'devices': {'disk': {'Hard disk 1': {'controller': 'SCSI controller 1', 'size': 20}}, - 'network': {'Network Adapter 1': {'mac': '44:44:44:44:44:42', + 'network': {'Network Adapter 1': {'mac': '88:88:88:88:88:42', 'name': 'vlan50', 'switch_type': 'standard'}}, 'scsi': {'SCSI controller 1': {'type': 'paravirtual'}}}, @@ -59,7 +49,7 @@ EXAMPLE_PROFILES = { 'power_on': True, 'profile': 'nyc-vm', 'provider': 'nyc_vcenter:vmware', - 'resourcepool': 'Resources'} + 'resourcepool': 'Resources'}, } EXAMPLE_MAP = { @@ -67,7 +57,10 @@ EXAMPLE_MAP = { 'devices': {'disk': {'Hard disk 1': {'size': 40}}, 'network': {'Network Adapter 1': {'mac': '22:4a:b2:92:b3:eb'}}}, 'memory': '16GB', - 'name': 'db1'}} + 'name': 'db1'}, + 'db2': {'name': 'db2', + 'password': '456', + 'provider': 'nj_vcenter:vmware'}} } @@ -87,6 +80,7 @@ class MapConfTest(TestCase): opts = {'extension_modules': '/var/cache/salt/master/extmods', 'providers': EXAMPLE_PROVIDERS, 'profiles': EXAMPLE_PROFILES} cloud_map = salt.cloud.Map(opts) + merged_profile = { 'create': {'db1': {'cluster': 'nycvirt', 'cpus': 4, @@ -110,6 +104,32 @@ class MapConfTest(TestCase): 'provider': 'nyc_vcenter:vmware', 'resourcepool': 'Resources', 'url': 'vca1.saltstack.com', + 'user': 'root'}, + 'db2': {'cluster': 'nycvirt', + 'datastore': 'datastore1', + 'devices': {'disk': {'Hard disk 1': {'controller': 'SCSI controller 1', + 'size': 20}}, + 'network': {'Network Adapter 1': {'mac': '88:88:88:88:88:42', + 'name': 'vlan50', + 'switch_type': 'standard'}}, + 'scsi': {'SCSI controller 1': {'type': 'paravirtual'}}}, + 'driver': 'vmware', + 'extra_config': {'mem.hotadd': 'yes'}, + 'folder': 'coreinfra', + 'image': 'rhel6_64Guest', + 'memory': '8GB', + 'name': 'db2', + 'num_cpus': 2, + 'password': '456', + 'power_on': True, + 'profile': 'nyc-vm', + 'provider': 'nj_vcenter:vmware', + 'resourcepool': 'Resources', + 'url': 'vca2.saltstack.com', 'user': 'root'}} } + # what we assert above w.r.t db2 using nj_vcenter:vmware provider: + # - url is from the overriden nj_vcenter provider, not nyc_vcenter + # - image from provider is still overridden by the nyc-vm profile + # - password from map override is still overriding both the provider and profile password self.assertEqual(cloud_map.map_data(), merged_profile) From 3dc821f795f2c4707e2f16692d1cea8df7b1c56e Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 15 May 2018 10:50:25 -0400 Subject: [PATCH 2/2] salt.cloud: use salt.utils.dictupdate.update for provider shallow copy was blasting things like minion_master in provider when grains were provided in map and profile. additionally noticed a bug while adding tests that map_data() is destructive on self.rendered_map; we now deepcopy before map_data does its thing. --- salt/cloud/__init__.py | 7 +- tests/unit/cloud/test_map_conf.py | 116 +++++++++++++++++++++++------- 2 files changed, 96 insertions(+), 27 deletions(-) diff --git a/salt/cloud/__init__.py b/salt/cloud/__init__.py index 5e34930015..0f87a64ac7 100644 --- a/salt/cloud/__init__.py +++ b/salt/cloud/__init__.py @@ -1916,7 +1916,8 @@ class Map(Cloud): pmap = self.map_providers_parallel(cached=cached) exist = set() defined = set() - for profile_name, nodes in six.iteritems(self.rendered_map): + rendered_map = copy.deepcopy(self.rendered_map) + for profile_name, nodes in six.iteritems(rendered_map): if profile_name not in self.opts['profiles']: msg = ( 'The required profile, \'{0}\', defined in the map ' @@ -1942,13 +1943,13 @@ class Map(Cloud): else: alias, driver = profile_data.get('provider').split(':') - provider_details = self.opts['providers'][alias][driver].copy() + provider_details = copy.deepcopy(self.opts['providers'][alias][driver]) del provider_details['profiles'] # Update the provider details information with profile data # Profile data and node overrides should override provider data, if defined. # This keeps map file data definitions consistent with -p usage. - provider_details.update(profile_data) + salt.utils.dictupdate.update(provider_details, profile_data) nodedata = copy.deepcopy(provider_details) # Update profile data with the map overrides diff --git a/tests/unit/cloud/test_map_conf.py b/tests/unit/cloud/test_map_conf.py index 82034a92a9..ad61716aed 100644 --- a/tests/unit/cloud/test_map_conf.py +++ b/tests/unit/cloud/test_map_conf.py @@ -22,11 +22,23 @@ EXAMPLE_PROVIDERS = { 'nyc_vcenter': {'vmware': {'driver': 'vmware', 'password': '123456', 'url': 'vca1.saltstack.com', + 'minion': { + 'master': 'providermaster', + 'grains': { + 'providergrain': True + } + }, 'profiles': {}, 'user': 'root'}}, 'nj_vcenter': {'vmware': {'driver': 'vmware', 'password': '333', 'profiles': {}, + 'minion': { + 'master': 'providermaster', + 'grains': { + 'providergrain': True + } + }, 'image': 'rhel6_64prod', 'url': 'vca2.saltstack.com', 'user': 'root'}} @@ -44,12 +56,28 @@ EXAMPLE_PROFILES = { 'extra_config': {'mem.hotadd': 'yes'}, 'folder': 'coreinfra', 'image': 'rhel6_64Guest', + 'minion': { + 'master': 'profilemaster', + 'grains': { + 'profilegrain': True + } + }, 'memory': '8GB', 'num_cpus': 2, 'power_on': True, 'profile': 'nyc-vm', 'provider': 'nyc_vcenter:vmware', 'resourcepool': 'Resources'}, + 'nj-vm': {'cluster': 'njvirt', + 'folder': 'coreinfra', + 'image': 'rhel6_64Guest', + 'memory': '8GB', + 'num_cpus': 2, + 'power_on': True, + 'profile': 'nj-vm', + 'provider': 'nj_vcenter:vmware', + 'resourcepool': 'Resources'}, + } EXAMPLE_MAP = { @@ -57,10 +85,19 @@ EXAMPLE_MAP = { 'devices': {'disk': {'Hard disk 1': {'size': 40}}, 'network': {'Network Adapter 1': {'mac': '22:4a:b2:92:b3:eb'}}}, 'memory': '16GB', + 'minion': { + 'master': 'mapmaster', + 'grains': { + 'mapgrain': True + } + }, 'name': 'db1'}, 'db2': {'name': 'db2', 'password': '456', - 'provider': 'nj_vcenter:vmware'}} + 'provider': 'nj_vcenter:vmware'}}, + 'nj-vm': {'db3': {'name': 'db3', + 'password': '789', + }} } @@ -96,6 +133,10 @@ class MapConfTest(TestCase): 'folder': 'coreinfra', 'image': 'rhel6_64Guest', 'memory': '16GB', + 'minion': {'grains': {'mapgrain': True, + 'profilegrain': True, + 'providergrain': True}, + 'master': 'mapmaster'}, 'name': 'db1', 'num_cpus': 2, 'password': '123456', @@ -105,31 +146,58 @@ class MapConfTest(TestCase): 'resourcepool': 'Resources', 'url': 'vca1.saltstack.com', 'user': 'root'}, - 'db2': {'cluster': 'nycvirt', - 'datastore': 'datastore1', - 'devices': {'disk': {'Hard disk 1': {'controller': 'SCSI controller 1', - 'size': 20}}, - 'network': {'Network Adapter 1': {'mac': '88:88:88:88:88:42', - 'name': 'vlan50', - 'switch_type': 'standard'}}, - 'scsi': {'SCSI controller 1': {'type': 'paravirtual'}}}, - 'driver': 'vmware', - 'extra_config': {'mem.hotadd': 'yes'}, - 'folder': 'coreinfra', - 'image': 'rhel6_64Guest', - 'memory': '8GB', - 'name': 'db2', - 'num_cpus': 2, - 'password': '456', - 'power_on': True, - 'profile': 'nyc-vm', - 'provider': 'nj_vcenter:vmware', - 'resourcepool': 'Resources', - 'url': 'vca2.saltstack.com', - 'user': 'root'}} + 'db2': {'cluster': 'nycvirt', + 'datastore': 'datastore1', + 'devices': {'disk': {'Hard disk 1': {'controller': 'SCSI controller 1', + 'size': 20}}, + 'network': {'Network Adapter 1': {'mac': '88:88:88:88:88:42', + 'name': 'vlan50', + 'switch_type': 'standard'}}, + 'scsi': {'SCSI controller 1': {'type': 'paravirtual'}}}, + 'driver': 'vmware', + 'extra_config': {'mem.hotadd': 'yes'}, + 'folder': 'coreinfra', + 'image': 'rhel6_64Guest', + 'memory': '8GB', + 'minion': {'grains': {'profilegrain': True, + 'providergrain': True}, + 'master': 'profilemaster'}, + 'name': 'db2', + 'num_cpus': 2, + 'password': '456', + 'power_on': True, + 'profile': 'nyc-vm', + 'provider': 'nj_vcenter:vmware', + 'resourcepool': 'Resources', + 'url': 'vca2.saltstack.com', + 'user': 'root'}, + 'db3': {'cluster': 'njvirt', + 'driver': 'vmware', + 'folder': 'coreinfra', + 'image': 'rhel6_64Guest', + 'memory': '8GB', + 'minion': {'grains': {'providergrain': True}, + 'master': 'providermaster'}, + 'name': 'db3', + 'num_cpus': 2, + 'password': '789', + 'power_on': True, + 'profile': 'nj-vm', + 'provider': 'nj_vcenter:vmware', + 'resourcepool': 'Resources', + 'url': 'vca2.saltstack.com', + 'user': 'root'}} } + # what we assert above w.r.t db2 using nj_vcenter:vmware provider: # - url is from the overriden nj_vcenter provider, not nyc_vcenter # - image from provider is still overridden by the nyc-vm profile # - password from map override is still overriding both the provider and profile password - self.assertEqual(cloud_map.map_data(), merged_profile) + # + # what we assert above about grain handling ( and provider/profile/map data in general ) + # - provider grains are able to be overridden by profile data + # - provider grain sare overridden by map data + # - profile data is overriden by map data + # ie, the provider->profile->map inheritance works as expected + map_data = cloud_map.map_data() + self.assertEqual(map_data, merged_profile)