Closed Bug 1119387 Opened 5 years ago Closed 5 years ago

Upload flame updates to balrog

Categories

(Taskcluster :: General, defect, P1)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jlal, Assigned: wcosta)

References

Details

Attachments

(12 files, 2 obsolete files)

39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
bhearsum
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
Details
39 bytes, text/x-review-board-request
garndt
: review+
Details
Portions of this are handled by mozharness but in particular we need to configure the mar path (should be an artifact url) and embed credentials for balrog to the phone images.

See submit_to_balrog in mozharness/scripts/b2g_build.py 

Note- it is likely easier to start submitting these to staging balrog first as we likely need to proxy to inside the VPC to post to production balrog.
Assignee: nobody → wcosta
Priority: -- → P1
Status: NEW → ASSIGNED
Depends on: 1119389
Depends on: 1124349
Depends on: 1128572
Depends on: 1085557
Blocks: 1134226
Depends on: 1134386
No longer depends on: 1085557
Depends on: 1139066
Attached file MozReview Request: bz://1119387/wcosta (obsolete) —
/r/5601 - Bug 1119387 part 1: Move basic build setup to a separate script.
/r/5603 - Bug 1119387 part 2: Add flame nightly build script.
/r/5605 - Bug 1119387 part 3: Add option to specify mozharness branch.
/r/5607 - Bug 1119387 part 4: Validate only artifacts that can leak blobs.
/r/5609 - Bug 1119387 part 5: Update docker images.

Pull down these commits:

hg pull review -r b6ed96f6616e2df3a5659abb9700c3fdfbad50e4
Attachment #8579384 - Flags: review?(jlal)
Attachment #8579384 - Flags: review?(garndt)
Attached file MozReview Request: bz://1119387/wcosta (obsolete) —
/r/5621 - Bug 1119387 part 1: Enable overriding of nightly build rules.
/r/5623 - Bug 1119387 part 2: Enable to pass balrog platform through command line.
/r/5625 - Bug 1119387 part 3: Allow the build configuration pass balrog username.
/r/5627 - Bug 1119387 part 4: Make buildbot properties optional.
/r/5629 - Bug 1119387 part 5: Setup branch name.
/r/5631 - Bug 1119387 part 6: Fix type error exception in upload method.
/r/5633 - Bug 1119387 part 7: Add nightly phone taskcluster build config.

Pull down these commits:

hg pull review -r 962f2aa6f8fa0be8ae24a7499caddad3f550b172
Attachment #8579395 - Flags: review?(jlund)
Attachment #8579395 - Flags: review?(jlal)
Attachment #8579395 - Flags: review?(bhearsum)
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

/r/5621 - Bug 1119387 part 1: Enable overriding of nightly build rules.
/r/5623 - Bug 1119387 part 2: Enable to pass balrog platform through command line.
/r/5625 - Bug 1119387 part 3: Allow the build configuration pass balrog username.
/r/5627 - Bug 1119387 part 4: Make buildbot properties optional.
/r/5629 - Bug 1119387 part 5: Setup branch name.
/r/5631 - Bug 1119387 part 6: Fix type error exception in upload method.
/r/5633 - Bug 1119387 part 7: Add nightly phone taskcluster build config.

Pull down these commits:

hg pull review -r 962f2aa6f8fa0be8ae24a7499caddad3f550b172
Attachment #8579395 - Flags: review?(jlund)
There are two points on this patch:

1) As Taskcluster currently cannot talk to production Balrog, we are pushing updates to staging. As the balrog config in gecko tree is shared among other phones, flame updates will not work.

2) As Taskcluster cannot push symbols either, we disable breakpad symbols too.
No longer depends on: 1119389, 1124349
https://reviewboard.mozilla.org/r/5607/#review4611

::: testing/docker/phone-builder/bin/validate_task.py
(Diff revision 1)
> -    if 'artifacts' in payload:
> +    locations = task["extra"]["locations"]

