Thinking about how to merge more, quicker (Was Re: [ansible-devel] Further adventures in variable resolution.)

Archives Ansible Developer
1

I think this is an issue that we have to devote some time to
addressing. We've taken some baby steps since the last release by
adding a few people who are more knowledgable about specific areas as
committers to the module repository so that they can care for those
modules that they know about. But no mistake, that was a baby step.
We'll need a lot more steps like that before we get to a place where
we are merging at a sufficient rate.

There've been a few ideas thrown out to the list in the past and it's
probably time to start writing those down and thinking in earnest
about which ones we want to do (and which ones we want to commit to
doing sooner. Even though baby steps don't get you very far per step,
they're really nice because the change doesn't seem as big and scary
as trying to do huge things all at once).

A few of us are meeting in a couple weeks to hack on v2. We probably
shouldn't co-opt the whole hack session with this issue but we could
probably take an hour out of the schedule to think about a new baby
step(s) to take.

In the meantime, if there's any issues that people think have been
given too low of a priority, feel free to bring it to our attention
and we'll take another another look at how it fits into the grand
scheme of things.

-Toshio "one step at a time" Kuratomi

2

Great! I hope I'm able to attend the next one. Looking forward to v2 :slight_smile:

Giovanni

3

Saw this today, http://www.slideshare.net/jimi-c/whats-new-in-v2-ansiblefest-london-2015 looks very promising.

One step at a time sounds like a good way to climb a mountain :wink:

A few of us are meeting in a couple weeks to hack on v2. We probably
shouldn’t co-opt the whole hack session with this issue but we could
probably take an hour out of the schedule to think about a new baby
step(s) to take.

Would love to take part in that conversation.

thanks,

Steve.

4

Toshio, you mention that we should be writing down ideas to improve the merge process. Anywhere in particular we should be doing this?

5

This thread seems as good a place as any for people to submit ideas about how to speed up merging without destabilizing the devel branch. A few things already been done/mentioned:

  • Give limited merge privileges to subject matter experts or partnerships (as we have for rax, gce, etc).

  • Create release candidate branches, these would only allow for bugfixes while devel can continue adding features?

  • Leverage a testing infrastructure connected to github PR submission. This save time on both the submitter’s and reviewer’s side.

  • Update the contribution guidelines, be clear about requirements and set expectations.

I am probably forgetting a few and/or hopefully someone out there has great ideas on how to make this much better.

All should also be aware that Ansible inc is also is currently in efforts to expand the core team (more people merging).

6

Is that an Ansible Inc devs meeting, or something else?​

7

This thread seems as good a place as any for people to submit ideas about
how to speed up merging without destabilizing the devel branch. A few
things already been done/mentioned:

* Give limited merge privileges to subject matter experts or partnerships
(as we have for rax, gce, etc).

​This is I believe about the two separate modules repo's. What about other
plugins?​ Given v2 has more plugins, etc.?

* Create release candidate branches, these would only allow for bugfixes
while devel can continue adding features?

​Ack!

<thinking out loud>

Perhaps somet​hing with branches for larger features, where majore core
updates can be tested etc?
This might get related to things like a roadmap, where specific features
get discussed separately?

Perhaps we kind get to a way to better classify certain PR's, where pure
bug resolutions are separated from features, where certain branches track
and integrate those respective PR's, to make it more easy to test a set of
PR's?

</thinking out loud>

I am probably forgetting a few and/or hopefully someone out there h

​​
as great ideas on how to make this much better.

How ​

​about the difference between pure core, and plugins in general?
How about discussing a roadmap for core?​ That might trigger other ideas.

8

Really low hanging fruit here would be to get the tests running in travis and avail of their automated testing for pull requests.

http://docs.travis-ci.com/user/pull-requests/

This would cover the python based testing in core at least and possibly some of the more generic modules, particularly things they provide containers for like datastores (relational dbs, mongo etc) and package tools like gem, pip etc.

Test-kitchen is pretty useful for integration testing of modules and roles though it would be nice if it, or a framework like it, fit better into the ansible ecosystem i.e. was defined with yml and had test fixtures like those in Serverspec to verify behavior. I could see a role having a test folder with plays to spin up a supported environment and run. It would be a great facility to have when evaluating roles from galaxy, looking at the tests you’d see how best to apply a role for an environment and easily take it for a test drive.

