Skip to content

Commit

Permalink
Set extended_hash vale to None instead of 'None'
Browse files Browse the repository at this point in the history 
Sometimes extended_hash value is getting set to 'None'
instead of None. This leads to commit_hash + distro_hash
+ extended_hash combination not found error. Added test
cases for extended hash.

Added hash validation functions, which checks that hash
is valid or not. This change forced to add valid hashes
to the test cases.

Change-Id: I1346ce23daef1f95119b9cc58c26d9165e5298d0
Signed-off-by: Amol Kahat <amolkahat@gmail.com>
  • Loading branch information
@amolkahat
amolkahat authored and Zuul CI committed Jun 4, 2021
1 parent 6c23507 commit 6e9a9be
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 72 deletions.
62 changes: 50 additions & 12 deletions ci-scripts/dlrnapi_promoter/dlrn_hash.py
View file
Expand Up @@ -11,6 +11,35 @@
import logging


def check_hash(hash_value):
"""
This function will check the hash function properties like
hash size, valid hash etc.
"""
if hash_value is not None:
try:
int(hash_value, 16)
len(hash_value) == 40
except (TypeError, ValueError):
raise DlrnHashError("Invalid hash format!!")


def check_extended_hash(extended_hash):
"""
This function will check valid extended hash.
"""
if extended_hash is not None and extended_hash not in ['None', '']:
try:
e_hash = extended_hash.split("_")
if len(e_hash) == 2:
check_hash(e_hash[0])
check_hash(e_hash[1])
else:
raise DlrnHashError("Invalid extended hash format")
except (TypeError, ValueError):
raise DlrnHashError("Invalid extended hash, not a hex hash!")


class DlrnHashError(Exception):
"""
Raised on various errors on DlrnHash operations
Expand Down Expand Up @@ -44,6 +73,10 @@ def __init__(self, commit_hash=None, distro_hash=None, extended_hash=None,
:param source: a dictionary with all the parameters as keys
:param component: the eventual component of the hash
"""
# Make sure extended_hash value should be None not "None"
if extended_hash in ['None', '']:
extended_hash = None

