From eb5f74b3c96ba25f507b998212037a545053ea48 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Oct 06 2017 21:16:03 +0000 Subject: add a default option to check_volume_policy --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 02357d4..5cc1b33 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4792,11 +4792,14 @@ def _set_build_volume(binfo, volinfo, strict=True): koji.plugin.run_callbacks('postBuildStateChange', attribute='volume_id', old=old_binfo['volume_id'], new=volinfo['id'], info=binfo) -def check_volume_policy(data, strict=False): +def check_volume_policy(data, strict=False, default=None): """Check volume policy for the given data - If strict is True, raises exception on bad policies or no matches - Returns volume info, or None if no match + If strict is True, raises exception when a volume cannot be determined. + The default option can either be None, or a valid volume id or name, and + is used when the policy rules do not return a match. + + Returns volume info or None """ result = None try: @@ -4808,19 +4811,28 @@ def check_volume_policy(data, strict=False): raise tb_str = ''.join(traceback.format_exception(*sys.exc_info())) logger.debug(tb_str) - if result is None: - if strict: - raise koji.GenericError('No volume policy match') - logger.warn('No volume policy match') - return None logger.debug('Volume policy returned %s', result) - vol = lookup_name('volume', result) - if not vol: + if result is not None: + vol = lookup_name('volume', result) + if vol: + return vol + # otherwise if strict: - raise koji.GenericError("Policy returned invalid volume: %s" % result) + raise koji.GenericError("Policy returned invalid volume: %s" + % result) logger.error('Volume policy returned unknown volume %s', result) - return None - return vol + # fall back to default + if default is not None: + vol = lookup_name('volume', default) + if vol: + return vol + if strict: + raise koji.GenericError("Invalid default volume: %s" % default) + logger.error('Invalid default volume: %s', default) + if strict: + raise koji.GenericError('No volume policy match') + logger.warn('No volume policy match') + return None def apply_volume_policy(build, strict=False): @@ -5035,7 +5047,7 @@ def import_build(srpm, rpms, brmap=None, task_id=None, build_id=None, logs=None) 'import': True, 'import_type': 'rpm', } - vol = check_volume_policy(policy_data, strict=False) + vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol: build['volume_id'] = vol['id'] build['volume_name'] = vol['name'] @@ -5294,7 +5306,7 @@ class CG_Importer(object): 'import': True, 'import_type': 'cg', } - vol = check_volume_policy(policy_data, strict=False) + vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol: self.buildinfo['volume_id'] = vol['id'] self.buildinfo['volume_name'] = vol['name'] @@ -11796,7 +11808,7 @@ class HostExports(object): 'import': True, 'import_type': 'maven', } - vol = check_volume_policy(policy_data, strict=False) + vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] build_info['volume_name'] = vol['name'] @@ -11875,7 +11887,7 @@ class HostExports(object): 'import': True, 'import_type': 'maven', } - vol = check_volume_policy(policy_data, strict=False) + vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] build_info['volume_name'] = vol['name'] @@ -12042,7 +12054,7 @@ class HostExports(object): 'import': True, 'import_type': 'win', } - vol = check_volume_policy(policy_data, strict=False) + vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] build_info['volume_name'] = vol['name'] diff --git a/tests/test_hub/test_check_volume_policy.py b/tests/test_hub/test_check_volume_policy.py index 081cb43..e8c9843 100644 --- a/tests/test_hub/test_check_volume_policy.py +++ b/tests/test_hub/test_check_volume_policy.py @@ -75,6 +75,37 @@ class TestCheckVolumePolicy(unittest.TestCase): self.lookup_name.assert_not_called() + def test_volume_policy_default(self): + self.load_policy({'volume': 'none :: othervol'}) + data = {} + self.lookup_name.return_value = mock.sentinel.volume_info + + ret = kojihub.check_volume_policy(data, default='myvol') + self.lookup_name.assert_called_once_with('volume', 'myvol') + self.assertEqual(ret, mock.sentinel.volume_info) + + # and again with strict on + self.lookup_name.reset_mock() + ret = kojihub.check_volume_policy(data, strict=True, default='myvol') + self.lookup_name.assert_called_once_with('volume', 'myvol') + self.assertEqual(ret, mock.sentinel.volume_info) + + def test_volume_policy_bad_default(self): + self.load_policy({'volume': 'none :: othervol'}) + data = {} + self.lookup_name.return_value = None + + ret = kojihub.check_volume_policy(data, default='badvol') + self.lookup_name.assert_called_once_with('volume', 'badvol') + self.assertEqual(ret, None) + + # and again with strict on + self.lookup_name.reset_mock() + with self.assertRaises(koji.GenericError): + kojihub.check_volume_policy(data, strict=True, default='badvol') + self.lookup_name.assert_called_once_with('volume', 'badvol') + self.assertEqual(ret, None) + def test_volume_policy_badvolume(self): self.load_policy({'volume': 'bool foo :: testvol'}) data = {'foo': True} @@ -90,6 +121,24 @@ class TestCheckVolumePolicy(unittest.TestCase): kojihub.check_volume_policy(data, strict=True) self.lookup_name.assert_called_once_with('volume', 'testvol') + def test_volume_policy_badvolume_with_default(self): + self.load_policy({'volume': 'all :: badvol'}) + data = {} + def my_lookup_name(table, info): + self.assertEqual(table, 'volume') + if info == 'goodvol': + return mock.sentinel.good_volume + else: + return None + self.lookup_name.side_effect = my_lookup_name + + ret = kojihub.check_volume_policy(data, default='goodvol') + self.assertEqual(ret, mock.sentinel.good_volume) + + # and again with strict on + with self.assertRaises(koji.GenericError): + kojihub.check_volume_policy(data, strict=True, default='goodvol') + def test_volume_policy_good(self): self.load_policy({'volume': 'bool foo :: testvol'}) data = {'foo': True}