One thing I see lacking from test-kitchen is the ability to spin up small clusters of machines as it only works with a single vm at a time. I’ve found testing inherently clustered software like cassandra and consul much easier with small local clusters. Having the ability to easily spin up a small network of machines, apply a role to cluster them and test it within the framework would be huge.

VMs might prove a little difficult to automate in CI, but having the capability for such test driven infrastructure built in would help greatly with development and collaboration. What are folks thoughts on this?

Thanks,

Steve

9

@svg, yes, the core team is meeting in person for a few days to try to
sync on many of these issues and future development.

@Stephen,

What you describe are some of the reasons it is hard to make an
automated PR-> test scenario, it becomes very dependent on the change.
Some are easy to do in single VM, others need clusters, then there is
the testing of the cloud modules which create/spin/destroy vms and
associated resources. You also have to multiply this by the number of
OSs supported. The worse case is 'all of the above' which happens a
lot more than I initially thought.

Ansible Inc has internal setups to do the clustered and cloud tests,
but this costs us real money, it is not viable to make it publicly
accessible, security aside (what do you mean someone pushed a test
play to run bitcoin miners on 20k instances???).

10

OpenStack has gone through a lot of this. Much could be learned from their test infrastructure and contribution practices.

One could split the tests up into phases. First phase automatically ran on incoming PRs would do the base level unit tests and other things that can be done on single system with a reasonable timeout. Then human eyes and acceptance would be needed for second phase testing which would utilize the larger functionality testing across multiple resources. The combination of unit level testing, human review from appropriately authorized people, and fuller functionality testing can be the arbiter for what goes in or not.

- jlk

11

This thread seems as good a place as any for people to submit ideas
about how to speed up merging without destabilizing the devel branch.
A few things already been done/mentioned:

* Give limited merge privileges to subject matter experts or
partnerships (as we have for rax, gce, etc).

* Create release candidate branches, these would only allow for
bugfixes while devel can continue adding features?

Speaking with both my fedora/epel maintainer and Fedora infrastructure
hats on, this would be great. I don't really have time to follow
upstream devel too much, so the first time I get to test things with
our playbooks is when a new release comes out. Having RC's to test
would be great and could also save releases from last minute issues.

* Leverage a testing infrastructure connected to github PR
submission. This save time on both the submitter's and reviewer's
side.

* Update the contribution guidelines, be clear about requirements and
set expectations.

I am probably forgetting a few and/or hopefully someone out there has
great ideas on how to make this much better.

All should also be aware that Ansible inc is also is currently in
efforts to expand the core team (more people merging).

I wonder additionally for testing if it would be worth creating a list
of public git repos of playbooks and you could pull and at least run
'--syntax-check' over them for new releases for any obvious
regressions that show up in lots of sites?

kevin

12

For the last 6 months I have been using Ansible heavily and I can honestly state that it is far better than competing tools. One place where I find Ansible lagging is exactly the topic of this post - not being able to take advantage of the enormous amount of contributors that are willing to help. Let me give you an example to make my point: search for a reusable way to configure Jenkins on Ubuntu, first with Puppet then with Ansible. In both products this not a part of the “official” modules/roles. So what is the difference:
For Puppet, you should be able to quickly find in their Puppet Forge a module that is “approved”, has 50+ times more downloads then other Jenkins modules and has a high Score. The module has multiple contributors and commits.
For Ansible, you can search the Ansible Galaxy and you find a bunch of roles with similar scores and when you dig a bit more similar functionalities. Each of them lacks functionalities that other have and each of them is created and maintained by just a few people.
I have a similar experience with other roles/modules that I needed.
I think that the way to improve that is by empowering the community to be able to better work on extensions (Ansible roles, modules, etc). Here are some concrete ideas:

  1. Improving Ansible Galaxy - show number of downloads together with the rating, better order roles by default.
  2. Encourage sharing of not only roles but modules and all other Ansible extension types though Ansible Galaxy. In this way there won’t be that many PRs for new modules but modules will be developed as extensions.
  3. When a module or other extension reaches a certain level of quality consider approving it for merging to the Ansible main repo(s).

My 2 cents.

13

