From d2630fa726dc24899057be4482049c6b2e3cc5ba Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Mon, 27 Feb 2017 13:19:29 +1300 Subject: [PATCH] Fix long resource id issue - cherry-pick https://review.openstack.org/#/c/434582/ - add unit test - fix drop_all() method in db api Change-Id: I9ca994b3d9b69628b4fc72e14cc0770e1ffe5524 --- distil/collector/base.py | 27 +++++- distil/db/sqlalchemy/api.py | 2 +- distil/tests/etc/distil.conf | 2 +- distil/tests/etc/meter_mappings.yaml | 10 +++ distil/tests/etc/transformer.yaml | 22 +++++ distil/tests/unit/service/__init__.py | 0 distil/tests/unit/service/test_collector.py | 99 +++++++++++++++++++++ test-requirements.txt | 1 + 8 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 distil/tests/etc/meter_mappings.yaml create mode 100644 distil/tests/etc/transformer.yaml create mode 100644 distil/tests/unit/service/__init__.py create mode 100644 distil/tests/unit/service/test_collector.py diff --git a/distil/collector/base.py b/distil/collector/base.py index ec6f874..ce76a96 100644 --- a/distil/collector/base.py +++ b/distil/collector/base.py @@ -14,6 +14,7 @@ import abc from datetime import timedelta +import hashlib import re import yaml @@ -86,6 +87,7 @@ class BaseCollector(object): # time of project within one session. db_api.usages_add(project['id'], resources, usage_entries, window_end) + LOG.info('Finish project %s(%s) slice %s %s', project['id'], project['name'], window_start, window_end) except Exception as e: @@ -171,8 +173,12 @@ class BaseCollector(object): if resource_type == 'Object Storage Container': # NOTE(flwang): It's safe to get container name by /, since # Swift doesn't allow container name with /. - idx = resource_id.index('/') + 1 - resource_info['name'] = resource_id[idx:] + # NOTE(flwang): Instead of using the resource_id from the + # input parameters, here we use the original resource id from + # the entry. Because the resource_id has been hashed(MD5) to + # avoid too long. + idx = entry['resource_id'].index('/') + 1 + resource_info['name'] = entry['resource_id'][idx:] return resource_info @@ -195,6 +201,23 @@ class BaseCollector(object): if transformed: res_id = mapping.get('res_id_template', '%s') % res_id + + # NOTE(flwang): Currently the column size of resource id in DB + # is 100 chars, but the container name of swift could be 256, + # plus project id and a '/', the id for a swift container + # could be 32+1+256. So this is a fix for the problem. But + # instead of checking the length of resource id, here I'm + # hashing the name only for swift to get a consistent + # id for swift billing. Another change will be proposed to + # openstack-billing to handle this case as well. + if 'o1.standard' in transformed: + res_id = hashlib.md5(res_id.encode('utf-8')).hexdigest() + + LOG.debug( + 'After transformation, usage for resource %s: %s' % + (res_id, transformed) + ) + res_info = self._get_resource_info( project_id, res_id, diff --git a/distil/db/sqlalchemy/api.py b/distil/db/sqlalchemy/api.py index 8059a64..797bea2 100644 --- a/distil/db/sqlalchemy/api.py +++ b/distil/db/sqlalchemy/api.py @@ -88,7 +88,7 @@ def setup_db(): def drop_db(): try: engine = get_engine() - m.Cluster.metadata.drop_all(engine) + m.Tenant.metadata.drop_all(engine) except Exception as e: LOG.exception("Database shutdown exception: %s", e) return False diff --git a/distil/tests/etc/distil.conf b/distil/tests/etc/distil.conf index 5318c2b..35d3306 100644 --- a/distil/tests/etc/distil.conf +++ b/distil/tests/etc/distil.conf @@ -24,7 +24,7 @@ trusted_sources = openstack type = odoo [database] -connection = mysql://root:passw0rd@127.0.0.1/distil?charset=utf8 +connection = sqlite:// backend = sqlalchemy [keystone_authtoken] diff --git a/distil/tests/etc/meter_mappings.yaml b/distil/tests/etc/meter_mappings.yaml new file mode 100644 index 0000000..0e23d53 --- /dev/null +++ b/distil/tests/etc/meter_mappings.yaml @@ -0,0 +1,10 @@ +- + meter: storage.containers.objects.size + service: o1.standard + type: Object Storage Container + transformer: max + unit: byte + metadata: + name: + sources: + - name diff --git a/distil/tests/etc/transformer.yaml b/distil/tests/etc/transformer.yaml new file mode 100644 index 0000000..86c72ac --- /dev/null +++ b/distil/tests/etc/transformer.yaml @@ -0,0 +1,22 @@ +uptime: + tracked_states: + - active + - paused + - rescue + - rescued + - resize + - resized + - verify_resize + - suspended + - shutoff + - stopped +from_image: + service: b1.standard + md_keys: + - image_ref + - image_meta.base_image_ref + none_values: + - None + - "" + size_keys: + - root_gb diff --git a/distil/tests/unit/service/__init__.py b/distil/tests/unit/service/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/distil/tests/unit/service/test_collector.py b/distil/tests/unit/service/test_collector.py new file mode 100644 index 0000000..088e791 --- /dev/null +++ b/distil/tests/unit/service/test_collector.py @@ -0,0 +1,99 @@ +# Copyright (C) 2017 Catalyst IT Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from datetime import datetime +import hashlib +import json +import os + +import mock + +from distil.collector import base as collector_base +from distil.db.sqlalchemy import api as db_api +from distil.tests.unit import base + + +class CollectorTest(base.DistilWithDbTestCase): + def setUp(self): + super(CollectorTest, self).setUp() + + meter_mapping_file = os.path.join( + os.environ["DISTIL_TESTS_CONFIGS_DIR"], + 'meter_mappings.yaml' + ) + self.conf.set_default( + 'meter_mappings_file', + meter_mapping_file, + group='collector' + ) + + transformer_file = os.path.join( + os.environ["DISTIL_TESTS_CONFIGS_DIR"], + 'transformer.yaml' + ) + self.conf.set_default( + 'transformer_file', + transformer_file, + group='collector' + ) + + @mock.patch('distil.collector.base.BaseCollector.get_meter') + def test_collect_swift_resource_id(self, mock_get_meter): + project_id = 'fake_project_id' + project_name = 'fake_project' + project = {'id': project_id, 'name': project_name} + start_time = datetime.strptime( + '2017-02-27 00:00:00', + "%Y-%m-%d %H:%M:%S" + ) + end_time = datetime.strptime( + '2017-02-27 01:00:00', + "%Y-%m-%d %H:%M:%S" + ) + + # Add project to db in order to satisfy the foreign key constraint of + # UsageEntry + db_api.project_add( + { + 'id': project_id, + 'name': 'fake_project', + 'description': 'project for test' + } + ) + + container_name = 'my_container' + resource_id = '%s/%s' % (project_id, container_name) + resource_id_hash = hashlib.md5(resource_id.encode('utf-8')).hexdigest() + + mock_get_meter.return_value = [ + { + 'resource_id': resource_id, + 'source': 'openstack', + 'volume': 1024 + } + ] + + collector = collector_base.BaseCollector() + collector.collect_usage(project, start_time, end_time) + + resources = db_api.resource_get_by_ids(project_id, [resource_id_hash]) + res_info = json.loads(resources[0].info) + + self.assertEquals(1, len(resources)) + self.assertEquals(container_name, res_info['name']) + + entries = db_api.usage_get(project_id, start_time, end_time) + + self.assertEquals(1, len(entries)) + self.assertEquals(resource_id_hash, entries[0].resource_id) diff --git a/test-requirements.txt b/test-requirements.txt index aef5d3d..d367a3d 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -12,3 +12,4 @@ testrepository>=0.0.18 testscenarios>=0.4 testtools>=0.9.34 WebTest>=2.0 # MIT +mock>=2.0 # BSD