1004617 - end_to_end_reconfig.sh should also update bug status once they are in production

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Pete Moore [:pmoore][:pete]]

In the end-to-end reconfig scripts that we have, we already perform the reconfig and update the wiki, but currently don't update the bug status for the bugs that get rolled out to our buildmasters. Would be good to do this automatically.

Comment 1

[Armen [:armenzg]]

This bug would help buildduty, however, it is a tools component type of bug.

Comment 2

[Pete Moore [:pmoore][:pete]]

See: https://bugzilla.mozilla.org/show_bug.cgi?id=1027394#c9

Comment 3

[Pete Moore [:pmoore][:pete]]

This would be good for a contributor. Unassigning to me so it will hopefully be picked up by http://www.joshmatthews.net/bugsahoy/?sh=1

end_to_end_reconfig.sh script is here:
https://hg.mozilla.org/build/tools/file/tip/buildfarm/maintenance/end_to_end_reconfig.sh

I'm happy to provide full explanations to anybody that wants to get involved with this bug.

Essentially, the reconfig procedure is how we deploy changes to our build infrastructure, when there are changes to the source repositories that contain all the code and configs for our (buildbot) build infrastructure.

After we deploy changes to the build infrastructure, we update all the bugzilla bugs (which were referenced in the new hg changesets we picked up) with a comment that the changes have landed in production.

We also publish a wiki page: https://wiki.mozilla.org/ReleaseEngineering/Maintenance

Here you can see the list of the bugs that landed with each "reconfig". The challenge here is to iterate through this list of bugs, and call bugzilla api in order to post messages in these bugs, to say that they have landed in production. Currently this is done manually.

Ideally we would also be able to compare the hg changesets we picked up, against any patches attached to the bugzilla bugs, and if they match, to mark the patches as checked in, and add a comment to say which patches have been deployed to production. If no attached patches match, we could attach a new patch to the bug, mark it as checked in, so that from the bug you can see the patch. At the least we could provide a http link to the changeset hosted on the hg server, even if we don't attach the actual patch to the bug.

This also serves as a sanity check - if you attach a patch to the bug, but it is different from the patch that lands in production, you will see it. This patch-matching process might need to be a little fuzzy, since for example you might have a different number of context lines surrounding the changes in the patch, and you may have different headers (such as with/without commit message, including enclosing function names or not, etc).

Comment 4

[Pete Moore [:pmoore][:pete]]

Working on this now!

Comment 5

[Pete Moore [:pmoore][:pete]]

Attachment: bug1004617_tools_v1.patch

Work in progress - completely untested, but I think complete. I'll run a few times before submitting a review request, to catch any errors in testing first.

Comment 6

[Pete Moore [:pmoore][:pete]]

OK, I definitely think we're good to go - just waiting for some changes to come in that we can reconfig!

Latest version is here: https://github.com/petemoore/build-tools/tree/bug1004617

Coop - if you reconfig before me, feel free to clone and try out.

Pete

Comment 7

[Pete Moore [:pmoore][:pete]]

(and this is the diff that I'll land as a review request, assuming the next reconfig(s) go well): https://github.com/petemoore/build-tools/compare/bug1004617?diff=split

Comment 8

[Pete Moore [:pmoore][:pete]]

Attachment: bug1004617_tools_v2.patch

I know.... it's a big patch. Just close your eyes, and select the "r+" from the drop-down menu. ;)

Comment 9

[Pete Moore [:pmoore][:pete]]

Attachment: bug1004617_tools_v3.patch

.. And an *incremental* patch on top of last patch to fix "unbound variable" error, when calling end_to_end_reconfig.sh with no arguments:

pmoore@Elisandra:~/git/tools/buildfarm/maintenance bug1004617 $ end_to_end_reconfig.sh 
/Users/pmoore/git/tools/buildfarm/maintenance/end_to_end_reconfig.sh: line 131: @: unbound variable
  * Start timestamp: 1413553229

Comment 10

[Chris Cooper [:coop] (he/him)]

Comment on attachment 8506969 [details] [diff] [review]
bug1004617_tools_v2.patch

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

I may find nits from running it, but looks good to me (with the follow-up patch).

Comment 11

[Chris Cooper [:coop] (he/him)]

One problem I found this afternoon:

I was already running under a venv, so when I ran the script I got the following error when manage_masters was called:

Traceback (most recent call last):
  File "./manage_masters.py", line 6, in <module>
    from fabric.api import env
ImportError: No module named fabric.api

I deactivated my venv to try again. The merge had already happened at that point, and I didn't want to lose my wiki and bugzilla update (since that's the point of your patch), so I added a continue to the very start of the for loop in merge_to_production(). Unfortunately, on resuming the reconfig script skipped the actual manage_masters part and jumped directly to the wiki and bugzilla updates (which did work correctly). I'm running the actual reconfig part by hand now.