@Brian Yes the combinatorial aspect of supported platforms makes it daunting for a developer to test all the variations manually. All the more reason to automate it as much as possible. Really the point I"m trying to get across here is that having a local system to test on that mirrors the systems used to verify would definitely increase the throughput of issues. No doubt there are technical challenges to such a framework but they are far from insurmountable. If it was integrated and automated there is no reason compute power for verification couldn’t be crowdsourced from the community. The maintainers would receive automated reports from well respected community members that have preverified potential changes reducing the work required from them to feel comfortable about merging.

It would be easy to mitigate that kind of bitcoin mining abuse; as Jesse Keating suggests putting an extra layer of human review in there before kicking off an automated set of tests to verify the latest cloud prs. I understand though that it does represent a cost to the company and I’m not trying to downplay that.

Most cloud module users submitting PRs would have first use their own accounts to implement them, the vast majority of compute time spent by the project would be in verification. Looking at the free tier in aws, there is an awful lot of testing/verification that could be squeezed into 750 hours of free compute time. I can’t really speak for the other cloud vendors, like Digital ocean, Rackspace et al, but clearly having well supported, complete modules for their platforms is in their interest as it drives business for them. Quid pro quo, it would not seem unreasonable for them to offer compute power for use in testing. Again, this is easier said than done, the folks that work there and help maintain the modules are better placed to comment, I’m just tossing it out as something worth investigating.

14

I have to agree with Ivaylo and so will toss in another 2 cents to make 4. A major contributor to the module bottleneck is the ‘batteries included’ approach to modules. Batteries included is fantastic for newcomers, a single pip install gets you off and running with the minimum of effort. When searching for support about a module they are not inundated with potential options but there is a single widely understood option and plenty of folks with experience with it to guide them. Its a core tenet of the project and a really worthy one, but it makes the core developers bottlenecks for updates.

The alternative of having modules in galaxy introduces the potential for fragmentation and confusion for newcomers who might be unsure which module to use among competing options, variations between them. But in favor of this approach the module maintainer will be a subject expert with the bandwidth to stay on top of requested features and pull requests. A module will get better quicker and without the bottleneck on core.

So why not a hybrid approach? Best of breed batteries included.

Key to what I’m about to suggest is a callback plugin for anonymous usage statistics. The plugin would capture

  • what modules are run
  • timing of how often and how long they take
  • how are they most usually configured
  • how often do they fail
  • Anonymized error reports to help with support.
  • etc

With would give valuable insight into where ansible is being most heavily used and help direct development effort to where it will most effective. This callback would be easy to implement, storing stats potentially here https://customers.influxdb.com/. This would be useful to the project even if the rest of what I suggest is not.

Next part of the hybrid approach would be a repo per module, with mechanisms to test the module and galaxy support to download and install a module. Stats would also be gathered for the repos.

  • what tests are there
  • what platforms has it been validated on
  • how many open PRs are there
  • meant time to resolution
  • how actively is it maintained.
  • ??

Combining this with usage statisics would be great health or fitness indicator and would make it easy to determine which modules are best of breed core candidates and which are ‘extras’. Either way it would delegate responsibility for modules to the community and free the core development team to focus on the work that needs doing with the core framework. The ‘best of breed’ modules and currently in core would still be included with a release, the build system would aggregate them when building a distribution. Other modules could be specified (via their galaxy id) in the config and downloaded on demand.

Fragmentation will be less of an issue as via the stats there will be clear indicators which modules are useful and which are not. Given the the correct indicators I think we’ll all be suprised at how quickly the community will quickly converge around the fittest and the pace of development that will result.

