Open Stack

On Sun, Jul 19, 2015 at 09:42:20PM -0400, Emilien Macchi wrote:
>I'm currently in holidays but I could not resist to take some time and
>reply.

Thanks for taking the time, apologies about distracting you from your 
vacation!

>Please read again my first comment on the Governance patch:
>
>"They are making progress and I'm sure they are doing their best."

Ok, I'm happy to drop this topic and focus on the positive message here! 
One way or the other, commits are a better way to measure progress than 
emails :)

>Andrew took the best metric in your advantage... the review metric,
>which is something given to anyone having signed the CLA.

Review metric is just what Stackalytics shows by default, if Andrew were 
shopping for the best metric he'd have used patch sets.

>I would rather focus on patchset, commits, bug fix, IRC, ML etc... which
>really show collaboration in a group.
>And I honestly think it's making progress, even though the numbers.

Patch set count is naturally the first metric to show progress. Fuel 
team's bugs triage process is already fairly mature, so I'm not 
surprised there's noticeable contribution in that area, too.

Keeping that up and pushing for more IRC & ML participation should help
improve the patch set to commit ratio, which would make Fuel team more 
productive and would improve the mutual trust, helping Puppet OpenStack 
core reviewers trust that Fuel developers will not mess things up for 
them, and helping Fuel developers trust that their commits will not get 
stuck on review.

I think this should be our primary focus of improvement in the near 
term. Once there's evidence that Fuel developers can produce quality 
patches that can be landed relatively quickly, next level would be
improving our +1/-1 ratio and reviewing specs for blueprints.

>> [3] https://review.openstack.org/198119
>>
>> (...)
>> Even before looking into the review comments, I could see a technical 
>> reason for abandoning the commit: if there is a bug in a component, 
>> fixing that bug in the package is preferrable to fixing it in puppet, 
>> because it allows anybody to benefit from the fix, not just the 
>> people deploying that package with puppet. 
>
>You are not providing official Ubuntu packaging, but your own packages
>mainly used by Fuel, while Puppet OpenStack modules are widely used by
>OpenStack community.

The only reason we've been doing our own packaging is lack of an 
official cross-distro upstream. We've already opened access to all our 
packaging code on review.fuel-infra.org, and want to move our packaging 
work directly to upstream as soon as it is becomes possible:

http://lists.openstack.org/pipermail/openstack-dev/2015-July/069377.html

>Fixing that bug in Fuel packaging is the shortest & easiest way for you
>to fix that,

It's neither shortest nor easiest: it's 26 changed lines in 4 files, vs 
4 changed lines in 2 files, and it was created only after we were told 
in the puppet-horizon review that packaging is the right place to fix 
it, so it was additional work on top of a workaround that we already 
had.

>while we are really doing something wrong in puppet-horizon
>about the 'compress' option.
>So Fuel is now fixed and puppet-horizon broken.

No, puppet-horizon simply lacks a workaround for broken horizon 
packages, and that workaround also was reverted from fuel-library. 
Mirantis may be the first to fix their packages, but if other distros 
such as RDO or Ubuntu have the same problem in their packages, they 
should fix it on their end, instead of relying on puppet, chef, ansible, 
and other deployment tools to implement workarounds for their bugs.

It's a general "right tools for the job" problem. You can do a lot of 
things with Puppet, but it doesn't mean that you should use Puppet for 
everything. Managing options in a service's config files is Puppet's 
job. Replacing package's init script, tweaking its directory structure, 
creating symlinks, compiling binaries from code and so on should all be 
done while you're building a package, not after you have installed it.