Comment 12

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8506969 [details] [diff] [review]
bug1004617_tools_v2.patch

https://hg.mozilla.org/build/tools/rev/77ed75a0cfe1

Comment 13

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8506990 [details] [diff] [review]
bug1004617_tools_v3.patch

https://hg.mozilla.org/build/tools/rev/77ed75a0cfe1

Comment 14

[Pete Moore [:pmoore][:pete]]

(In reply to Chris Cooper [:coop] from comment #11)
> One problem I found this afternoon:
> 
> I was already running under a venv, so when I ran the script I got the
> following error when manage_masters was called:
> 
> Traceback (most recent call last):
>   File "./manage_masters.py", line 6, in <module>
>     from fabric.api import env
> ImportError: No module named fabric.api
> 
> I deactivated my venv to try again. The merge had already happened at that
> point, and I didn't want to lose my wiki and bugzilla update (since that's
> the point of your patch), so I added a continue to the very start of the for
> loop in merge_to_production(). Unfortunately, on resuming the reconfig
> script skipped the actual manage_masters part and jumped directly to the
> wiki and bugzilla updates (which did work correctly). I'm running the actual
> reconfig part by hand now.

Thanks Coop for this feedback.

I see the bug - essentially, we loop through dependencies, and if we find one missing, we do this:

if already have created a virtualenv, add the new dependency to our existing virtualenv, else:
create the virtualenv, and then add the dependency.

In the use case you provided, we first checked fabric was available, which is was, but then requests was not - at which point a new virtualenv was created. However, this new virtualenv did not have fabric, but had already passed the fabric check. In other words, if we create a virtualenv, we should iterate through the loop from scratch. d'oh!

Comment 15

[Chris Cooper [:coop] (he/him)]

