-
Notifications
You must be signed in to change notification settings - Fork 19
Bug 1470942 - Support maven on S3 #163
Changes from all commits
d9df485
e1b61ec
76aa949
7b5901d
262e9b2
7943a29
2747d97
f8ef32d
0a4c86b
5c292ba
9f55e9c
3f850cc
b638c20
a05f3c2
a58f7a4
381d845
41382a7
af1f061
abbabd3
215ddb7
23b85d8
d9ed157
54b5368
39294f9
0985fe8
570e4e5
e461f6a
54a6fde
32ecbe3
d69ae80
2b66aa8
44a526a
8c88f83
f98517f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,7 +56,7 @@ | |
"type" : "string" | ||
} | ||
}, | ||
"required" : ["appName", "buildid", "appVersion", "hashType", "platform", "branch"] | ||
"required" : ["appName", "buildid", "appVersion", "branch"] | ||
}, | ||
"upstreamArtifacts": { | ||
"type": "array", | ||
|
@@ -79,9 +79,12 @@ | |
"items": { | ||
"type": "string" | ||
} | ||
}, | ||
"zipExtract": { | ||
"type": "boolean" | ||
} | ||
}, | ||
"required": ["taskId", "taskType", "paths", "locale"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume you remove 'locale' to fit the maven jobs right? The old beetmover jobs are still having the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that's what this change is about. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
"required": ["taskId", "taskType", "paths"] | ||
}, | ||
"minItems": 1, | ||
"uniqueItems": true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,9 +46,12 @@ | |
"items": { | ||
"type": "string" | ||
} | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: in the near future, in my backlog, I have a task to add integration tests for beetmover in such a way that we have beetmover schema for all possible beetmover jobs + actual tests against them. Like we currently have in bouncerscript or shipitscript. Right now we are reusing these two possible tasks but it's not enough. |
||
"zipExtract": { | ||
"type": "boolean" | ||
} | ||
}, | ||
"required": ["taskId", "taskType", "paths", "locale"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why trim There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. geckoview tasks don't specify a locale to beetmove. That's why I removed it from the required fields. I'm fine putting it back and creating a different schema. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is my fault from the very beginnings of beetmoverscript. In Again, this is not blocking so we can definitely follow-up in a separate PR. I have a backlog task to take over (hopefully) within declarative artifacts, to rewrite all the tasks to have integration tests for all possible beetmover jobs so this work will be undertaken anyway at some point this quarter so not sure it's worth time investing now. If it's easy for you to tweak tasks to fit to a new schema and not touch existing, that's be ideal, but again, we're going to do that sooner or later anyway for all tasks. Up to you, am fine either way 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got! I'd prefer to make it in a follow up. I think we're safe for now because beetmoverscript relies on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
"required": ["taskId", "taskType", "paths"] | ||
}, | ||
"minItems": 0, | ||
"uniqueItems": true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import os | ||
|
||
_MAVEN_ZIP_NAME = 'target.maven.zip' | ||
|
||
|
||
def get_maven_expected_files_per_archive_per_task_id(upstream_artifacts_per_task_id, mapping_manifest): | ||
task_id, maven_zip_full_path = _get_task_id_and_full_path_of_maven_archive(upstream_artifacts_per_task_id) | ||
|
||
return { | ||
task_id: { | ||
maven_zip_full_path: _get_maven_expected_files_in_archive(mapping_manifest) | ||
} | ||
} | ||
|
||
|
||
def _get_task_id_and_full_path_of_maven_archive(upstream_artifacts_per_task_id): | ||
candidate_task_id = '' | ||
candidate_path = '' | ||
|
||
for task_id, upstream_definitions in upstream_artifacts_per_task_id.items(): | ||
for upstream_definition in upstream_definitions: | ||
for path in upstream_definition['paths']: | ||
if path.endswith(_MAVEN_ZIP_NAME): | ||
if candidate_task_id: | ||
raise ValueError( | ||
'Too many upstream artifact ending with "{}" found: ({}, {}) and ({}, {})'.format( | ||
_MAVEN_ZIP_NAME, candidate_task_id, candidate_path, task_id, path | ||
) | ||
) | ||
|
||
candidate_task_id = task_id | ||
candidate_path = path | ||
|
||
if not candidate_task_id: | ||
raise ValueError('No upstream artifact ending with "{}" found. Given: {}'.format( | ||
_MAVEN_ZIP_NAME, upstream_artifacts_per_task_id) | ||
) | ||
|
||
return candidate_task_id, candidate_path | ||
|
||
|
||
def _get_maven_expected_files_in_archive(mapping_manifest): | ||
files = mapping_manifest['mapping']['en-US'].keys() | ||
return [ | ||
os.path.join( | ||
_remove_first_directory_from_bucket(mapping_manifest['s3_bucket_path']), | ||
file | ||
) for file in files | ||
] | ||
|
||
|
||
def _remove_first_directory_from_bucket(s3_bucket_path): | ||
# remove 'maven2' because it's not in the archive, but it exists on the maven server | ||
return '/'.join(s3_bucket_path.split('/')[1:]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
RESTRICTED_BUCKET_PATHS, | ||
CHECKSUMS_CUSTOM_FILE_NAMING | ||
) | ||
from scriptworker import artifacts as scriptworker_artifacts | ||
from scriptworker.exceptions import ScriptWorkerTaskException | ||
|
||
log = logging.getLogger(__name__) | ||
|
@@ -109,29 +110,45 @@ def add_balrog_manifest_to_artifacts(context): | |
utils.write_json(abs_file_path, context.balrog_manifest) | ||
|
||
|
||
def get_upstream_artifact(context, taskid, path): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah, nice cleanup, didn't know of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just realize I implemented it a while ago in mozilla-releng/scriptworker#95. I knew beetmover had its own logic, but I didn't cc you on it. Sorry about this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, absolutely no worries, this is super nice! 👍 |
||
abs_path = os.path.abspath(os.path.join(context.config['work_dir'], 'cot', taskid, path)) | ||
if not os.path.exists(abs_path): | ||
raise ScriptWorkerTaskException( | ||
"upstream artifact with path: {}, does not exist".format(abs_path) | ||
) | ||
return abs_path | ||
|
||
|
||
def get_upstream_artifacts(context, preserve_full_paths=False): | ||
artifacts = {} | ||
for artifact_dict in context.task['payload']['upstreamArtifacts']: | ||
locale = artifact_dict['locale'] | ||
artifacts[locale] = artifacts.get(locale, {}) | ||
for path in artifact_dict['paths']: | ||
abs_path = get_upstream_artifact(context, artifact_dict['taskId'], path) | ||
abs_path = scriptworker_artifacts.get_and_check_single_upstream_artifact_full_path( | ||
context, artifact_dict['taskId'], path | ||
) | ||
if preserve_full_paths: | ||
artifacts[locale][path] = abs_path | ||
else: | ||
artifacts[locale][os.path.basename(abs_path)] = abs_path | ||
return artifacts | ||
|
||
|
||
def get_upstream_artifacts_with_zip_extract_param(context): | ||
# XXX A dict comprehension isn't used because upstream_definition would be erased if the same | ||
# taskId is present twice in upstreamArtifacts | ||
upstream_artifacts_per_task_id = {} | ||
|
||
for artifact_definition in context.task['payload']['upstreamArtifacts']: | ||
task_id = artifact_definition['taskId'] | ||
upstream_definitions = upstream_artifacts_per_task_id.get(task_id, []) | ||
|
||
new_upstream_definition = { | ||
'paths': [ | ||
scriptworker_artifacts.get_and_check_single_upstream_artifact_full_path(context, task_id, path) | ||
for path in artifact_definition['paths'] | ||
], | ||
'zip_extract': artifact_definition.get('zipExtract', False), | ||
} | ||
|
||
upstream_definitions.append(new_upstream_definition) | ||
upstream_artifacts_per_task_id[task_id] = upstream_definitions | ||
|
||
return upstream_artifacts_per_task_id | ||
|
||
|
||
def get_release_props(context, platform_mapping=STAGE_PLATFORM_MAP): | ||
"""determined via parsing the Nightly build job's payload and | ||
expanded the properties with props beetmover knows about.""" | ||
|
@@ -151,7 +168,7 @@ def update_props(context, props, platform_mapping): | |
`stage_platform` as we need both in the beetmover template manifests.""" | ||
props = deepcopy(props) | ||
|
||
stage_platform = props["platform"] | ||
stage_platform = props.get('platform', '') | ||
# for some products/platforms this mapping is not needed, hence the default | ||
props["platform"] = platform_mapping.get(stage_platform, stage_platform) | ||
props["stage_platform"] = stage_platform | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
--- | ||
metadata: | ||
name: "Maven repository" | ||
description: "Maps artifacts to spec'd maven location" | ||
owner: "release@mozilla.com" | ||
|
||
s3_bucket_path: maven2/org/mozilla/{{ artifact_id }}/{{ version }}/ # Maven groupId is org.mozilla | ||
|
||
mapping: | ||
{% for locale in ['en-US'] %} | ||
"{{ locale }}": # Locale is not needed for geckoview, it's used by move_beets, though | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I feel you. However this is going to be washed away when declarative artifacts is done. |
||
{% for product in ['geckoview'] %} | ||
"{{ artifact_id }}-{{ version }}.aar": | ||
s3_key: {{ artifact_id }}-{{ version }}.aar | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.aar | ||
"{{ artifact_id }}-{{ version }}.aar.md5": | ||
s3_key: {{ artifact_id }}-{{ version }}.aar.md5 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.aar.md5 | ||
"{{ artifact_id }}-{{ version }}.aar.sha1": | ||
s3_key: {{ artifact_id }}-{{ version }}.aar.sha1 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.aar.sha1 | ||
"{{ artifact_id }}-{{ version }}.pom": | ||
s3_key: {{ artifact_id }}-{{ version }}.pom | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.pom | ||
"{{ artifact_id }}-{{ version }}.pom.md5": | ||
s3_key: {{ artifact_id }}-{{ version }}.pom.md5 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.pom.md5 | ||
"{{ artifact_id }}-{{ version }}.pom.sha1": | ||
s3_key: {{ artifact_id }}-{{ version }}.pom.sha1 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}.pom.sha1 | ||
"{{ artifact_id }}-{{ version }}-javadoc.jar": | ||
s3_key: {{ artifact_id }}-{{ version }}-javadoc.jar | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-javadoc.jar | ||
"{{ artifact_id }}-{{ version }}-javadoc.jar.md5": | ||
s3_key: {{ artifact_id }}-{{ version }}-javadoc.jar.md5 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-javadoc.jar.md5 | ||
"{{ artifact_id }}-{{ version }}-javadoc.jar.sha1": | ||
s3_key: {{ artifact_id }}-{{ version }}-javadoc.jar.sha1 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-javadoc.jar.sha1 | ||
"{{ artifact_id }}-{{ version }}-sources.jar": | ||
s3_key: {{ artifact_id }}-{{ version }}-sources.jar | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-sources.jar | ||
"{{ artifact_id }}-{{ version }}-sources.jar.md5": | ||
s3_key: {{ artifact_id }}-{{ version }}-sources.jar.md5 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-sources.jar.md5 | ||
"{{ artifact_id }}-{{ version }}-sources.jar.sha1": | ||
s3_key: {{ artifact_id }}-{{ version }}-sources.jar.sha1 | ||
destinations: | ||
- {{ artifact_id }}-{{ version }}-sources.jar.sha1 | ||
{% endfor %} | ||
{% endfor %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ on the comment. Any reason why we're not moving these two ZIP_* related-constants within the script_config ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good call. I had forgotten about script_config. I added the max file size in mozilla-releng/build-puppet@bcef5fc.
I don't foresee a need to change the compression ratio as much as the max file size. So, I haven't added it. Moreover, I think increasing the ratio can be a bigger risk than just increasing the file size. If one day we realize we need to deal with zip files that are compressed more than 10 times, we probably want to revisit the ratio check within beetmover. That's why, I think we shouldn't expose that value in script_config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks for clarifying.