>> [4] https://review.openstack.org/190548
>>
>> Here's what I see in this review:
>>
>> a) Fuel team has spent more than a month (since June 11) on trying to 
>> land this patch. 
>>
>> b) 2 out of 5 negative reviews are from Fuel team, proving that Fuel 
>> developers are not "rubberstamping" each other's commits as was 
>> implied by Emilien's comments on the TC review above. 
>>
>> c) There was one patch set that was pushed 3 business days after a negative
>> review, all other patch sets (11 total) were pushed no later than next day
>> after a negative review.
>>
>> All in all, I see great commitment and effort on behalf of Fuel team,
>> eventually awarded by a +2 from Michael Chapman.
>>
>> On the same day (June 30), Emilien votes -2 for a correction in a 
>> comment, and walks away from the review for good. 18 days and 4 patch 
>> sets and 2 outstanding +1's later, the review remains blocked by that 
>> -2. Reaching out on #puppet-openstack [5] didn't help, either. 
>>
>> [5] http://irclog.perlgeek.de/puppet-openstack/2015-07-08#i_10867124
>
>I -2 for this reason: this patch already had a +2 so it was closed to be
>merged by someone else, while the patch was *breaking backward
>compatiblity* in puppet-horizon, because Vasyl was changing the default
>cache driver to force users to use Memcached by default.
>We *never* break backward compatibility (this is a general rule in
>OpenStack) and *never* change OpenStack default configuration (general
>rule in Configuration management), but rather provide the interface
>(called Puppet parameters) to change it.

That's fair enough, and on July 8 we pushed patch set 10 that has fixed 
this problem, and set the default to LocMemCache. AFACT at this point 
you've lost track of this commit because your gerrit dashboard excludes 
all commits that have a -2 vote on them, so it got stuck in limbo for 
two weeks.

I still disagree with your use of -2 vote here, a -1 from a core 
reviewer should be enough to prevent other cores from merging a patch 
set, and should tell them what to watch out for in subsequent patch 
sets.

But anyway, adding a Disagreement section to the gerrit dashboard, and 
going through the reviews that come up in it should help mitigate this 
problem.

>> Fuel team can commit to reporting bugs and posting patches, Fuel team 
>> can spend reasonable amount of time and effort to respond to review 
>> comments, but all that is worth nothing if there's no commitment from 
>> Puppet OpenStack team to review and merge these patches in a timely 
>> manner. 
>
>Please consider our group is part of a few people comparing to Fuel's
>team, and we are not all working 100% on Puppet OpenStack (I'm lucky, I
>do). Please take that in consideration.

I understand that, and I know that most OpenStack projects are 
struggling with keeping their review queue under control. As I've 
mentioned, I understand that right now influx of patches from Fuel team 
increases the burden on Puppet OpenStack core reviewers (both due to 
core/non-core ratio, and due to patches/commits ratio). Hopefully it 
won't be long before some Fuel developers gain enough knowledge and 
reputation to start spreading out that additional burden.

>We have weekly review/bug triage, feel free to participate, it happens
>during our meetings.

We will, thanks.

>Please don't get me wrong, but you're aggravating the situation.

I hope you meant "exagerrating" :)

>We are having much more collaboration than before.
>Fuel's team is much more involved on reviews as you highlighted, and
>also on IRC, which is good.
>Of course there are still technical things that will be improved with
>more experience in working with Puppet OpenStack modules, but so far all
>Puppet grouped noticed more collaboration I think.
>
>I totally assume my -1 for adding Fuel to OpenStack because it's too
>early now, even all your recent efforts.
>
>Now, I think we just need you to let us more time, so your engineers and
>our group can work together. I'm pretty sure the things will go
>naturally like they happened for our other contributors.
>
>Rather than doing yet another very long thread, maybe can we arrange a
>public meeting on IRC and we can do some triage together. After my
>holidays, I can even organize a 1 or 2 hour meeting so we can solve our
>first blockers and make progress on both sides. I'll try to get
>attention from our group if some help is needed.
>What do you think?

I'll try to get a better attendance from Fuel contributors on the Puppet 
OpenStack weekly meeting, hopefully that and gerrit dashboard tweaks 
would be enough to improve the situation. I think we should do another 
review of our collaboration around end of July, after two more weekly 
meetings.

-- 
Dmitry Borodaenko