(In reply to Pete Moore [:pete][:pmoore] from comment #14)
> In the use case you provided, we first checked fabric was available, which
> is was, but then requests was not - at which point a new virtualenv was
> created. However, this new virtualenv did not have fabric, but had already
> passed the fabric check. In other words, if we create a virtualenv, we
> should iterate through the loop from scratch. d'oh!

How about I write a patch to allow for skipping the venv creation entirely? The script would still check to make sure the modules it needs are available, but would simply error out if it can't find them.

I think this makes sense in a managed environment anyway since you want to be running known-good versions of tools rather than relying on whatever pip decides to download.

Comment 16

[Chris Cooper [:coop] (he/him)]

(In reply to Chris Cooper [:coop] from comment #15)
> How about I write a patch to allow for skipping the venv creation entirely?
> The script would still check to make sure the modules it needs are
> available, but would simply error out if it can't find them.
> 
> I think this makes sense in a managed environment anyway since you want to
> be running known-good versions of tools rather than relying on whatever pip
> decides to download.

Meh, don't block on this. The easier answer is just to make sure my local venv has the modules installed for now.

Comment 17

[Pete Moore [:pmoore][:pete]]

Attachment: bug1004617_tools_v4.patch

Fixes problem discussed in comment 11, comment 14, comment 15, comment 16 above. Incremental patch for this matter only, since previous patches have already landed.

Comment 18

[Chris Cooper [:coop] (he/him)]

Comment on attachment 8509437 [details] [diff] [review]
bug1004617_tools_v4.patch

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

Thanks, Pete.

Comment 19

[Chris Cooper [:coop] (he/him)]

Callek pointed out that it would be better if we only updated a given bug once (with all the relevant changesets), even if multiple patches were merged. 

See https://bugzilla.mozilla.org/show_bug.cgi?id=1087013 for an example of the multiple updates (comments #6 and #7).

Comment 20

[Pete Moore [:pmoore][:pete]]

(In reply to Chris Cooper [:coop] from comment #19)
> Callek pointed out that it would be better if we only updated a given bug
> once (with all the relevant changesets), even if multiple patches were
> merged. 
> 
> See https://bugzilla.mozilla.org/show_bug.cgi?id=1087013 for an example of
> the multiple updates (comments #6 and #7).

Originally I was thinking about matching the attachment to the diff, so that the posted comments would be against the attachments (maybe we could also add an "in production" flag - e.g. to complement the "checked in" per attachment flag). However, matching can be a nasty fuzzy affair, so then I started thinking more about an auto-land alternative - i.e. you attach a patch for tools/bb-configs/bb-custom, it gets r+'d, and we auto-land it.

Advantages are:
1) Less human steps, so roll out is quicker
2) Matching is easier

I've heard talk to autoland type tools - is this covered by an existing autoland project? If not, should it be? Or is this not worth considering.

Refactoring to put multiple changes into a single comment might move against this strategy of updating individual attachments, as if we do end up matching attached patches to merged commits, I think we would want to add a comment to each attachment, to add the url or the merge commit, potentially the url of the source commit on default (if it has not been added) and then set the "in production" and possibly also the "checked in" flags against the attachment. But a lot of this depends on which model we choose - matching existing patches against discovered commits, or auto-committing/landing bug attachments.

Comment 21

[Pete Moore [:pmoore][:pete]]

(In reply to Pete Moore [:pete][:pmoore] from comment #20)
> I've heard talk to autoland type tools - is this covered by an existing
> autoland project? If not, should it be? Or is this not worth considering.

I found https://wiki.mozilla.org/ReleaseEngineering:Autoland - it looks like this is something different, for try pushes, and not tied to the review cycle.

One extra complication is that roll out of changes can often be tied to other changes landing, or are time sensitive. So auto-landing patches might often not be wanted. Maybe another kind of bugzilla integration would be cool - like, when you have a patch r+'d, you could see it in your Bugzilla Dashboard, and auto-land it from there yourself.

Comment 22

[Pete Moore [:pmoore][:pete]]

I've created a bugzilla feature bug for comment 21 (bug 1089570), to explain a proposed workflow, and see if it is possible to integrate this directly into bugzilla functionality.

Comment 23

[Pete Moore [:pmoore][:pete]]

Attachment: bug1004617_tools_show-bugzilla-curl-commands_v1.patch

So .... today I did a reconfig and bugzilla did not get updated. Turns out to be a bugzilla bug, which I will raise separately. However it highlighted:

1) We are not outputting much when we publish to bugzilla
2) We are not testing that the publish was successful

This first patch loosely addresses issue 1) in that it at least outputs the curl command that it is essentially running via python requests module.

Comment 24

[Pete Moore [:pmoore][:pete]]

(In reply to Pete Moore [:pete][:pmoore] from comment #23)
> Created attachment 8516659 [details] [diff] [review]
> bug1004617_tools_show-bugzilla-curl-commands_v1.patch
> 
> So .... today I did a reconfig and bugzilla did not get updated. Turns out
> to be a bugzilla bug, which I will raise separately.

See bug 1093600

Comment 25

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8516659 [details] [diff] [review]
bug1004617_tools_show-bugzilla-curl-commands_v1.patch

https://hg.mozilla.org/build/tools/rev/046e9d5ec108

Comment 26

[Pete Moore [:pmoore][:pete]]

Given that we are hoping to get rid of reconfigs altogether this quarter, and are moving towards task cluster rather than investing in buildbot much more, I think the outstanding optimisation of having a single comment post rather than one per commit is not worth the investment. So let's mark this as fixed. We are already updating bugs with reconfigs automatically, so I think we can say we resolved this bug (and the one-comment-per-reconfig is really an enhancement request on top of the original scope of this bug).

Pete