<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 26, 2015 at 1:10 PM, Walter A. Boring IV <span dir="ltr"><<a href="mailto:walter.boring@hp.com" target="_blank">walter.boring@hp.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey folks,<br>
Today we found out that a patch[1] that was made against our lefthand driver caused the driver to fail. The 3rd party CI that we have setup<br>
to test our driver (hp_lefthand_rest_proxy.py) caught the CI failure and reported it correctly[2]. The patch that broke the driver was reviewed and<br>
approved without a mention of 3rd Party CI failures.<br>
<br>
This is a perfect example of 3rd Party CI working perfectly and catching a failure, and being completely ignored by everyone<br>
involved in the review process for the patch<br>
<br>
I know that 3rd party CI isn't perfect, and has been ripe with false failures, which is one of the reasons why they aren't voting today.<br>
But, that being said, if patch submitters aren't even looking at the failures for CI when they are touching drivers that they don't maintain, and reviewers<br>
aren't looking at the CI failures, then why are we even doing 3rd party CI?<br>
<br>
Our team is partially responsible for not seeing the failure as well. We should be watching the CI failures closely, but we are doing the best we<br>
can. There are enough patches for Cinder ongoing at any one time, that we simply can't watch every single one of them for failures. We did eventually<br>
see that every single patchset in gerrit was now failing against our driver, and this is how we caught it. Yes, it was a bit after the fact, but we did notice<br>
it and now have a new patch up that fixes it. So, in that regard 3rd party CI did eventually vet out a problem that our team caught.<br>
<br>
How can we prevent this in the future?<br>
1) Make 3rd party CI voting. I don't believe this is ready yet.<br></blockquote><div> </div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Agreed, look at the history on that driver (and many others) and you'll see we are in no way ready for that.</div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) Authors and reviews need to look at 3rd party CI failures when a patch touches a driver. If a failure is seen, contact the CI maintainer and work with them and<br>
see if the failure is related to the patch, if it's not obvious. In this case, the failure was obvious. The import changed, and now the package can't find the module.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">If things were more "stable" yeah, I might, but the reality is as I've pointed out we have a serious signal to noise ratio problem IMO</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
3) CI maintainers watch every single patchset and report -1's on reviews? (ouch)<br>
4) ?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Option 4 in my opinion is exactly what I've been doing since this started. I receive a notification for any change that my CI setup fails on, and then it's up to me to go and verify if something is truly broken or if it's my system that's messed up. It's not perfect and it's not really a true CI but it is a continuous test system which for me is what's most important here. I completely understand that's not the case for other things.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"><br></div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">This is where the churn of typo fixes, hacking changes etc can bight us. Sure, while reviewing that probably could've/should've been caught. The problem is for me at least if we're going to do this sorts of semantic changes that touch so many files I'm likely to be half asleep before I get to the "cinder/volume" section. Doesn't make it right, but kinda how it is.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"><br></div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Anyway, yeah it sucks, but I'd argue this worked out GREAT. A change was made that broke things in LHN driver, but historically nobody would've known until a customer tried to use it. In this case, the problem was found in less than a day and fixed. That's a pretty huge win in my opinion!</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"><br></div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Thanks,</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">John</div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<br>
Here is the patch that broke the lefthand driver[1]<br>
Here is the reported failure in the c-vol log for the patch by our 3rd party CI system[2]<br>
Here is my new patch that fixes the lefthand driver again.[3]<br>
<br>
[1] <a href="https://review.openstack.org/#/c/145780/" target="_blank">https://review.openstack.org/#<u></u>/c/145780/</a><br>
[2] <a href="http://15.126.198.151/80/145780/15/check/lefthand-iscsi-driver-master-client-pip-dsvm/3927e3d/logs/screen-c-vol.txt.gz?level=ERROR" target="_blank">http://15.126.198.151/80/<u></u>145780/15/check/lefthand-<u></u>iscsi-driver-master-client-<u></u>pip-dsvm/3927e3d/logs/screen-<u></u>c-vol.txt.gz?level=ERROR</a><br>
[3] <a href="https://review.openstack.org/#/c/159586/" target="_blank">https://review.openstack.org/#<u></u>/c/159586/</a><br>
<br>
$0.02<br>
Walt<br>
<br>
______________________________<u></u>______________________________<u></u>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.<u></u>openstack.org?subject:<u></u>unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
</blockquote></div><br></div></div>