Closed Bug 1154801 Opened 7 years ago Closed 6 years ago

add support in ScriptFactory for getting an S3 archive of mozharness

Categories

(Release Engineering :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jlund, Unassigned)

References

Details

Attachments

(7 files, 1 obsolete file)

Blocks: 1179476
No longer blocks: 1131856
this ticket covers all mozharness based continuous integration based jobs.

essentially: parts 1 and 2 from https://bugzilla.mozilla.org/show_bug.cgi?id=1179476#c0
rail I think you reviewed armen's mh pinning patches. feel free to redirect if you can't review this

essentially this patch without any buildbot-config patch will be a noop. the logic extends the pinning/caching ScriptFactory steps we have and allows for a third option: mozharness in gecko tree based jobs by using relengapi archiver (bug 1154795)

this is the buildbotcustom changes needed to complete parts 1 + 2 from https://bugzilla.mozilla.org/show_bug.cgi?id=1179476#c0

up next is the bbot-cfg change to enable this for ash
Attachment #8628481 - Flags: review?(rail)
enables previous bbotcustom patch for ash only

dump_master diff:
http://people.mozilla.org/~jlund/ash_in_tree_mh-dump_master.diff
Attachment #8628482 - Flags: review?(rail)
Attachment #8628482 - Flags: review?(rail) → review+
I should add an example of this working: http://dev-master2.bb.releng.use1.mozilla.com:8037/builders/OS%20X%2010.7%20mozilla-central%20build/builds/26

(see build step 'download_and_extract_scripts_archive')
Comment on attachment 8628481 [details] [diff] [review]
150701_bug_1154801_scriptfactory_in_tree_mh-bbotcustom.patch

Review of attachment 8628481 [details] [diff] [review]:
-----------------------------------------------------------------

A couple of nits, otherwise looks good.

::: process/factory.py
@@ +4884,5 @@
> +                self.addStep(ShellCommand(
> +                    command=['bash', '-c',
> +                             WithProperties('wget -Oarchiver_client.py ' + \
> +                                            '--no-check-certificate --tries=10 --waitretry=3 ' + \
> +                                            'http://hg.mozilla.org/build/tools/raw-file/default/buildfarm/utils/archiver_client.py')],

A nit. WithProperties() is redundant here. Feel free to remove it when you land.

@@ +4909,5 @@
> +                             '--debug')],
> +                log_eval_func=rc_eval_func({0: SUCCESS, None: EXCEPTION}),
> +                haltOnFailure=True,
> +            ))
> +            if scriptName[0] == '/':

scriptName.startswith('/') is more Pythonic ;)
Attachment #8628481 - Flags: review?(rail) → review+
Comment on attachment 8628481 [details] [diff] [review]
150701_bug_1154801_scriptfactory_in_tree_mh-bbotcustom.patch

thanks!

interdiff from r?:
http://people.mozilla.org/~jlund/150701_1154801_bbotcustom-interdiff.diff

full patch on default:
remote:   https://hg.mozilla.org/build/buildbotcustom/rev/442f22927d85
Attachment #8628481 - Flags: checked-in+
so far so good. Ash is looking like the jobs are working as expected. this patch enables in-gecko-tree mh based jobs across all trees.

it depends on https://bugzil.la/1154796 landing across every tree first. Will ask for r? once that happens.
this changes the destination from being an abs path to a relative path.

instead of relying on buildbot properties (basedir), which varies on windows slaves, we can actually just use a relative path to achieve the same outcome.

archiver client patch to accommodate this, incoming
Attachment #8629465 - Flags: review?(catlee)
this supports the change from destination being an abs path to a relative path from buildbot.

it also adds some debug lines for ... debugging
Attachment #8629466 - Flags: review?(catlee)
Comment on attachment 8629465 [details] [diff] [review]
150703_bug_1154801_scriptfactory_in_tree_mh-destination_rel_path-bbotcustom.patch

