Replace PartnerRepackFactory with a mozharness script

RESOLVED FIXED

Status

Release Engineering
Releases: Custom Builds
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: coop, Assigned: coop)

Tracking

other
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, b2g-v2.5 fixed)

Details

Attachments

(6 attachments, 2 obsolete attachments)

3.13 KB, patch
kmoir
: review+
Details | Diff | Splinter Review
8.96 KB, patch
kmoir
: review+
Details | Diff | Splinter Review
15.13 KB, patch
kmoir
: review+
Details | Diff | Splinter Review
8.10 KB, patch
jlund
: review+
Details | Diff | Splinter Review
2.22 KB, patch
kmoir
: review+
Details | Diff | Splinter Review
5.80 KB, patch
kmoir
: review+
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
This amounts to "re-integrate partner repacks into the release process in a way that makes it easy to move them forward into TaskCluster."

I have a working mozharness script that encapsulates the changes, along with the requisite suite of buildbotcustom, configs, and puppet patches to make it work.

Patches coming up.
(Assignee)

Updated

2 years ago
Assignee: nobody → coop
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8704794 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files

I used the other scripts in the mozharness/scripts dir as templates and tried to keep the new script as simple as possible. The new script is a thin wrapper around the partner repo sync steps[1] and the subsequent call to partner-repacks.py[2] with some basic release config vars. This is the same sequence I've been running for months on the standalone partner repacks machine.

1. https://github.com/mozilla-partners/repack-manifests/blob/master/README.md
2. https://github.com/mozilla-partners/repack-scripts/blob/master/partner-repacks.py

The config files aren't really used at present, but it felt important to keep a config source in-tree just in case.

Here is output of the script running in my staging env against a clone of mozilla-beta against all the release platforms: 

http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-linux_partner_repack/builds/0
http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-linux64_partner_repack/builds/0
http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-macosx64_partner_repack/builds/11
http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-win32_partner_repack/builds/3
http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-win64_partner_repack/builds/0

Once we're done with reviews, I'll need to uplift the script all the way up to mozilla-release, hopefully before the next release cycle.
Attachment #8704794 - Flags: review?(jlund)
(Assignee)

Comment 2

2 years ago
Created attachment 8704796 [details] [diff] [review]
[puppet] s3cfg files for release and partners

This patch adds two new configs files used by the s3cmd tool for uploading partner repacks to the appropriate partner/mozilla buckets in S3. The contents of the files are already in the hiera cache. 

At present, we only do partner repacks on Mac, so I only install the config files there.

Since these are new secrets, they'll need to be added the cleanup instructions for slave loan:

https://wiki.mozilla.org/ReleaseEngineering/How_To/Loan_a_Slave#bld-lion-r5.2C_talos-mtnlion-r5.2C_t-yosemite-r5.2C_t-yosemite-r7
Attachment #8704796 - Flags: review?(kmoir)
(Assignee)

Comment 3

2 years ago
Created attachment 8704803 [details] [diff] [review]
[buildbot-configs] Re-enable partner repacks on beta and release

In addition to re-enabling the partner repacks for beta and release, this patch also:

* switches teh repacks to use the mozharness script
* removes the old partner repo from the list of tagged repos
* adds win64 to the list of platforms for staging releases, since it's in production now
Attachment #8704803 - Flags: review?(kmoir)
(Assignee)

Comment 4

2 years ago
Created attachment 8704807 [details] [diff] [review]
[buildbotcustom] Replace PartnerRepackFactory with a SigningScriptFactory instance

The bulk of this patch is removing the old PartnerRepackFactory class. Yay for code removal. There is some other stuff going on, though.

In SigningScriptFactory, I've switched the toolsdir SetProperty step to use the tools_repo_cache variable when set. It seems silly that we have a canonical tools checkout on each machine that we keep up-to-date but never use. It also avoids the problem where you can't use a SigningScriptFactory with a mozharness archive because it can't find signtool.py where it expects to. See my staging master run[1] for an example of this failure.

You can work around this by cloning tools within your mozharness script, but this seems cleaner to me. I did a quick survey of other SigningScriptFactory invocations, and none of them seem to set tools_repo_cache, so this shouldn't affect them anyway.

The actual factory creation is pretty straightforward. We pass our releaseConfig variables through to the new mozharness script.

I am planning to add a separate, standalone partner repack builds like the l10n one (bug 1236954).

1. http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-macosx64_partner_repack/builds/6/steps/run_script/logs/stdio
Attachment #8704807 - Flags: review?(kmoir)

