diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index d3c6352c7..55510df57 100644 --- a/endpoints/api/tag.py +++ b/endpoints/api/tag.py @@ -112,16 +112,13 @@ class RepositoryTag(RepositoryParamResource): model.tag.create_or_update_tag(namespace, repository, tag, image_id) username = get_authenticated_user().username - tag_data = {'username': username, 'repo': repository, 'tag': tag, 'namespace': namespace, - 'image': image_id, 'original_image': original_image_id} + log_action('move_tag' if original_image_id else 'create_tag', namespace, + {'username': username, 'repo': repository, 'tag': tag, + 'namespace': namespace, 'image': image_id, + 'original_image': original_image_id}, + repo=model.repository.get_repository(namespace, repository)) - log_action('move_tag' if original_image_id else 'create_tag', namespace, tag_data, - repo=model.repository.get_repository(namespace, repository)) - try: - _generate_and_store_manifest(namespace, repository, tag) - except: - # log and move on since we'll attempt to store manifest on first pull as well - logger.exception('unable to store manifest for tag', extra=tag_data) + _generate_and_store_manifest(namespace, repository, tag) return 'Updated', 201 @@ -244,12 +241,7 @@ class RestoreTag(RepositoryParamResource): existing_image = model.tag.restore_tag_to_manifest(repo, tag, manifest_digest) else: existing_image = model.tag.restore_tag_to_image(repo, tag, image_id) - try: - _generate_and_store_manifest(namespace, repository, tag) - except: - # log and move on since we'll attempt to store manifest on first pull as well - logger.exception('unable to store manifest for tag', extra=log_data) - + _generate_and_store_manifest(namespace, repository, tag) if existing_image is not None: log_data['original_image'] = existing_image.docker_image_id diff --git a/endpoints/api/test/test_tag.py b/endpoints/api/test/test_tag.py index ea8c09b0e..01e1eb22f 100644 --- a/endpoints/api/test/test_tag.py +++ b/endpoints/api/test/test_tag.py @@ -32,7 +32,7 @@ def create_or_update_tag(): @pytest.fixture() def generate_manifest(): def mock_callable(namespace, repository, tag): - if tag is 'generatemanifestfail': + if tag == 'generatemanifestfail': raise Exception('test_failure') with patch('endpoints.api.tag._generate_and_store_manifest', side_effect=mock_callable) as mk: yield mk @@ -47,11 +47,15 @@ def authd_client(client): ('image1', '.INVALID-TAG-NAME', 400), ('image1', 'INVALID-TAG_NAME-BECAUSE-THIS-IS-WAY-WAY-TOO-LOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOONG', 400), ('nonexistantimage', 'newtag', 404), - ('image1', 'generatemanifestfail', 201), + ('image1', 'generatemanifestfail', None), ('image1', 'existing-tag', 201), ('image1', 'newtag', 201), ]) def test_move_tag(test_image, test_tag, expected_status, get_repo_image, get_repo_tag_image, create_or_update_tag, generate_manifest, authd_client): params = {'repository': 'devtable/repo', 'tag': test_tag} request_body = {'image': test_image} - conduct_api_call(authd_client, RepositoryTag, 'put', params, request_body, expected_status) + if expected_status is None: + with pytest.raises(Exception): + conduct_api_call(authd_client, RepositoryTag, 'put', params, request_body, expected_status) + else: + conduct_api_call(authd_client, RepositoryTag, 'put', params, request_body, expected_status)