send e-mail to release-drivers when a build is requested through ship it

RESOLVED FIXED

Status

Release Engineering
Release Automation
P3
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: bhearsum, Assigned: sylvestre)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [shipit])

Attachments

(5 attachments, 4 obsolete attachments)

779 bytes, patch
rail
: review+
bhearsum
: review+
sylvestre
: checked-in+
Details | Diff | Splinter Review
1.43 KB, patch
rail
: review+
bhearsum
: review+
sylvestre
: checked-in+
Details | Diff | Splinter Review
3.75 KB, patch
rail
: review+
Details | Diff | Splinter Review
3.27 KB, patch
rail
: review+
Details | Diff | Splinter Review
7.73 KB, patch
bhearsum
: review+
sylvestre
: checked-in+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Alex, I understood this to mean "send mail to release-drivers upon submission" (as opposed to when it's marked as ready or actually started) - is that what you meant?
Flags: needinfo?(akeybl)
(Reporter)

Comment 1

5 years ago
Rail and I just talked about this and we're wondering if urgent/not urgent is going to be enough, or whether we need a something more finer grained. Also, do chemspills need to be distinguished explicitly, or only by their priority?
(Reporter)

Comment 2

5 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #1)
> Rail and I just talked about this and we're wondering if urgent/not urgent
> is going to be enough, or whether we need a something more finer grained.
> Also, do chemspills need to be distinguished explicitly, or only by their
> priority?

Oops, this comment was meant for 847496.
(Reporter)

Updated

5 years ago
Priority: -- → P3
Product: mozilla.org → Release Engineering
(Reporter)

Updated

4 years ago
Flags: needinfo?(akeybl)
(Reporter)

Comment 3

4 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #0)
> Alex, I understood this to mean "send mail to release-drivers upon
> submission" (as opposed to when it's marked as ready or actually started) -
> is that what you meant?

ping?
Flags: needinfo?(akeybl)
(Reporter)

Comment 4

4 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #3)
> (In reply to Ben Hearsum [:bhearsum] from comment #0)
> > Alex, I understood this to mean "send mail to release-drivers upon
> > submission" (as opposed to when it's marked as ready or actually started) -
> > is that what you meant?
> 
> ping?

I'm going to assume yes.
Flags: needinfo?(akeybl)
(Assignee)

Comment 5

3 years ago
Created attachment 8441943 [details] [diff] [review]
0001-Send-a-mail-to-r-d-on-the-Do-Eet-action.-Add-a-comme.patch

Here is the implementation.

Send a mail to r-d on the 'Do Eet' action.
Add a comment textarea field in the form to add extra information.
(Reporter)

Comment 6

3 years ago
Comment on attachment 8441943 [details] [diff] [review]
0001-Send-a-mail-to-r-d-on-the-Do-Eet-action.-Add-a-comme.patch

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

Thank you for working on this, it's great to see it getting fixed! It does need a bit of work though:

I don't think this is the right place to be sending the e-mail. Doing so as part of processing an HTTP request is nasty, and blocks the client. I think this should be done in release runner instead, which is a script that polls Ship It for release information. A few notes on that:
* Release runner currently only polls for releases that are "ready" but not "complete". The main challenge is probably going te modifying it to also look for not "ready" and not "complete" in order to send this e-mail at the right time. That code is here:
https://github.com/mozilla/build-tools/blob/master/buildfarm/release/release-runner.py#L68
https://github.com/mozilla/build-tools/blob/master/buildfarm/release/release-runner.py#L68
https://github.com/mozilla/build-tools/blob/master/lib/python/kickoff/api.py#L68
* Testing it might be difficult - you'll need both your own instance of Ship It and to run release runner locally. We have some documentation on that here: https://wiki.mozilla.org/Release:Release_Automation_on_Mercurial:Staging_Specific_Notes#Setup_Ship_It -- you can ignore the parts that talk about buildbot masters - your code won't be getting that far into the process.
* Please put the e-mail address in the release runner config file, not hardcoded.

The addition of the custom comments field looks OK, but you'll need to add a new database schema version so we can upgrade it. You can see a simple example of doing that here: http://git.mozilla.org/?p=build/release-kickoff.git;a=blob;f=migrate_repo/versions/003_Add_submitted_at.py;h=201a46a89d6c1faca358d17c1b747e0bb4891a02;hb=HEAD

I'm going to be away starting this Friday and not back until July 7th. In the meantime, Rail is happy to help with testing and reviews - he knows this code just as well as me.
Attachment #8441943 - Flags: review-
(Assignee)

Comment 7

3 years ago
Created attachment 8443532 [details] [diff] [review]
Improve the documentation with the "migrate" command
(Assignee)

Comment 8

3 years ago
Created attachment 8443533 [details] [diff] [review]
Add a new field comment

I will request for review once I finished the development.
(Assignee)

Comment 9

3 years ago
Created attachment 8446684 [details] [diff] [review]
0003-Manage-the-new-field-comment.patch

This patch add the new comment field in the interface.
Attachment #8441943 - Attachment is obsolete: true
(Assignee)

Comment 10

3 years ago
Created attachment 8446685 [details] [diff] [review]
Runner: Send an email when the build of a new release has been requested

Last but not least, the update of the runner to send the email
(Assignee)

Updated

3 years ago
Attachment #8446685 - Flags: review?(rail)
Attachment #8446685 - Flags: review?(bhearsum)
(Assignee)

Updated

3 years ago
Attachment #8446684 - Flags: review?(rail)
Attachment #8446684 - Flags: review?(bhearsum)
(Assignee)

Updated

3 years ago
Attachment #8443533 - Flags: review?(rail)
Attachment #8443533 - Flags: review?(bhearsum)
(Assignee)

Updated

3 years ago
Attachment #8443532 - Flags: review?(rail)
Attachment #8443532 - Flags: review?(bhearsum)
(Assignee)

Comment 11

3 years ago
The result is:

== Subject ==
Build of Firefox-28.0b2-build1

== Content==

Comment:
zaeazezaeazqsd

A new build has been submitted through ship-it:
Commit: https://hg.mozilla.org/release/mozilla-beta/rev/df888ea3ce68

A fantastic release manager
(Assignee)

Updated

3 years ago
Assignee: nobody → sledru
Attachment #8443532 - Flags: review?(rail) → review+
Comment on attachment 8443533 [details] [diff] [review]
Add a new field comment

lgtm
Attachment #8443533 - Flags: review?(rail) → review+
Comment on attachment 8446684 [details] [diff] [review]
0003-Manage-the-new-field-comment.patch

lgtm
Attachment #8446684 - Flags: review?(rail) → review+
Comment on attachment 8446685 [details] [diff] [review]
Runner: Send an email when the build of a new release has been requested

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

In overall it looks good. Just a nit below.

::: buildfarm/release/release-runner.ini.example
@@ +7,4 @@
>  [release-runner]
>  notify_from: Release Eng <DONOTreleaseME@mozilla.com>
>  notify_to: Release Duty <you@example.com>
> +notify_to_release: Release Drivers <the-mailing-list@mozilla.org>

Before deploying this we also need to update puppet manifests: http://hg.mozilla.org/build/puppet/file/4b3943871f93/modules/releaserunner/templates/release-runner.ini.erb and add the value to http://hg.mozilla.org/build/puppet/file/4b3943871f93/manifests/moco-config.pp#l198 and http://hg.mozilla.org/build/puppet/file/4b3943871f93/modules/config/manifests/base.pp#l194

::: buildfarm/release/release-runner.py
@@ +215,5 @@
> +
> +    Subject = 'Build of %s' % r["name"]
> +
> +    sendmail(from_=From, to=To, subject=Subject, body=contentMail,
> +             smtp_server="localhost")

Instead of using hard coded "localhost", can you use the same approach as in http://hg.mozilla.org/build/tools/file/1882f6927826/buildfarm/release/release-runner.py#l243. We already have this variable in the configs.
Attachment #8446685 - Flags: review?(rail) → review-
(Assignee)

Comment 15

3 years ago
Created attachment 8449461 [details] [diff] [review]
0001-Send-an-email-when-the-build-of-a-new-release-has-be.patch

right (about the smtp server). I will fix the puppet declaration in a separate patch.
Thanks for the reviews!
Attachment #8446685 - Attachment is obsolete: true
Attachment #8446685 - Flags: review?(bhearsum)
(Assignee)

Updated

3 years ago
Attachment #8449461 - Flags: review?(rail)
(Assignee)

Comment 16

3 years ago
Created attachment 8449465 [details] [diff] [review]
update-puppet-configuration-with-recent-shipit-update.diff

Update of the puppet configuration
Attachment #8449465 - Flags: review?(rail)
Attachment #8449461 - Flags: review?(rail) → review+
Comment on attachment 8449465 [details] [diff] [review]
update-puppet-configuration-with-recent-shipit-update.diff

\o/

Thanks a lot!
Attachment #8449465 - Flags: review?(rail) → review+
Comment on attachment 8449461 [details] [diff] [review]
0001-Send-an-email-when-the-build-of-a-new-release-has-be.patch

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

::: buildfarm/release/release-runner.py
@@ +202,5 @@
> +def sendMailRD(smtpServer, From, To, r):
> +    # Send an email to the mailing after the build
> +    contentMail = ""
> +
> +    comment = r["comment"]

If you change r["comment"] to r.get("comment"), we can land this any time.
(Assignee)

Comment 19

3 years ago
Created attachment 8449561 [details] [diff] [review]
0001-Send-an-email-when-the-build-of-a-new-release-has-be.patch

updated with r.get("comment")
Attachment #8449461 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8449561 - Flags: review?(rail)
Attachment #8449561 - Flags: review?(rail) → review+
Comment on attachment 8449465 [details] [diff] [review]
update-puppet-configuration-with-recent-shipit-update.diff

remote:   https://hg.mozilla.org/build/puppet/rev/aa7cad043948
remote:   https://hg.mozilla.org/build/puppet/rev/bc947fba5228
Attachment #8449465 - Flags: checked-in+
Comment on attachment 8449561 [details] [diff] [review]
0001-Send-an-email-when-the-build-of-a-new-release-has-be.patch

https://hg.mozilla.org/build/tools/rev/5e3c477a2afc
Attachment #8449561 - Flags: checked-in+
Comment on attachment 8449465 [details] [diff] [review]
update-puppet-configuration-with-recent-shipit-update.diff

Follow up fix:
remote:   https://hg.mozilla.org/build/puppet/rev/d81af09882b8
remote:   https://hg.mozilla.org/build/puppet/rev/8218472050c9
The releaserunner part is in production now. We should see emails tomorrow.
We got two emails about 31.0b7 build1 starting, because there was a release-runner failure and it ran again after the config was corrected. I suggest we move sendMailRD() down to line 409, and only send email once the sendchange has completed successfully.
(In reply to Nick Thomas [:nthomas] from comment #24)
> We got two emails about 31.0b7 build1 starting, because there was a
> release-runner failure and it ran again after the config was corrected. I
> suggest we move sendMailRD() down to line 409, and only send email once the
> sendchange has completed successfully.

The idea was to send this email even if our automation fails. We already send an email once the master gets the sendchange ("Tagging started"). Please correct if I missed something.
(Reporter)

Updated

3 years ago
Attachment #8443532 - Flags: review?(bhearsum) → review+
(Reporter)

Comment 26

3 years ago
Comment on attachment 8446684 [details] [diff] [review]
0003-Manage-the-new-field-comment.patch

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

::: kickoff/templates/includes/releases_submitted.html
@@ +48,5 @@
>   <div class="release_footer"></div>
>  </div><!-- rel -->
>  {% if loop.last %}
> +{{ form.comment.label()|safe }}<br />
> +{{ form.comment(title='Extra information to release-drivers:')|safe }}

This isn't the right place to put this field. The form on this page is to mark releases as ready or delete them. The forms that are used when submitting/editing are in includes/{fennec,firefox,thunderbird}_release.html. I think this is fine otherwise...
Attachment #8446684 - Flags: review?(bhearsum) → review-
(Reporter)

Comment 27

3 years ago
Comment on attachment 8443533 [details] [diff] [review]
Add a new field comment

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

Next time, it would be nicer to put this in the same patch that modifies kickoff/model.py so that they're always in sync re: the latest schema.
Attachment #8443533 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 28

3 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #26)
> ::: kickoff/templates/includes/releases_submitted.html
> @@ +48,5 @@
> >   <div class="release_footer"></div>
> >  </div><!-- rel -->
> >  {% if loop.last %}
> > +{{ form.comment.label()|safe }}<br />
> > +{{ form.comment(title='Extra information to release-drivers:')|safe }}
> 
> This isn't the right place to put this field. The form on this page is to
> mark releases as ready or delete them. 
Actually, it was on purpose. In my mind, comments should be general to a build start. Not specific to a release.
For example, a comment which applies for both Desktop and Mobile like "I am starting the build even if TBPL is not green. I want to go to bed/diving/whatever".

However, if you have a strong feeling about this, I don't mind implementing your request.
(Assignee)

Updated

3 years ago
Attachment #8446684 - Flags: review- → review?(bhearsum)
(Reporter)

Comment 29

3 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #28)
> (In reply to Ben Hearsum [:bhearsum] from comment #26)
> > ::: kickoff/templates/includes/releases_submitted.html
> > @@ +48,5 @@
> > >   <div class="release_footer"></div>
> > >  </div><!-- rel -->
> > >  {% if loop.last %}
> > > +{{ form.comment.label()|safe }}<br />
> > > +{{ form.comment(title='Extra information to release-drivers:')|safe }}
> > 
> > This isn't the right place to put this field. The form on this page is to
> > mark releases as ready or delete them. 
> Actually, it was on purpose. In my mind, comments should be general to a
> build start. Not specific to a release.
> For example, a comment which applies for both Desktop and Mobile like "I am
> starting the build even if TBPL is not green. I want to go to
> bed/diving/whatever".
> 
> However, if you have a strong feeling about this, I don't mind implementing
> your request.

I think there's an argument to be made both ways, but if RelMan thinks this is more useful than attaching comments at submission time, I'm fine with it - y'all are the users!

However, if you go with this can you pretty it up a touch? I think the comment field needs some container, rather than sitting unaligned with the releases.
(Reporter)

Updated

3 years ago
Attachment #8446684 - Flags: review?(bhearsum) → feedback+
(Assignee)

Comment 30

3 years ago
Improve a little the documentation => http://git.mozilla.org/?p=build/release-kickoff.git;a=commit;h=dba423e28effa308140530c6130492d4776afc9d
add a new field 'comment' to the tables => http://git.mozilla.org/?p=build/release-kickoff.git;a=commit;h=a15c2e82f25f0ae016d285b85a674b28d9e8f387
Will requires an update of the tables.
(Assignee)

Updated

3 years ago
Attachment #8443533 - Flags: checked-in+
(Assignee)

Updated

3 years ago
Attachment #8443532 - Flags: checked-in+
(Assignee)

Comment 31

3 years ago
Created attachment 8452983 [details] [diff] [review]
0001-Manage-the-new-field-comment.patch

+ "comment" in the css
+ div in the HTML
it is the same otherwise
Attachment #8446684 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8452983 - Flags: review?(bhearsum)
(Reporter)

Comment 32

3 years ago
Comment on attachment 8452983 [details] [diff] [review]
0001-Manage-the-new-field-comment.patch

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

Much more beautiful, thanks! Feel free to land this. Once all your in progress patches to this repo are landed, I'll push them to production.
Attachment #8452983 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 33

3 years ago
Comment on attachment 8452983 [details] [diff] [review]
0001-Manage-the-new-field-comment.patch

http://git.mozilla.org/?p=build/release-kickoff.git;a=commit;h=9d8327ea3c68e464c7a55c9a37380d86dcc7d809

With the bug number and r=
\o/
Attachment #8452983 - Flags: checked-in+

Updated

3 years ago
Depends on: 1042470
(Reporter)

Comment 34

3 years ago
The Ship It change was pushed to production today. Is this bug all done now?
(Assignee)

Comment 35

3 years ago
Yes. Closing. Thanks.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Depends on: 1074228
You need to log in before you can comment on or make changes to this bug.