checkBuilds() returns true even when builds are failing?

[ghost]

https://github.com/kubernetes/contrib/blob/023e2c05df5899bf9734b0d6d21fad0b0617fb45/submit-queue/e2e.go#L93

As far as I can tell by looking at this code, this function returns true even if stable==false for some or all builds.

In theory this will cause PR's to be merged even if Jenkins e2e tests are failing.

@gmarek has reported this to be the case and @lavalamp has therefore disabled the merge bot.

Need to verify that this theoretical bug is in fact manifesting. The fix is pretty straightforward.

@brendandburns FYI.

[eparis]

Isn't that why @brendandburns filed #111

[gmarek]

Indeed it is. We can keep this issue as a reminder:)

[ghost]

Before I forget, we should only merge PR's if Jenkins e2e tests are stable across multiple runs. If I'm reading the code correctly, it deems a single successful test run per job to constitute a "stable" job. In theory, a single fluky success could cause all pending PR's to auto-merge, which is bad.

In practise, we have multiple test jobs being checked, so if all of them become flaky, their intermittent successes would all have to line up correctly for PR's to merge (which is statistically unlikely). But even so, I'd suggest adding a "number of consecutive successes required" config value per job. Faster, more reliable test jobs (e.g. e2e-gce-parallel, at 20 min and 90%+ reliability) might require, say, 4 consecutive successes, while slower jobs (e.g. scalability, at multiple hours per fun), might require fewer.

I'll open a separate issue...