Review of attachment 8629465 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/factory.py
@@ +4874,5 @@
>                  self.addStep(ShellCommand(
>                      command=['bash', '-c',
>                               'wget -Oarchiver_client.py ' +
>                               '--no-check-certificate --tries=10 --waitretry=3 ' +
>                               'http://hg.mozilla.org/build/tools/raw-file/default/buildfarm/utils/archiver_client.py'],

should this be https?
Attachment #8629465 - Flags: review?(catlee) → review+
Comment on attachment 8629466 [details] [diff] [review]
150703_bug_1154801_archiver_client_support_relative_destination-tools.patch

Review of attachment 8629466 [details] [diff] [review]:
-----------------------------------------------------------------

I noticed above in this script (https://hg.mozilla.org/build/tools/file/bdd22bfc601f/buildfarm/utils/archiver_client.py#l45) that we're configuring logging in the top-level module scope. This can cause problems if you ever want to import this module from another script. It's better to configure logging inside your main() method.

::: buildfarm/utils/archiver_client.py
@@ +162,4 @@
>  
>      try:
>          tar = tarfile.open(fileobj=response, mode='r|gz')
> +        log.debug("unpacking tar archive at: %s" % (extract_root))

log.debug("unpacking tar archive at: %s", extract_root) is more idiomatic python. it also saves you from having your logging calls generate exceptions if you mess up the number or type of arguments for the string interpolation.

@@ +196,5 @@
>          extract_root = os.path.join(extract_root, subdir)
>  
> +    destination = os.path.join(os.getcwd(), config_key)
> +    if options.destination:
> +        destination = os.path.join(os.getcwd(), options.destination)

I think we should support either relative or absolute paths in this code, it makes it far easier to use from other code.

os.path.abspath(options.destination) should give you an absolute path given either a relative or absolute input.
Attachment #8629466 - Flags: review?(catlee) → review+
Comment on attachment 8629466 [details] [diff] [review]
150703_bug_1154801_archiver_client_support_relative_destination-tools.patch

thanks, I updated the patch to your feedback. I didn't move the log configuration yet as I'll do that after I fix bustage. It will be good to do even if though the scripts contents was never planned to be used outside of the script ®

interdiff: http://people.mozilla.org/~jlund/destination_abs_path-interdiff-tools.patch

final patch: https://hg.mozilla.org/build/tools/rev/97aa0aee9eb2
Attachment #8629466 - Flags: checked-in+
Comment on attachment 8629465 [details] [diff] [review]
150703_bug_1154801_scriptfactory_in_tree_mh-destination_rel_path-bbotcustom.patch

thanks, using https

remote:   https://hg.mozilla.org/build/buildbotcustom/rev/e86f26add3c8
Attachment #8629465 - Flags: checked-in+
looks like my above plan will work for everything but thunderbird. to accommodate the special snowflake I am changing the structure around in the factory.

We now pivot off a much more useful variable: relengapi_archiver_repo_path which changes depending on if we are using thunderbird_config or not.

this patch will make more sense with incoming bbot-cfg patch
Attachment #8630721 - Flags: review?(rail)
this is the big one. I'm still landing mh across all branches but I'd like to get the review now.

Basically this will enable in-gecko-tree mh based jobs across all branches for everything that uses ScriptFactory bar release jobs (release.py is being addressed tomorrow)

This also supports the new thunderbird case from previous patch
Attachment #8629163 - Attachment is obsolete: true
Attachment #8630722 - Flags: review?(rail)
Attachment #8630722 - Flags: review?(rail) → review+
Attachment #8630721 - Flags: review?(rail) → review+
I looked at all 3 patches at once, so don't worry about the r+ bomb. :)
bad /me

I forgot after my last change to support thunderbird to sanity check test masters (I only checked build).

This patch changes the old archiver var for the new one: s/relengapi_archiver/relengapi_archiver_repo_path
Attachment #8631836 - Flags: review?(catlee)
Attachment #8631836 - Flags: review?(catlee) → review+
this has been live and used everywhere since last week. Resolving
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.