Hrm, so how does this prevent us from leaking things as a public artifact?  This is just a location that can be referenced later on, but not preventing something within the task from putting it into artifacts-public from what I can tell.
I think we need to land better try permissions before we push this .. It might be pretty easy to get the backup folders as is since we allow just about anyone on try access to this worker type right now...
(In reply to Greg Arndt [:garndt] from comment #6)
> https://reviewboard.mozilla.org/r/5607/#review4611
> 
> ::: testing/docker/phone-builder/bin/validate_task.py
> (Diff revision 1)
> > -    if 'artifacts' in payload:
> > +    locations = task["extra"]["locations"]
> 
> Hrm, so how does this prevent us from leaking things as a public artifact? 
> This is just a location that can be referenced later on, but not preventing
> something within the task from putting it into artifacts-public from what I
> can tell.

This has been discussed in irc.
https://reviewboard.mozilla.org/r/5607/#review4647

> Hrm, so how does this prevent us from leaking things as a public artifact?  This is just a location that can be referenced later on, but not preventing something within the task from putting it into artifacts-public from what I can tell.

The issue is not related to this commit. We discussed that in irc and going to fix that in another bug.
https://reviewboard.mozilla.org/r/5621/#review4649

I'm not sure about the architectural changes this patch is making. I think Jordan is a better reviewer than me for them....
https://reviewboard.mozilla.org/r/5621/#review4651

::: configs/b2g/taskcluster-phone.py
(Diff revision 1)
> +        'submit-to-balrog'

One thing I will say though, is that I'm not sure we want Balrog submission for non-Nightly builds. Do you have Nightly builds for this configuration?
https://reviewboard.mozilla.org/r/5623/#review4653

::: scripts/b2g_build.py
(Diff revision 1)
> +            self.set_buildbot_property("platform", self.config["platform"])

I think you should flip these around -- the config specified platform should override the properties.
https://reviewboard.mozilla.org/r/5625/#review4655

::: mozharness/mozilla/updates/balrog.py
(Diff revision 1)
> +            product = None

This one is surprising to me. Product is required to submit to Balrog - the submitter can't generate the release name without it. How is balrog-submitter.py succeeding for you without it?
https://reviewboard.mozilla.org/r/5633/#review4659

::: configs/b2g/taskcluster-phone-nightly.py
(Diff revision 1)
> +    "balrog_username": "stage-b2gbld",

It's generally a bad idea to specify dev or prod specific things such as the balrog api root and username in configs like this. Doing so means that you have to modify _build_ configs to run things in dev. We have these defined already in the balrog production.py/staging.py configs. You should pass one of those to your script in addition to this config -- Mozharness will layer them together. This probably removes the need for some of your changes to the BalrogMixin.
Attachment #8579395 - Flags: review?(bhearsum) → feedback+
Another thing to note is that we need to make sure we disable the buildbot Flame builds either completely, or just disable their Balrog uploading, prior to this landing. (I'm assuming these new builds are meant to take the place of them.)
Comment on attachment 8579384 [details]
MozReview Request: bz://1119387/wcosta

looks good, but now that we are not validating where artifacts get put, the img location validation doesn't really help and could be removed (that location isn't used to tell the task where to put something, it's just for reference by other tasks).

Also, the comments I had about the security of this image are being addressed in a separate bug and not related to this patch.  Nothing drastically changed in those regards with how things get to different places.
Attachment #8579384 - Flags: review?(garndt) → review+
https://reviewboard.mozilla.org/r/5621/#review4673

> One thing I will say though, is that I'm not sure we want Balrog submission for non-Nightly builds. Do you have Nightly builds for this configuration?

After talking to lightsofapollo in irc, I am going to drop this for now.
Comment on attachment 8579384 [details]
MozReview Request: bz://1119387/wcosta

/r/5601 - Bug 1119387 part 1: Move basic build setup to a separate script.
/r/5603 - Bug 1119387 part 2: Add flame nightly build script.
/r/5605 - Bug 1119387 part 3: Add option to specify mozharness branch.
/r/5607 - Bug 1119387 part 4: Validate only artifacts that can leak blobs.
/r/5609 - Bug 1119387 part 5: Update docker images.

Pull down these commits:

hg pull review -r 7fe42dfaee7c4e246eb71d09e1601e92a0ba869b
Attachment #8579384 - Flags: review+ → review?(garndt)
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

/r/5621 - Bug 1119387 part 1: Enable to pass balrog platform through command line.
/r/5623 - Bug 1119387 part 2: Allow the build configuration pass balrog product.
/r/5625 - Bug 1119387 part 3: Make buildbot properties optional.
/r/5627 - Bug 1119387 part 4: Setup branch name.
/r/5629 - Bug 1119387 part 5: Fix type error exception in upload method.
/r/5631 - Bug 1119387 part 6: Add nightly phone taskcluster build config.

Pull down these commits:

hg pull review -r 006a15b11cb91478bfd533c8b46fbf37651781d6
Attachment #8579395 - Flags: feedback+ → review?(bhearsum)
Attachment #8579384 - Flags: review?(jlal)
Attachment #8579395 - Flags: review?(jlal) → review?(garndt)
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

/r/5621 - Bug 1119387 part 1: Enable to pass balrog platform through command line.
/r/5623 - Bug 1119387 part 2: Allow the build configuration pass balrog product.
/r/5625 - Bug 1119387 part 3: Make buildbot properties optional.
/r/5627 - Bug 1119387 part 4: Setup branch name.
/r/5629 - Bug 1119387 part 5: Fix type error exception in upload method.
/r/5631 - Bug 1119387 part 6: Add nightly phone taskcluster build config.

Pull down these commits:

hg pull review -r 006a15b11cb91478bfd533c8b46fbf37651781d6
Attachment #8579395 - Flags: review?(garndt)
Comment on attachment 8579384 [details]
MozReview Request: bz://1119387/wcosta

/r/5601 - Bug 1119387 part 1: Move basic build setup to a separate script.
/r/5603 - Bug 1119387 part 2: Add flame nightly build script.
/r/5605 - Bug 1119387 part 3: Add option to specify mozharness branch.
/r/5607 - Bug 1119387 part 4: Validate only artifacts that can leak blobs.
/r/5609 - Bug 1119387 part 5: Update docker images.

Pull down these commits:

hg pull review -r 7fe42dfaee7c4e246eb71d09e1601e92a0ba869b
Comment on attachment 8579384 [details]
MozReview Request: bz://1119387/wcosta

https://reviewboard.mozilla.org/r/5599/#review4779

Ship It!
Attachment #8579384 - Flags: review?(garndt) → review+
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

https://reviewboard.mozilla.org/r/5619/#review4803

::: mozharness/mozilla/updates/balrog.py
(Diff revisions 1 - 2)
>          c = self.config

It looks like product is unused here now. r=me if you stop passing it.
Attachment #8579395 - Flags: review?(bhearsum)
https://reviewboard.mozilla.org/r/5619/#review4845

::: mozharness/mozilla/updates/balrog.py
(Diff revision 2)
> +            buildbot_properties = []

Just noticed this, you should set buildbot_properties to {} not []. Items returns a dict, not a list.

::: mozharness/mozilla/updates/balrog.py
(Diff revision 2)
> +

Can you raise an error if product is None after this point? It's not valid to submit a release without product, and could lead to bad things...
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

/r/5621 - Bug 1119387 part 1: Enable to pass balrog platform through command line.
/r/5623 - Bug 1119387 part 2: Allow the build configuration pass balrog product.
/r/5625 - Bug 1119387 part 3: Make buildbot properties optional.
/r/5627 - Bug 1119387 part 4: Setup branch name.
/r/5629 - Bug 1119387 part 5: Fix type error exception in upload method.
/r/5631 - Bug 1119387 part 6: Add nightly phone taskcluster build config.

Pull down these commits:

hg pull review -r c36fc18c533ae55bc82570e8908514f8b9d3681e
Attachment #8579395 - Flags: review?(bhearsum)
Comment on attachment 8579395 [details]
MozReview Request: bz://1119387/wcosta

https://reviewboard.mozilla.org/r/5619/#review4853

Ship It!
Attachment #8579395 - Flags: review?(bhearsum) → review+
Keywords: checkin-needed
Attachment #8579395 - Attachment is obsolete: true
Attachment #8579384 - Attachment is obsolete: true
Attachment #8619068 - Flags: review+
Attachment #8619069 - Flags: review+
Attachment #8619070 - Flags: review+
Attachment #8619071 - Flags: review+
Attachment #8619072 - Flags: review+
Attachment #8619073 - Flags: review+
Attachment #8619074 - Flags: review+
Attachment #8619075 - Flags: review+
Attachment #8619076 - Flags: review+
Attachment #8619077 - Flags: review+
Attachment #8619078 - Flags: review+
Attachment #8619079 - Flags: review+
Component: TaskCluster → General
Product: Testing → Taskcluster
Target Milestone: mozilla39 → mozilla41
Version: unspecified → Trunk
Resetting Version and Target Milestone that accidentally got changed...
Target Milestone: mozilla41 → ---
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.