<div dir="ltr"><div>Aleksandra,<br><br></div>As you may know, there is a randomly failing nailgun unit test in fuel-web repo, which fails for the major part of review requests. It's been happening for a few days. But I need to merge some stuff and cannot wait for the fix of this well known issue. So for every request with -1 from Fuel CI I write an explanation why I decided to merge the request. Are you ok with this? Here is an example: <a href="https://review.openstack.org/#/c/131079/">https://review.openstack.org/#/c/131079/</a><br></div><div class="gmail_extra"><br><div class="gmail_quote">2014-10-28 23:10 GMT+07:00 Aleksandra Fedorova <span dir="ltr"><<a href="mailto:afedorova@mirantis.com" target="_blank">afedorova@mirantis.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi everyone,<br>
<br>
with recent disruption in our CI process I'd like to discuss again the<br>
issues in our merge workflow.<br>
<br>
See the summary at the end.<br>
<br>
<br>
As a starting point, here is the list of patches which were merged<br>
into fuel-library repository without "Verified +1" label from Fuel CI:<br>
<br>
<a href="https://review.openstack.org/#/q/project:stackforge/fuel-library+AND+status:merged+AND+NOT+label:Verified%252B1%252Cuser%253Dfuel-ci,n,z" target="_blank">https://review.openstack.org/#/q/project:stackforge/fuel-library+AND+status:merged+AND+NOT+label:Verified%252B1%252Cuser%253Dfuel-ci,n,z</a><br>
<br>
And the list of merged patches with explicit "Verified -1" label:<br>
<br>
<a href="https://review.openstack.org/#/q/project:stackforge/fuel-library+AND+status:merged+AND+label:Verified-1%252Cuser%253Dfuel-ci,n,z" target="_blank">https://review.openstack.org/#/q/project:stackforge/fuel-library+AND+status:merged+AND+label:Verified-1%252Cuser%253Dfuel-ci,n,z</a><br>
<br>
There are two common reasons I know why these patchsets exist:<br>
<br>
Case 1: "Who cares about CI anyway".<br>
<br>
Case 2: These patches can not pass CI because of some real reason,<br>
which makes Fuel CI result irrelevant.<br>
<br>
I am not sure, if i need to comment on the first one, but please just<br>
remember: CI is not a devops playground made to disrupt your otherwise<br>
clean and smooth development process. It is an extremely critical<br>
service, providing the clear reference point for all the work we do.<br>
And we all know how important the reference point is [1].<br>
<br>
So let's move on to the Case 2 and talk about our CI limitations and<br>
what could possibly make the test result irrelevant.<br>
<br>
1) Dependencies.<br>
<br>
Let's say you have a chain of dependent patchsets and none of them<br>
could pass the CI on its own. How do you merge it?<br>
<br>
Here is the trick: the "leaf", i.e. last, topmost patch in the chain<br>
should pass the CI.<br>
<br>
The test we run for this patchset automatically pulls all dependencies<br>
involved. Which makes Fuel CI result for this patchset perfectly<br>
relevant for the whole chain.<br>
<br>
2) Environment.<br>
<br>
Fuel CI test environment usually uses slightly outdated version of<br>
Fuel iso image and fuel-main code. Therefore it happens that you write<br>
and test your patch against latest code via custom iso builds and it<br>
works, but it can not pass CI. Does it make test results irrelevant?<br>
No. It makes them even more valuable.<br>
<br>
CI environment can be broken and can be outdated. This is the part of<br>
the process. To deal with these situations we first need to fix the<br>
environment, then run tests, and then merge the code.<br>
<br>
And it helps if you contact devops team in advanceĀ  and inform us that<br>
you soon will need the ISO with this particular features.<br>
<br>
3) ?<br>
<br>
Please add your examples and let's deal with them one by one.<br>
<br>
<br>
Summary:<br>
<br>
I'd like to propose the following merge policy:<br>
<br>
1. any individual patchset MUST have +1 from Fuel CI;<br>
<br>
2. any chain of dependent patchsets MUST have +1 from Fuel CI for the<br>
topmost patch;<br>
<br>
3. for all exceptional cases the person who does the merge MUST<br>
explicitly contact devops team, and make sure that there will be<br>
devops engineer available who will run additional checks before or<br>
right after the merge. The very same person who does the merge also<br>
MUST be available for some time after the merge to help the devops<br>
engineer to deal with the test failures if they appear.<br>
<br>
<br>
<br>
[1] <a href="http://www.youtube.com/watch?feature=player_embedded&v=QkCQ_-Id8zI#t=211" target="_blank">http://www.youtube.com/watch?feature=player_embedded&v=QkCQ_-Id8zI#t=211</a><br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
--<br>
Aleksandra Fedorova<br>
Fuel Devops Engineer<br>
bookwar<br>
<br>
--<br>
You received this message because you are subscribed to the Google Groups "fuel-core-team" group.<br>
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="mailto:fuel-core-team%2Bunsubscribe@mirantis.com">fuel-core-team+unsubscribe@mirantis.com</a>.<br>
For more options, visit <a href="https://groups.google.com/a/mirantis.com/d/optout" target="_blank">https://groups.google.com/a/mirantis.com/d/optout</a>.<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr">Vitaly Kramskikh,<br>Software Engineer,<br>Mirantis, Inc.</div>
</div>