# Load from default values into unified source
_source = {
'commit_hash': commit_hash,
Expand Down Expand Up @@ -107,9 +140,11 @@ def sanity_check(self):
Checks if the basic components of the hash are present
component and timestamp are optional
"""
if self.commit_hash is None or self.distro_hash is None or \
self.extended_hash == '':
raise DlrnHashError("Invalid commit or distro or extended hash")
if self.commit_hash is None or self.distro_hash is None:
raise DlrnHashError("Invalid commit or distro hash")
check_hash(self.commit_hash)
check_hash(self.distro_hash)
check_extended_hash(self.extended_hash)

def __repr__(self):
"""
Expand All @@ -119,8 +154,8 @@ def __repr__(self):
"""
return ("<DlrnCommitDistroExtendedHash object commit: %s,"
" distro: %s, component: %s, extended: %s, timestamp: %s>"
"" % (self.commit_hash, self.distro_hash, self.extended_hash,
self.component, self.timestamp))
"" % (self.commit_hash, self.distro_hash, self.component,
self.extended_hash, self.timestamp))

def __str__(self):
"""
Expand All @@ -129,9 +164,9 @@ def __str__(self):
:return: The string representation of the hash informations
"""
return ("commit: %s, distro: %s, extended: %s, component: %s,"
"timestamp: %s"
"" % (self.commit_hash, self.distro_hash, self.extended_hash,
self.component, self.timestamp))
"timestamp: %s" % (self.commit_hash, self.distro_hash,
self.extended_hash, self.component,
self.timestamp))

def __ne__(self, other):
"""
Expand Down Expand Up @@ -179,7 +214,7 @@ def full_hash(self):
Work only with single norma dlrn haseh
:return: The full hash format or None
"""
if self.extended_hash not in [None, 'None']:
if self.extended_hash is not None:
ext_distro_hash, ext_commit_hash = self.extended_hash.split('_')
return '{0}_{1}_{2}_{3}'.format(self.commit_hash,
self.distro_hash[:8],
Expand Down Expand Up @@ -245,9 +280,12 @@ def sanity_check(self):
component and timestamp are optional
"""
if self.commit_hash is None or self.distro_hash is None or \
self.extended_hash == '' or self.aggregate_hash is None:
raise DlrnHashError("Invalid commit or distro or "
"extended aggregate hash")
self.aggregate_hash is None:
raise DlrnHashError("Invalid commit or distro or aggregate hash")
check_hash(self.commit_hash)
check_hash(self.distro_hash)
check_hash(self.aggregate_hash)
check_extended_hash(self.extended_hash)

def __repr__(self):
"""
Expand Down
43 changes: 21 additions & 22 deletions ci-scripts/dlrnapi_promoter/test_dlrn_client_unit.py
View file
Expand Up @@ -58,19 +58,22 @@ def setUp(self):

# Set up some ready to use hashes
self.dlrn_hash_commitdistro1 = DlrnCommitDistroExtendedHash(
commit_hash='a',
distro_hash='b',
component="comp1",
timestamp=1)
commit_hash='90633a3785687ddf3d37c0f86f9ad9f93926d639',
distro_hash='d68290fed3d9aa069c95fc16d0d481084adbadc6',
extended_hash='6137f83ab8defe688e70a18ef1c7e5bf3fbf02ef_'
'3945701fc2ae9b1b14e4261e87e203b2a89ccdca',
component="tripleo",
timestamp=1)
self.dlrn_hash_commitdistro2 = DlrnCommitDistroExtendedHash(
commit_hash='c',
distro_hash='d',
component="comp2",
timestamp=2)
self.dlrn_hash_aggregate = DlrnAggregateHash(commit_hash='abc',
distro_hash='def',
aggregate_hash='ghjk',
timestamp=1)
commit_hash='4f4774d4e410ce72b024c185d3054cf649e5c578',
distro_hash='fe88530aa04df13ebc63287c819c721740837aae',
component="tempest",
timestamp=2)
self.dlrn_hash_aggregate = DlrnAggregateHash(
commit_hash='98da7b0933a2975598844bf40edec4b61714db40',
distro_hash='c3a41aaf53b9ea10333387b7d40797ba2c1018d2',
aggregate_hash='26b9d4d1d8fd09cdc2b11c7dd0f71f93',
timestamp=1)
self.promote_log_header = ("Dlrn promote '{}' from {} to {}:"
"".format(self.dlrn_hash_commitdistro1,
'tripleo-ci-testing',
Expand Down Expand Up @@ -433,21 +436,20 @@ def setUp(self):
super(TestNamedHashes, self).setUp()
dlrn_start_hash_dict = {
'timestamp': '1528085427',
'commit_hash': 'd221f4b33cf2763875fc6394902f7923108a34da',
'distro_hash': '70bdcd40eb5cc62e4762a7db0086e09f6edf2e5c'
'commit_hash': '326452e5851e8347b15b53c3d6b70e6f5225f3ea',
'distro_hash': '589b556babb2d0c5c6e79d5c2a505341b70ef370'
}
dlrn_changed_hash_dict = {
'timestamp': '1528085529',
'commit_hash': 'e3d9fffbf82ec71deff60ba914f1db0e1625466a',
'distro_hash': 'Iba78e857267ac771d23919fbd1e3c9fcc5813c9'
'commit_hash': '6b3bf3bba01055ca8e544ce258b44e4f5da3da34',
'distro_hash': '6aaa73f4925b38ae77d468257bced8d3baf8dd97'
}
self.dlrn_changed_hash = DlrnHash(source=dlrn_changed_hash_dict)
self.dlrn_start_hash = DlrnHash(source=dlrn_start_hash_dict)

@patch('logging.Logger.error')
@patch('dlrn_client.DlrnClient.fetch_hashes')
def test_named_hashes_unchanged(self, mock_fetch_hashes, mock_log_err):

mock_fetch_hashes.side_effect = [self.dlrn_start_hash,
self.dlrn_start_hash]
# positive test for hashes_unchanged
Expand Down Expand Up @@ -506,7 +508,7 @@ def test_fetch_current_named_hash_no_store(self, mock_fetch_hashes,
@patch('logging.Logger.debug')
@patch('dlrn_client.DlrnClient.fetch_hashes')
def test_update_current_named_hash(
self, mock_fetch_hashes, mock_log_debug):
self, mock_fetch_hashes, mock_log_debug):
mock_fetch_hashes.side_effect = [
self.dlrn_changed_hash, self.dlrn_changed_hash
]
Expand All @@ -523,7 +525,7 @@ def test_update_current_named_hash(
@patch('logging.Logger.debug')
@patch('dlrn_client.DlrnClient.fetch_promotions')
def test_fetch_current_named_hashes_no_hashes(
self, fetch_promotions_mock, mock_log_debug, mock_log_warn):
self, fetch_promotions_mock, mock_log_debug, mock_log_warn):
fetch_promotions_mock.return_value = []
self.client.fetch_current_named_hashes()
self.assertFalse(mock_log_debug.called)
Expand All @@ -543,7 +545,6 @@ def test_get_promotion_aggregate_hashes_success(self,
mock_log_debug,
mock_log_info,
mock_log_error):

self.maxDiff = None
delorean_repo_path, tmp_dir = self.get_tmp_delorean_repo()
# Extremely important that we ensure this method does not produce
Expand Down Expand Up @@ -594,7 +595,6 @@ def test_get_promotion_aggregate_hashes_no_components(self,
mock_log_debug,
mock_log_info,
mock_log_error):

delorean_repo_path, tmp_dir = self.get_tmp_delorean_repo(empty=True)
with self.assertRaises(PromotionError):
self.client.get_promotion_aggregate_hashes("",
Expand All @@ -614,7 +614,6 @@ def test_get_promotion_aggregate_hashes_no_components(self,
def test_get_promotion_aggregate_url_error(self,
get_hash_mock,
mock_log_error):

self.config.repo_url = "file:///not.exist"
with self.assertRaises(PromotionError):
self.client.get_promotion_aggregate_hashes("",
Expand Down
64 changes: 54 additions & 10 deletions ci-scripts/dlrnapi_promoter/test_dlrn_hash_unit.py
View file
Expand Up @@ -12,20 +12,52 @@
from test_unit_fixtures import hashes_test_cases


def get_commit_dir(hash_dict, hash_type="commitdistro"):
if isinstance(hash_dict, dict):
commit_hash = hash_dict.get("commit_hash")
distro_hash = hash_dict.get("distro_hash")
else:
commit_hash = hash_dict.commit_hash
distro_hash = hash_dict.distro_hash
if hash_type == 'commitdistro':
try:
extended_hash = hash_dict.extended_hash
except (AttributeError):
extended_hash = hash_dict.get('extended_hash')
e_hash = '_'
if extended_hash:
e_d_hash, e_c_hash = extended_hash.split("_")
e_hash += "{}_{}".format(e_d_hash[:8], e_c_hash[:8])

return "{}/{}/{}_{}{}".format(commit_hash[:2], commit_hash[2:4],
commit_hash, distro_hash[:8], e_hash)
elif hash_type == "aggregate":
try:
aggregate_hash = hash_dict.aggregate_hash
except (AttributeError):
aggregate_hash = hash_dict.get("aggregate_hash")

return "{}/{}/{}".format(aggregate_hash[:2], aggregate_hash[2:4],
aggregate_hash)


class TestDlrnHashSubClasses(unittest.TestCase):

def test_build_valid(self):
for hash_type, source_types in hashes_test_cases.items():
values = source_types['dict']['valid']
if hash_type == "commitdistro":
dh = DlrnCommitDistroExtendedHash(
commit_hash=values['commit_hash'],
distro_hash=values['distro_hash'],
timestamp=values['timestamp'])
commit_hash=values['commit_hash'],
distro_hash=values['distro_hash'],
extended_hash=values['extended_hash'],
timestamp=values['timestamp'])
self.assertEqual(dh.commit_hash,
source_types['dict']['valid']['commit_hash'])
self.assertEqual(dh.distro_hash,
source_types['dict']['valid']['distro_hash'])
self.assertEqual(dh.extended_hash,
source_types['dict']['valid']['extended_hash'])
elif hash_type == "aggregate":
aggregate_hash = source_types['dict']['valid'][
'aggregate_hash']
Expand All @@ -46,6 +78,8 @@ def test_build_valid_from_source(self):
source_types['dict']['valid']['commit_hash'])
self.assertEqual(dh.distro_hash,
source_types['dict']['valid']['distro_hash'])
self.assertEqual(dh.extended_hash,
source_types['dict']['valid']['extended_hash'])
elif hash_type == "aggregate":
aggregate_hash = source_types['dict']['valid'][
'aggregate_hash']
Expand Down Expand Up @@ -117,8 +151,10 @@ def test_properties(self):
source = source_types['object']['valid']
dh = DlrnHash(source=source)
if hash_type == "commitdistro":
full_hash = "{}_{}".format(source.commit_hash,
source.distro_hash[:8])
e_d_hash, e_c_hash = source.extended_hash.split("_")
full_hash = "{}_{}_{}_{}".format(source.commit_hash,
source.distro_hash[:8],
e_d_hash[:8], e_c_hash[:8])
self.assertEqual(dh.full_hash, full_hash)
elif hash_type == "aggregate":
self.assertEqual(dh.full_hash, source.aggregate_hash)
Expand All @@ -139,23 +175,31 @@ def test_commit_dir(self):
for hash_type, source_types in hashes_test_cases.items():
dh = DlrnHash(source=source_types['object']['valid'])
if hash_type == "commitdistro":
self.assertEqual(dh.commit_dir, "ab/cd/abcd_defg")
commit_dir = get_commit_dir(source_types['object'][
'valid'])
self.assertEqual(dh.commit_dir, commit_dir)
elif hash_type == "aggregate":
dh.label = "label"
self.assertEqual(dh.commit_dir, "label/ab/cd/abcd")
commit_dir = get_commit_dir(source_types['object']['valid'],
"aggregate")
self.assertEqual(dh.commit_dir, "label/{}".format(commit_dir))

def test_commit_dir_no_label(self):
for hash_type, source_types in hashes_test_cases.items():
dh = DlrnHash(source=source_types['object']['valid'])
if hash_type == "commitdistro":
self.assertEqual(dh.commit_dir, "ab/cd/abcd_defg")
commit_dir = get_commit_dir(source_types['object']['valid'])
self.assertEqual(dh.commit_dir, commit_dir)
elif hash_type == "aggregate":
self.assertEqual(dh.commit_dir, "ab/cd/abcd")
commit_dir = get_commit_dir(source_types['object']['valid'],
'aggregate')
self.assertEqual(dh.commit_dir, commit_dir)

def test_commit_dir_component(self):
for hash_type, source_types in hashes_test_cases.items():
dh = DlrnHash(source=source_types['object']['valid'])
if hash_type == "commitdistro":
dh.component = "component1"
commit_dir = get_commit_dir(source_types['object']['valid'])
self.assertEqual(dh.commit_dir,
"component/component1/ab/cd/abcd_defg")
"component/component1/{}".format(commit_dir))
17 changes: 12 additions & 5 deletions ci-scripts/dlrnapi_promoter/test_qcow_client_unit.py
View file
Expand Up @@ -103,12 +103,17 @@ def setUp(self):

self.images_root = self.client.root
self.images_dir = self.client.images_dir
self.previous_hash_dir = os.path.join(self.images_dir, "efgh")
self.previous_hash_dir = os.path.join(
self.images_dir,
DlrnHash(source=hashes_test_cases['aggregate']['object'][
'valid_notimestamp']).full_hash)
self.current_hash_dir = os.path.join(self.images_dir, "dunno")
self.candidate_hash_dir = os.path.join(self.images_dir, "abcd")
self.candidate_hash_dir = os.path.join(
self.images_dir,
DlrnHash(source=hashes_test_cases['aggregate']['object'][
'valid']).full_hash)
self.target_label = "test-label"
self.previous_target_label = "previous-{}".format(self.target_label)

try:
os.makedirs(self.candidate_hash_dir)
except FileExistsError:
Expand Down Expand Up @@ -138,7 +143,6 @@ def test_promote_success_no_previous(self,
mock_log_error):
self.client.promote(self.valid_candidate_hash, self.target_label,
create_previous=False, validation=False)

promotion_link = os.path.join(self.images_dir, self.target_label)
check_links(os, promotion_link, "test", self.candidate_hash_dir)

Expand All @@ -159,7 +163,10 @@ def test_promote_success_previous(self,

promotion_link = os.path.join(self.images_dir, self.target_label)
previous_link = os.path.join(self.images_dir, "previous-test-label")
previous_dir = os.path.join(self.images_dir, "efgh")
previous_dir = os.path.join(
self.images_dir,
DlrnHash(source=hashes_test_cases['aggregate']['object'][
'valid_notimestamp']).full_hash)
check_links(os, promotion_link, "test", self.candidate_hash_dir,
previous_link=previous_link, previous_dir=previous_dir)
self.assertFalse(mock_log_error.called)
Expand Down

0 comments on commit 6e9a9be

Please sign in to comment.