Comment 5

2 years ago
Comment on attachment 8704794 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files

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

script looks great! I'm going to r- just because of the unused imports and variables I point out at the top that may have been copied from somewhere else. All the other comments are nits so feel free to argue back :)

::: testing/mozharness/scripts/desktop_partner_repacks.py
@@ +9,5 @@
> +This script manages Desktop partner repacks for beta/release builds.
> +"""
> +import multiprocessing
> +import os
> +import re

I think from the `re` import to `FATAL` there are many modules that are not actually used. Maybe just leftover from copy/paste?

@@ +47,5 @@
> +
> +# some other values such as "%(version)s", "%(buildid)s", ...
> +# are defined at run time and they cannot be enforced in the _pre_config_lock
> +# phase
> +runtime_config_tokens = ('version', 'buildnumber', 'platform', 'partner')

likewise, `SUCCESS` `FAILURE` `configuration_tokens` and `runtime_config_tokens` I don't think are needed for this script

@@ +50,5 @@
> +# phase
> +runtime_config_tokens = ('version', 'buildnumber', 'platform', 'partner')
> +
> +# DesktopPartnerRepacks {{{1
> +class DesktopPartnerRepacks(ReleaseMixin, BuildbotMixin,

are all these subclasses needed? Looks like you are running most of your actions via subprocess (self.run_command). Ones in question would be VCSMixin, SigningMixin, and TransferMixin.

@@ +101,5 @@
> +            'default_actions': DesktopPartnerRepacks.actions,
> +            'config': {
> +                "buildbot_json_path": "buildprops.json",
> +                "log_name": "partner-repacks",
> +                "appName": "Firefox",

would this ever change? Wondering if this is worthy of putting into your already existing in-tree config: release_mozilla-release_desktop.py

@@ +111,5 @@
> +                    's3cmd==1.6.0',
> +                ],
> +                'virtualenv_path': 'venv',
> +                'workdir': 'partner-repacks',
> +                'repack_manifests_url': 'git@github.com:mozilla-partners/repack-manifests.git',

likewise, would this value ever change?

@@ +134,5 @@
> +        self.s3cfg = self.config.get('s3cfg', None)
> +        self.hgroot = self.config.get('hgroot', None)
> +        self.repo = self.config.get('repo', None)
> +
> +        self.workdir = os.path.abspath(self.config['work_dir'])

fyi - for these paths you can use or subclass: https://dxr.mozilla.org/build-central/source/mozharness/mozharness/base/script.py#1729

e.g. the above assigned self.workdir could be replaced by: self.query_abs_dirs()['abs_work_dir']

@@ +143,5 @@
> +
> +    # Actions {{{
> +    def setup(self):
> +        """setup step"""
> +        self.download_file("https://raw.githubusercontent.com/mozilla/git-repo/master/repo",

since you are not sanity checking the return value here of download_file: which will either == file_name if complete, or `failure_status`( which I think in this case equates to None since you don't overwrite _retry_download's default), you may want to put error_level=FATAL so we halt here after exhausting retries. just so we don't cause spurious results in subsequent lines

@@ +147,5 @@
> +        self.download_file("https://raw.githubusercontent.com/mozilla/git-repo/master/repo",
> +                           file_name="repo", parent_dir=self.workdir)
> +        repo = os.path.join(self.workdir, "repo")
> +        self.chmod(repo, 0755)
> +        self.mkdir_p(self.workdir)

iiuc the self.chmod() call would require self.workdir to already exist. iirc, parent_dir in download_file will create it for you every time if it doesn't exist.

@@ +152,5 @@
> +        self.run_command([repo, "init", "--no-repo-verify",
> +                          "-u", self.config['repack_manifests_url']],
> +                         cwd=self.workdir,
> +                         halt_on_failure=True)
> +        self.run_command([repo, "sync"],

do either of these run_command calls require try/catch or retries?

@@ +163,5 @@
> +        repack_cmd = [python, "partner-repacks.py",
> +                      "-v", self.version,
> +                      "-n", self.buildnumber]
> +        if self.platform:
> +            repack_cmd.extend(["--platform", self.platform])

nit: aki preferred we reference config items only from self.config because self.config is immutable unlike all the self attrs you defined in the above __init__. I don't always stick to this idiom so I won't block you here but the mh norm would be more like:

if self.config.get('platform'):
    repack_cmd.extend(["--platform", self.config['platform'])
Attachment #8704794 - Flags: review?(jlund) → review-

Updated

2 years ago
Attachment #8704796 - Flags: review?(kmoir) → review+

Updated

2 years ago
Attachment #8704803 - Flags: review?(kmoir) → review+

Updated

2 years ago
Attachment #8704807 - Flags: review?(kmoir) → review+
(Assignee)

Comment 6

2 years ago
(In reply to Jordan Lund (:jlund) from comment #5)
> script looks great! I'm going to r- just because of the unused imports and
> variables I point out at the top that may have been copied from somewhere
> else. All the other comments are nits so feel free to argue back :)

Thanks for the thorough review. Can you tell this is my first mh script? ;)

> @@ +101,5 @@
> > +            'default_actions': DesktopPartnerRepacks.actions,
> > +            'config': {
> > +                "buildbot_json_path": "buildprops.json",
> > +                "log_name": "partner-repacks",
> > +                "appName": "Firefox",
> 
> would this ever change? Wondering if this is worthy of putting into your
> already existing in-tree config: release_mozilla-release_desktop.py

Unlikely now that we won't be worried about Thunderbird, and Fennec has it's own script.
 
> @@ +111,5 @@
> > +                    's3cmd==1.6.0',
> > +                ],
> > +                'virtualenv_path': 'venv',
> > +                'workdir': 'partner-repacks',
> > +                'repack_manifests_url': 'git@github.com:mozilla-partners/repack-manifests.git',
> 
> likewise, would this value ever change?

Might be useful to be able to specify a user repo here, but we'd want to do that in our staging configs. Maybe this should be default with a option to override?

> @@ +134,5 @@
> > +        self.s3cfg = self.config.get('s3cfg', None)
> > +        self.hgroot = self.config.get('hgroot', None)
> > +        self.repo = self.config.get('repo', None)
> > +
> > +        self.workdir = os.path.abspath(self.config['work_dir'])
> 
> fyi - for these paths you can use or subclass:
> https://dxr.mozilla.org/build-central/source/mozharness/mozharness/base/
> script.py#1729
> 
> e.g. the above assigned self.workdir could be replaced by:
> self.query_abs_dirs()['abs_work_dir']

OK, good to know. I'll make the change.

> @@ +143,5 @@
> > +
> > +    # Actions {{{
> > +    def setup(self):
> > +        """setup step"""
> > +        self.download_file("https://raw.githubusercontent.com/mozilla/git-repo/master/repo",
> 
> since you are not sanity checking the return value here of download_file:
> which will either == file_name if complete, or `failure_status`( which I
> think in this case equates to None since you don't overwrite
> _retry_download's default), you may want to put error_level=FATAL so we halt
> here after exhausting retries. just so we don't cause spurious results in
> subsequent lines

You're right, I should be checking the return value here.

> @@ +147,5 @@
> > +        self.download_file("https://raw.githubusercontent.com/mozilla/git-repo/master/repo",
> > +                           file_name="repo", parent_dir=self.workdir)
> > +        repo = os.path.join(self.workdir, "repo")
> > +        self.chmod(repo, 0755)
> > +        self.mkdir_p(self.workdir)
> 
> iiuc the self.chmod() call would require self.workdir to already exist.
> iirc, parent_dir in download_file will create it for you every time if it
> doesn't exist.

Are you suggesting I create the workdir myself before the download just to be safe?

> @@ +152,5 @@
> > +        self.run_command([repo, "init", "--no-repo-verify",
> > +                          "-u", self.config['repack_manifests_url']],
> > +                         cwd=self.workdir,
> > +                         halt_on_failure=True)
> > +        self.run_command([repo, "sync"],
> 
> do either of these run_command calls require try/catch or retries?

It wouldn't hurt, certainly. The only failure case I've run into here is when running the sync with parallelism (-j). Sometimes we end up in a logjam and need to remove the repos and start over. I'll add some extra checking and recovery here.

This does raise the question of what is a better mozharness script to use as a template for learning, because the ones I was copying from (desktop_l10n.py in particular) seem to play fast and loose here.

> nit: aki preferred we reference config items only from self.config because
> self.config is immutable unlike all the self attrs you defined in the above
> __init__. I don't always stick to this idiom so I won't block you here but
> the mh norm would be more like:
> 
> if self.config.get('platform'):
>     repack_cmd.extend(["--platform", self.config['platform'])

Well, that's why I pinged you for review - I want to learn good mh habits before I start reinforcing bad ones.
(Assignee)

Comment 7

2 years ago
Created attachment 8705734 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files, v2

Incorporates Jordan's feedback from comment #5, namely:

* removes unused variables and imports
* moved some variables that might possibly change (i.e. in staging) into the config files
* subclassed query_abs_dir() for this script
* wrapped repo actions in retry, with cleanup
* access config vars via self.config

Tested it against bing repacks (mac, win) on bld-lion-r5-002 and it seems to work fine.
Attachment #8704794 - Attachment is obsolete: true
Attachment #8705734 - Flags: review?(jlund)

Comment 8

2 years ago
(In reply to Chris Cooper [:coop] from comment #6)

> 
> > @@ +147,5 @@
> > > +        self.download_file("https://raw.githubusercontent.com/mozilla/git-repo/master/repo",
> > > +                           file_name="repo", parent_dir=self.workdir)
> > > +        repo = os.path.join(self.workdir, "repo")
> > > +        self.chmod(repo, 0755)
> > > +        self.mkdir_p(self.workdir)
> > 
> > iiuc the self.chmod() call would require self.workdir to already exist.
> > iirc, parent_dir in download_file will create it for you every time if it
> > doesn't exist.
> 
> Are you suggesting I create the workdir myself before the download just to
> be safe?
> 

sorry for not being clear - I meant that 1) self.workdir would have to exist here because download_file call would have created it. 2) you call self.mkdir_p(path) _after_ self.chmod(path)

> 
> This does raise the question of what is a better mozharness script to use as
> a template for learning, because the ones I was copying from
> (desktop_l10n.py in particular) seem to play fast and loose here

I care more about mozharness/{base,mozilla} than I do about the end scripts. Saying that, for some example scripts, you can poke at https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/examples

> 
> > nit: aki preferred we reference config items only from self.config because
> > self.config is immutable unlike all the self attrs you defined in the above
> > __init__. I don't always stick to this idiom so I won't block you here but
> > the mh norm would be more like:
> > 
> > if self.config.get('platform'):
> >     repack_cmd.extend(["--platform", self.config['platform'])
> 
> Well, that's why I pinged you for review - I want to learn good mh habits
> before I start reinforcing bad ones.

:)

Comment 9

2 years ago
Comment on attachment 8705734 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files, v2

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

::: testing/mozharness/scripts/desktop_partner_repacks.py
@@ +110,5 @@
> +        self.platform = self.config.get('platform', None)
> +        self.partner = self.config.get('partner', None)
> +        self.s3cfg = self.config.get('s3cfg', None)
> +        self.hgroot = self.config.get('hgroot', None)
> +        self.hgrepo = self.config.get('hgrepo', None)

iiuc, most (all?) these attr assignments be deleted now that you reference self.config directly?

@@ +112,5 @@
> +        self.s3cfg = self.config.get('s3cfg', None)
> +        self.hgroot = self.config.get('hgroot', None)
> +        self.hgrepo = self.config.get('hgrepo', None)
> +
> +    def query_abs_dirs(self):

++

@@ +140,5 @@
> +        status = self.run_command([repo, "init", "--no-repo-verify",
> +                                   "-u", self.config['repack_manifests_url']],
> +                                  cwd=self.query_abs_dirs()['abs_work_dir'],
> +                                  halt_on_failure=True)
> +        if status != 0:

this could be simply `if status`

but I think I have steered you wrong. looking at this again, you have halt_on_failure=True for your run_command calls. run_command automatically evaluates the return value for you and if it it's not in success_codes (defaults to [0]), will throw an error and in your case, halt: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#1158

which means, 1) you don't have to check `if status` at all. 2) halt_on_failure won't even let self.retry retry.

so you could remove the retry and status condition checks on these if you are happy without retrying.

Or, if you do want retry, you will need to remove the halt_on_failure in run_command, and instead add, good_statuses=[0], error_level=FATAL to the self.retry()[1] call. Unfortunately, self.retry and self.run_command have different param names that do similar things. good_statuses == success_codes except it's defaulted to None. and self.retry doesn't have halt_on_failure but I believe error_level=FATAL will achieve the same thing after retries are exhausted. Does this make sense?

[1] https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#800

@@ +154,5 @@
> +                                  parent_dir=self.query_abs_dirs()['abs_work_dir'],
> +                                  error_level=FATAL)
> +        if not os.path.exists(repo):
> +            self.fatal("Unable to download repo tool.")
> +        #repo = os.path.join(self.query_abs_dirs()['abs_work_dir'], "repo")

can this line be deleted?
Attachment #8705734 - Flags: review?(jlund) → review-
(Assignee)

Comment 10

2 years ago
(In reply to Jordan Lund (:jlund) from comment #9)
> Comment on attachment 8705734 [details] [diff] [review]
> [mozilla-inbound] New mozharness script for desktop partner repacks, and
> associated config files, v2
> 
> Review of attachment 8705734 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mozharness/scripts/desktop_partner_repacks.py
> @@ +110,5 @@
> > +        self.platform = self.config.get('platform', None)
> > +        self.partner = self.config.get('partner', None)
> > +        self.s3cfg = self.config.get('s3cfg', None)
> > +        self.hgroot = self.config.get('hgroot', None)
> > +        self.hgrepo = self.config.get('hgrepo', None)
> 
> iiuc, most (all?) these attr assignments be deleted now that you reference
> self.config directly?

You're right. I'll fix it up.
 
> which means, 1) you don't have to check `if status` at all. 2)
> halt_on_failure won't even let self.retry retry.
> 
> so you could remove the retry and status condition checks on these if you
> are happy without retrying.

For simplicity's sake, I think I'll just revert to a the halt_on_failure without retry.

> > +        #repo = os.path.join(self.query_abs_dirs()['abs_work_dir'], "repo")
> 
> can this line be deleted?

Yep, will remove.
(Assignee)

Comment 11

2 years ago
Created attachment 8706949 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files, v3

(In reply to Chris Cooper [:coop] from comment #10)
> For simplicity's sake, I think I'll just revert to a the halt_on_failure
> without retry.

Second-guessing myself here. I think it's more valuable to have a retry built-in in case there's a transitory failure. I'd rather spend a few minutes of machine time retrying than need to run the repacks by hand.

The other suggestions from comment #9 have also been incorporated.
Attachment #8705734 - Attachment is obsolete: true
Attachment #8706949 - Flags: review?(jlund)
(Assignee)

Comment 12

2 years ago
Comment on attachment 8704796 [details] [diff] [review]
[puppet] s3cfg files for release and partners

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

https://hg.mozilla.org/build/puppet/rev/ee145a659339
Attachment #8704796 - Flags: checked-in+
(Assignee)

Comment 13

2 years ago
(In reply to Chris Cooper [:coop] from comment #2)
> Since these are new secrets, they'll need to be added the cleanup
> instructions for slave loan:
> 
> https://wiki.mozilla.org/ReleaseEngineering/How_To/Loan_a_Slave#bld-lion-r5.
> 2C_talos-mtnlion-r5.2C_t-yosemite-r5.2C_t-yosemite-r7

Instructions have been updated.
Comment on attachment 8706949 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files, v3

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

looks like we are pretty much ready to go :D

::: testing/mozharness/scripts/desktop_partner_repacks.py
@@ +98,5 @@
> +        )
> +
> +        if 'version' not in self.config:
> +            self.fatal("Version (-v) not supplied.")
> +        self.version = self.config.get('version')

nit: I think this can be removed too

@@ +152,5 @@
> +                      args=(repo,),
> +                      error_level=FATAL,
> +                      cleanup=self._repo_cleanup(),
> +                      good_statuses=[0],
> +                      sleeptime=5) != 0:

because error_level is set to FATAL and you supplied good_statuses, you don't need to put this as a condition or the self.fatal line below because iiuc self.retry() will fatal internally.
Attachment #8706949 - Flags: review?(jlund) → review+
I haven't looked over this yet, but wanted to point out bug 1239032 I just posted.
(Assignee)

Comment 17

2 years ago
Comment on attachment 8706949 [details] [diff] [review]
[mozilla-inbound] New mozharness script for desktop partner repacks, and associated config files, v3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4ff5db9559

...and now we start the uplift dance.
Attachment #8706949 - Flags: checked-in+
(Assignee)

Comment 18

2 years ago
(In reply to Chris Cooper [:coop] from comment #17)
> Comment on attachment 8706949 [details] [diff] [review]
> [mozilla-inbound] New mozharness script for desktop partner repacks, and
> associated config files, v3
> 
> Review of attachment 8706949 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4ff5db9559
> 
> ...and now we start the uplift dance.

Hey Ritu,

Given the uplift lockdown we're in, I wanted to ping you directly about this.

This is a mozharness script that releng wants to use in the release process for beta and release, but to get it there for 44 will require uplifts. It's a self-contained script with its own config files that will only be used in the release process, so it's pretty much the definition of NPOTB.

Do you have any issues with me uplifting this all the way to mozilla-beta?
Flags: needinfo?(rkothari)
(In reply to Chris Cooper [:coop] from comment #18)
> (In reply to Chris Cooper [:coop] from comment #17)
> > Comment on attachment 8706949 [details] [diff] [review]
> > [mozilla-inbound] New mozharness script for desktop partner repacks, and
> > associated config files, v3
> > 
> > Review of attachment 8706949 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4ff5db9559
> > 
> > ...and now we start the uplift dance.
> 
> Hey Ritu,
> 
> Given the uplift lockdown we're in, I wanted to ping you directly about this.
> 
> This is a mozharness script that releng wants to use in the release process
> for beta and release, but to get it there for 44 will require uplifts. It's
> a self-contained script with its own config files that will only be used in
> the release process, so it's pretty much the definition of NPOTB.
> 
> Do you have any issues with me uplifting this all the way to mozilla-beta?

Hi Chris, thanks for checking with me. If this is NPOTB, we are good. Also hoping this will not affect our ability to get Beta9 build ready and pushed out to Beta channel.
Flags: needinfo?(rkothari)

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ec4ff5db9559
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 21

2 years ago
Uplifted to aurora and beta:

https://hg.mozilla.org/releases/mozilla-aurora/rev/47c244a99493
https://hg.mozilla.org/releases/mozilla-beta/rev/6512ec26d8c1

Re-opening because I still need to land the buildbot patches.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 22

2 years ago
Comment on attachment 8704807 [details] [diff] [review]
[buildbotcustom] Replace PartnerRepackFactory with a SigningScriptFactory instance

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

https://hg.mozilla.org/build/buildbotcustom/rev/089b94bab05c
Attachment #8704807 - Flags: checked-in+
(Assignee)

Comment 23

2 years ago
Comment on attachment 8704803 [details] [diff] [review]
[buildbot-configs] Re-enable partner repacks on beta and release

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

https://hg.mozilla.org/build/buildbot-configs/rev/5bdbf1ac3f5f
Attachment #8704803 - Flags: checked-in+
(Assignee)

Comment 27

2 years ago
Created attachment 8707860 [details] [diff] [review]
[buildbotcustom] Set script_repo_revision for ScriptFactory when archiver is used

I made a mistake in my testing. I thought I needed to pass 'script_repo_revision' manually because that was something that the release process would normally do, and I was simply doing one-offs. However, it turns out that when you use a ScriptFactory with an archiver mozharness archive, the script_repo_revision property is never set, so the build fails later when it tries to echo it out for display. I guess I'm the first person to try this permutation of ScriptFactory options. :/

This patch attempts to set script_repo_revision for archiver-based scripts using data that the factory already has, namely (in preferential order):
* relengapi_archiver_release_tag
* relengapi_archiver_rev
* revision

We're unlikely to have a revision in the release context, but we will always have a release tag.

I cancelled the job early once I was convinced it was working, but here's an example of script_repo_revision being set to FIREFOX_44_0b4_RELEASE automatically in a staging run:

http://dev-master2.bb.releng.use1.mozilla.com:8044/builders/release-mozilla-beta-macosx64_partner_repack/builds/12
Attachment #8707860 - Flags: review?(kmoir)

Updated

2 years ago
Attachment #8707860 - Flags: review?(kmoir) → review+
(Assignee)

Comment 29

2 years ago
Created attachment 8709047 [details] [diff] [review]
[buildbot-configs] Update partner repack stanza in release templates

Release configs got overwritten by automation because I didn't update the templates.
Attachment #8709047 - Flags: review?(kmoir)
(Assignee)

Comment 30

2 years ago
Comment on attachment 8707860 [details] [diff] [review]
[buildbotcustom] Set script_repo_revision for ScriptFactory when archiver is used

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

https://hg.mozilla.org/build/buildbotcustom/rev/aa885f5cdbc3
Attachment #8707860 - Flags: checked-in+

Updated

2 years ago
Attachment #8709047 - Flags: review?(kmoir) → review+
(Assignee)

Comment 31

2 years ago
Comment on attachment 8709047 [details] [diff] [review]
[buildbot-configs] Update partner repack stanza in release templates

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

https://hg.mozilla.org/build/buildbot-configs/rev/2f6ad8dd5b7e
Attachment #8709047 - Flags: checked-in+

Updated

2 years ago
Depends on: 1242088
(Assignee)

Comment 36

2 years ago
Partner repacks for Firefox 44.0 build 3 were automatically generated.
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.