Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add step to ensure Buildbot checks out right commit #531

Conversation

@aneeshusa
Copy link
Member

aneeshusa commented Oct 30, 2016

We are using retryFetch=True to have Buildbot retry git fetches if
they error out, as we often encounter intermittent errors (e.g. network
problems).

However, Buildbot will allow not only the first fetch + checkout to
fail, but also the second (final) fetch + checkout to fail as well
(which I consider a bug in Buildbot), making it possible for Buildbot
to run a build on the wrong revision, causing confusion.

Retrying failed fetches is too useful to disable.
Hence, to work around this, add a custom step to all builds that checks
if the commit we requested (the revision property) matches the
actual commit Buildbot checks out, and fail the build if not the case.


This change is Reviewable

@aneeshusa
Copy link
Member Author

aneeshusa commented Oct 30, 2016

cc @jdm
Will cause Buildbot to complain loudly if #530 happens again.

r? @larsbergstrom

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 31, 2016

This is awesome! Thanks for doing this. I'm willing to give this a try - should really help! Just one issue:

  ./buildbot/master/files/config/factories.py:37:16: E711 comparison to None should be 'if cond is not None:'
@aneeshusa aneeshusa force-pushed the aneeshusa:check-that-actual-revision-matches-requested branch from d5aafe4 to 6d76cc2 Oct 31, 2016
@aneeshusa
Copy link
Member Author

aneeshusa commented Oct 31, 2016

Fixed the nit. I haven't seen this happen too many times in the past (I'm hoping it didn't just silently happen!) so I'm not sure how helpful this is, but it does give me more peace of mind.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 31, 2016

The latest upstream changes (presumably #533) made this pull request unmergeable. Please resolve the merge conflicts.

We are using `retryFetch=True` to have Buildbot retry git fetches if
they error out, as we often encounter intermittent errors (e.g. network
problems).

However, Buildbot will allow not only the first fetch + checkout to
fail, but also the second (final) fetch + checkout to fail as well
(which I consider a bug in Buildbot), making it possible for Buildbot
to run a build on the wrong revision, causing confusion.

Retrying failed fetches is too useful to disable.
Hence, to work around this, add a custom step to all builds that checks
if the commit we requested (the `revision` property) matches the
actual commit Buildbot checks out, and fail the build if not the case.
@aneeshusa aneeshusa force-pushed the aneeshusa:check-that-actual-revision-matches-requested branch from 6d76cc2 to de46d64 Oct 31, 2016
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Nov 1, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 1, 2016

📌 Commit de46d64 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Nov 1, 2016

Test exempted - status

@bors-servo bors-servo merged commit de46d64 into servo:master Nov 1, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test exempted
Details
bors-servo added a commit that referenced this pull request Nov 1, 2016
…uested, r=larsbergstrom

Add step to ensure Buildbot checks out right commit

We are using `retryFetch=True` to have Buildbot retry git fetches if
they error out, as we often encounter intermittent errors (e.g. network
problems).

However, Buildbot will allow not only the first fetch + checkout to
fail, but also the second (final) fetch + checkout to fail as well
(which I consider a bug in Buildbot), making it possible for Buildbot
to run a build on the wrong revision, causing confusion.

Retrying failed fetches is too useful to disable.
Hence, to work around this, add a custom step to all builds that checks
if the commit we requested (the `revision` property) matches the
actual commit Buildbot checks out, and fail the build if not the case.

<!-- Reviewable:start -->
---

This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/531)

<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.