Fix multi-node list with parallel mode

[isaiahfrantz]

Overview

This pull request fixes erroneously waited pids inside the OctocatalogDiff::Util::Parallel.run_tasks_parallel method.

I was testing parallel runs with a comma separated list of hosts in the -n option. It consistently failed if I ran without the --no-parallel flag with the following error:

breaks with multiple hosts with --parallel

octocatalog-diff -d -n prdthing0028.unix.org,prdthing0026.unix.org --parallel
...
D, [2019-02-18T17:50:20.427185 #17245] DEBUG -- : File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] @ environments/production/modules/puppet_conf/manifests/init.pp:8
/opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:166:in `system': No child processes - Another thread waited the process started by system(). (Errno::ECHILD)
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:166:in `block in diff_bin'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:166:in `each'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:166:in `find'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:166:in `diff_bin'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:57:in `diff'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:89:in `each'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/format.rb:23:in `to_a'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/format.rb:23:in `text'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/diffy-3.3.0/lib/diffy/diff.rb:146:in `to_s'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display/text.rb:348:in `diff_two_hashes_with_diffy'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display/text.rb:176:in `display_added_item'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display/text.rb:77:in `block in generate'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display/text.rb:58:in `each'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display/text.rb:58:in `generate'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/catalog-diff/display.rb:56:in `output'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/cli/printer.rb:30:in `printer'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/cli.rb:156:in `run_octocatalog_diff'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/octocatalog-diff-1.5.4/lib/octocatalog-diff/cli.rb:127:in `block in cli'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/parallel-1.13.0/lib/parallel.rb:487:in `call_with_index'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/parallel-1.13.0/lib/parallel.rb:345:in `block (2 levels) in work_in_threads'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/parallel-1.13.0/lib/parallel.rb:498:in `with_instrumentation'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/parallel-1.13.0/lib/parallel.rb:344:in `block in work_in_threads'
        from /opt/puppetlabs/puppet/lib/ruby/gems/2.4.0/gems/parallel-1.13.0/lib/parallel.rb:206:in `block (2 levels) in in_threads'

diff production/prdthing0026.unix.org pipeline_merge/prdthing0026.unix.org
*******************************************
+ File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] =>
   parameters =>
     "backup": "main",
     "content": "previous_puppet_mode=production\n",
     "ensure": "file",
     "group": "root",
     "owner": "root"
*******************************************

works with --no-parallel

octocatalog-diff -d -n prdthing0026.unix.org,prdthing0028.unix.org --no-parallel
...
D, [2019-02-18T17:52:55.867095 #17716] DEBUG -- : File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] @ environments/production/modules/puppet_conf/manifests/init.pp:8
diff production/prdthing0028.unix.org pipeline_merge/prdthing0028.unix.org
*******************************************
+ File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] =>
   parameters =>
     "backup": "main",
     "content": "previous_puppet_mode=production\n",
     "ensure": "file",
     "group": "root",
     "owner": "root"
*******************************************
diff production/prdthing0026.unix.org pipeline_merge/prdthing0026.unix.org
*******************************************
+ File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] =>
   parameters =>
     "backup": "main",
     "content": "previous_puppet_mode=production\n",
     "ensure": "file",
     "group": "root",
     "owner": "root"
*******************************************

with fix (explicitly with --parallel)

octocatalog-diff -d -n prdthing0026.unix.org,prdthing0028.unix.org --parallel
...
D, [2019-02-18T18:12:46.195536 #18748] DEBUG -- : File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] @ environments/production/modules/puppet_conf/manifests/init.pp:8
diff production/prdthing0028.unix.org pipeline_merge/prdthing0028.unix.org
*******************************************
+ File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] =>
   parameters =>
     "backup": "main",
     "content": "previous_puppet_mode=production\n",
     "ensure": "file",
     "group": "root",
     "owner": "root"
*******************************************
diff production/prdthing0026.unix.org pipeline_merge/prdthing0026.unix.org
*******************************************
+ File[/opt/puppetlabs/facter/facts.d/previous_puppet_mode.txt] =>
   parameters =>
     "backup": "main",
     "content": "previous_puppet_mode=production\n",
     "ensure": "file",
     "group": "root",
     "owner": "root"
*******************************************

I tracked it down to the fact that the usage of Process.wait(0) will reap any completed child in the context of the main ppid. Diffy apparently forks its differs and expects the child pids to be available to wait.

I thought that I had a good test in the parallel_rspec.rb file but I cant get it to work. The code however fixes all of my issues. If anybody is interested in taking a look at the tests and suggesting a better way, I would be grateful.

Checklist

• Make sure that all of the tests pass, and fix any that don't. Just run rake in your checkout directory, or review the CI job triggered whenever you push to a pull request.
• Make sure that there is 100% test coverage by running rake coverage:spec or ignoring untestable sections of code with # :nocov comments. If you need help getting to 100% coverage please ask; however, don't just submit code with no tests.
• If you have added a new command line option, we would greatly appreciate a corresponding integration test that exercises it from start to finish. This is optional but recommended.
• If you have added any new gem dependencies, make sure those gems are licensed under the MIT or Apache 2.0 license. We cannot add any dependencies on gems licensed under GPL.
• If you have added any new gem dependencies, make sure you've checked in a copy of the .gem file into the vendor/cache directory.

/cc [related issues] [teams and individuals, making sure to mention why you're CC-ing them]

[isaiahfrantz]

merged in master branch to keep diffs limited to my code

[ahayworth]

Intriguing. We're not using this in --parallel mode internally (at least, not that I can find), so we haven't experienced this personally.. However, I think your fix looks good and I'll merge it in for 1-6-0 later this week!