Prevent triggering autoland while autoland is in progress

RESOLVED FIXED

Status

P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mcote, Assigned: mdoglio)

Tracking

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
If someone triggers autoland for a commit series, we need to be sure no one else can trigger one while it's in progress.  This state (autoland in progress) needs to be displayed in the UI as well.
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

3 years ago
Created attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

mozreview: prevent concurrent autoland requests (bug 1220231); r?dminor
Attachment #8685476 - Flags: review?(dminor)

Comment 2

3 years ago
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

https://reviewboard.mozilla.org/r/24797/#review22335

This looks good to me, but I'm not familiar enough with django's cache behaviour to feel comfortable giving a "ship-it". I'll flag smacleod for review in bugzilla.

::: pylib/mozreview/mozreview/autoland/resources.py:167
(Diff revision 1)
>              # Rather than hard coding the destination it would make sense
>              # to extract it from metadata about the repository. That will have
>              # to wait until we fix Bug 1168486.

This comment is no longer relevant, please remove it while you're here.
Attachment #8685476 - Flags: review?(dminor)

Updated

3 years ago
Attachment #8685476 - Flags: review?(smacleod)
(Assignee)

Comment 3

3 years ago
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/1-2/
Attachment #8685476 - Attachment description: MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?dminor → MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?smacleod
(Assignee)

Comment 4

3 years ago
Created attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

mozreview: remove outdated comment; r?dminor
Attachment #8685944 - Flags: review?(dminor)

Updated

3 years ago
Attachment #8685944 - Flags: review?(dminor) → review+

Comment 5

3 years ago
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

https://reviewboard.mozilla.org/r/24925/#review22447

thanks!

Comment 6

3 years ago
:mdoglio, one way of testing this would be to use the treestatus container to close the trees so that Autoland can't land anything while the test is running.

There are mach commands for this, check out autoland/tests/test-autoland-closed-tree.t, but I guess you'll have to adapt the mach stuff for selenium.
(Assignee)

Comment 7

3 years ago
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/2-3/
(Assignee)

Comment 8

3 years ago
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/1-2/
(Assignee)

Comment 9

3 years ago
Created attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

mozreview: add test for concurrent autoland requests (bug 1220231); r?dminor
Attachment #8688476 - Flags: review?(dminor)

Comment 10

3 years ago
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

https://reviewboard.mozilla.org/r/25329/#review22849

We could also put the treestatus functions in the superclass in unittest.py but I'm ok with not doing that until we have a second test which uses them.
Attachment #8688476 - Flags: review?(dminor) → review+
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

https://reviewboard.mozilla.org/r/24797/#review22843

looks good, comments can be addressed on commit.

::: pylib/mozreview/mozreview/autoland/resources.py:42
(Diff revision 3)
> +    return cache.add(lock_id, "true")

as per irc, we should explictly set the cache entry to expire after 24 hours

::: pylib/mozreview/mozreview/autoland/resources.py:51
(Diff revision 3)
> +def get_autoland_lock(review_request_id, repository_url, revision):

nit: i think this would be clearer if it were named "get_autoland_lock_id"
Attachment #8685476 - Flags: review+
(Assignee)

Comment 12

3 years ago
https://reviewboard.mozilla.org/r/24797/#review22857

::: pylib/mozreview/mozreview/autoland/resources.py:42
(Diff revision 3)
> +    return cache.add(lock_id, "true")

just marking this as an issue

::: pylib/mozreview/mozreview/autoland/resources.py:51
(Diff revision 3)
> +def get_autoland_lock(review_request_id, repository_url, revision):

just marking this as an issue
(Assignee)

Comment 13

3 years ago
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/3-4/
(Assignee)

Comment 14

3 years ago
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/2-3/
(Assignee)

Comment 15

3 years ago
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25329/diff/1-2/
(Assignee)

Comment 16

3 years ago
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/4-5/
Attachment #8685476 - Attachment description: MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?smacleod → MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob
Attachment #8685476 - Flags: review?(smacleod)
(Assignee)

Comment 17

3 years ago
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/3-4/
Attachment #8685944 - Attachment description: MozReview Request: mozreview: remove outdated comment; r?dminor → MozReview Request: mozreview: remove outdated comment; r=dminor
(Assignee)

Updated

3 years ago
Attachment #8688476 - Attachment description: MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r?dminor → MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor
(Assignee)

Comment 18

3 years ago
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25329/diff/2-3/
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.