The same usage stats and fitness could be gathered and calculated for role repos to replace the star rating. Stars ratings are problematic for many reasons (this excellent examination explains why https://www.youtube.com/watch?v=Yn7e0J9m6rE). This would help focus role development and stop the proliferation of roles that we currently see.

An approach like this will alleviate one of the major bottlenecks in the project without sacrificing one of its best features.

Another 2c.

rgds,

Steve.

15

are technical challenges to such a framework but they are far from
insurmountable. If it was integrated and automated there is no reason
compute power for verification couldn't be crowdsourced from the community.
The maintainers would receive automated reports from well respected
community members that have preverified potential changes reducing the work
required from them to feel comfortable about merging.

I agree, this is doable technically, it is much harder to get to the
level of trust that this requires, it takes time (we already have this
with certain people but we need to expand to scale).

It would be easy to mitigate that kind of bitcoin mining abuse; as Jesse
Keating suggests putting an extra layer of human review in there before
kicking off an automated set of tests to verify the latest cloud prs. I
understand though that it does represent a cost to the company and I'm not
trying to downplay that.

This is how we currently work with our internal cloud, we manually
verify the PR and then run it through testing, this does not scale
very well but as you agreed, making it totally automated opens it up
for abuse.

business for them. Quid pro quo, it would not seem unreasonable for them to
offer compute power for use in testing. Again, this is easier said than
done, the folks that work there and help maintain the modules are better
placed to comment, I'm just tossing it out as something worth investigating.

We have been talking to vendors and established some very good
relationships, others are ongoing and probably require more of their
user base to use ansible for them to be more amenable (chicken/egg).

16

Hey, been thinking about this a little, here’s a couple of other ideas…

1/ give contributors a way to tag their PRs so they can indicate how important they think the PR is to b/ ansible project as a whole, b/ their particular use case. I think it might help a little with the ‘triage’ side of dealing with PRs, that is, working out when they should be dealt with. I just had a quick look and the largest group of PRs I could find were P3 feature pull requests.
I believe not everyone will say every PR is vital to ansible and their personal needs. In fact, My https://github.com/ansible/ansible/pull/9475 is super low priority, so there’s one less that needs dealing with any time soon :slight_smile:

That said, from memory, github doesn’t seem to have a facility for PR submitters to tag their PRs like this, so it would have to be something custom (which would take time) or a convention (which isn’t then enforced), so its not obvious how this could be done usefully.

2/ slowly, and with advance warning, start raising the requirements for a PR.

I’m just talking about requiring say, documentation or tests here, not requiring particular coding standards. Obviously its in everybody’s interests that whatever becomes a core part of ansible is as good as it can be. The more features that go into Ansible the more important it becomes that the new features don’t break existing ones.

But… maybe this would just be against the spirit of Ansible? Contribution seems to have been ‘on the front’ page of this project from the start. Plus, it might not always be viable or even valuable to insist on tests. (I’m thinking of things like the filters which are often very thin wrappers around python functions). Should they be unit or integration tests, or both? Some thought would need to go into changing the contribution guidelines so that they don’t result in confusion.

Another idea that I had, which I don’t really think would work but am going to share anyway is to give someone access to commit documentation only changes. Maybe this would take a bit of work off the core maintainers, but looking at the current PRs there aren’t that many that are tagged as documentation and I think there’s probably a surprising amount of work in there to keep things in sync with what’s actually in latest devel branch.

Well anyway, hope something from the above is of use.

Jon

17

Thanks to everyone for all the input. Great conversation.

And now, $0.02 from someone who hasn't touched any of this stuff. :slight_smile:

Hey, been thinking about this a little, here's a couple of other ideas...

1/ give contributors a way to tag their PRs so they can indicate how
important they think the PR is to b/ ansible project as a whole, b/ their
particular use case. I think it might help a little with the 'triage' side
of dealing with PRs, that is, working out when they should be dealt with. I
just had a quick look and the largest group of PRs I could find were P3
feature pull requests.
I believe not everyone will say every PR is vital to ansible and their
personal needs. In fact, My https://github.com/ansible/ansible/pull/9475 is
super low priority, so there's one less that needs dealing with any time
soon :slight_smile:

That said, from memory, github doesn't seem to have a facility for PR
submitters to tag their PRs like this, so it would have to be something
custom (which would take time) or a convention (which isn't then enforced),
so its not obvious how this could be done usefully.

Triage is so tricky to do well. I think we're better served generally
just to figure out how to get people to a place where they can commit
directly -- which brings us to the next point.

2/ slowly, and with advance warning, start raising the requirements for a
PR.

I'm just talking about requiring say, documentation or tests here, not
requiring particular coding standards. Obviously its in everybody's
interests that whatever becomes a core part of ansible is as good as it can
be. The more features that go into Ansible the more important it becomes
that the new features don't break existing ones.

So there's an important difference between "raising the requirements
of a PR" and "raising the requirements of a PR that is likely to get
accepted." It's very difficult to do the first, but pretty much
essential to do the second.

I think this is one of the key reasons why the focus on testability in
2.0 is so important. If we don't have a strong test framework, it's
easy for potential contributors to say "there's a hundred ways I could
test this, I don't know which one to pick, I'll just leave it."
Whereas if there's a standard for testability, and part of the PR is
"here's evidence that my PR isn't a breaking change," then it becomes
more defensible to just merge changes.

Of course, that's still in the future. But it's a closer future now, I hope.

But.. maybe this would just be against the spirit of Ansible? Contribution
seems to have been 'on the front' page of this project from the start.
Plus, it might not always be viable or even valuable to insist on tests.
(I'm thinking of things like the filters which are often very thin wrappers
around python functions). Should they be unit or integration tests, or
both? Some thought would need to go into changing the contribution
guidelines so that they don't result in confusion.

Ayup. Lots of work here.

But this is also one of those places where we might see some payoff in
the module split between ansible-core and ansible-extras. The point of
that is to be able to lower the barrier, and to give us a "contrib"
space to prove out some modules that we don't necessarily want to ship
to all Ansible users... yet. That obviously doesn't help us with the
current batch of core modules -- but it could be argued (and has been)
that we have too many core modules anyway. Maybe a rigorous
examination of which modules are truly "core" is in order, and modules
that don't make the cut can be kicked into extras (and thus
potentially to the maintainers who are most interested in them.)

Another idea that I had, which I don't really think would work but am going
to share anyway is to give someone access to commit documentation only
changes. Maybe this would take a bit of work off the core maintainers, but
looking at the current PRs there aren't that many that are tagged as
documentation and I think there's probably a surprising amount of work in
there to keep things in sync with what's actually in latest devel branch.

Hmmmmm. Interesting. Doubly interesting because there's also an
i18n/l10n issue here, and splitting out docs also makes it possible to
split out translation of docs. Need to figure out how to make that
happen.

Well anyway, hope something from the above is of use.

Got me thinking, anyway. :slight_smile:

--g

18

The problem with this is that it compounds the current problem that it
seems like pull requests for new modules sit in limbo almost
indefinitely - the new modules/extra repo was made and contributors had
to create new PRs, with the carrot of more rapid acceptance being
offered, but some PRs (including mine) are still languishing since
September. As a potential contributor, this is quite demoralising, and
the idea of continually having to jump through more hoops while my PR
remains in a limbo of "accepted in theory but not merged" is pretty
off-putting.

Regards,

Matthew

19

I don’t think it would be in anyone’s interests to apply such a change to existing pull requests, I was thinking you could only do this by making it known that after a certain date in the future the rules were going to change.

I wonder how many current PRs are adding or updating a module? If there are lots how about an
‘ansible-modules-contributed’ sub project where stuff gets merged in with the minimum of ceremony (perhaps automatically). If you are interested enough you can pick through the contributions and add them to your /etc/ansible/library or role/library and make use of them, but they wouldn’t form part of your module path by default. Perhaps a folder per contributor to allow multiple modules of the same name to exist. Its kind of chaos but ‘optional chaos’.

Sounds somewhat similar to galaxy though, and I imagine there’s a lot of PRs that just don’t fall into this category.

20

A lot of the suggestions here could potentially be very beneficial but I think a lot of them seem to be overkill for the many, many PRs that are currently sitting in the queue and are perhaps only a one or two line change.

I agree with with the first part of Matthew’s statement:

As a potential contributor, this is quite demoralising, and
the idea of continually having to jump through more hoops while my PR
remains in a limbo of “accepted in theory but not merged” is pretty
off-putting.

But not the second. It is indeed quite demoralising to see you submit a PR bugfix or new feature and then see it sit there gathering dust over many months. However, I am quite happy to jump through some more hoops if it meant I could speed up the time for this PR to be accepted.

So here I guess we need some guidance from the core team… @bcoca ; … what is it that stops a small, simple PR from being merged? What things could a PR contributor do to relieve your burden and make it simple for you to click that merge button!?

Would automated testing help? I assume not, as some modules already have tests written for them and the PR is so simple i guarantee it wouldn’t change the result of the test, yet the PR still sits there for months - or is this because as discussed previously, the infrastructure required to run the tests (e.g. for a cloud module) doesn’t exist?

Is the PR just not a good one? I know that the contributions page says to discuss any PR first before submitting but i don’t see that happening very often. If there was a clear discussion here that the PR could link to, would the merge team have more faith in merging it?

What are the other roadblocks?