Allow --hostname to receive a comma separated list of hosts and work on them in parallel

[EdgeJ]

Overview

This pull request changes the hostname option to accept a string of multiple, comma separated hosts to diff and runs the diffs in parallel.

This changeset allows for octocatalog-diff to run against multiple hosts without having to resort to using some sort of loop (e.g. bash for loop) to iterate against the hosts. My team currently uses octocatalog-diff in a CI setup to verify changes against multiple hosts before pushing up changes to our production branch, and it is currently both slow and somewhat inelegant to implement.
These changes allow users to set up a CI system to run against many hosts to get a better picture of how changes might affect the overall environment by adding multiple hosts to the -n or --hostname option in a comma separated list, e.g.

octocatalog-diff -n example1.github.com,example2.github.com,webserver3.github.com

This also uses the parallel gem to do the diffing in parallel to greatly speed up the operation from either using shell loops or simply using ruby iteration (i.e. nodes.each do). By my calculations (using the Unix time utility and running against two hosts), this results in a speedup of 2x:

[with parallel]
octocatalog-diff -n host1,host2  41.62s user 21.10s system 77% cpu 1:21.36 total

[iterating normally]
octocatalog-diff -n host1,host2  39.89s user 19.17s system 40% cpu 2:26.36 total

[shell iteration on single hosts]
( for host in host1 host2; do;  -n )  41.14s user 20.10s system 41% cpu 2:29.29 total

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]

[EdgeJ]

Note, spec tests on my development machine were not passing 100%, but neither were they 100% on the master branch, either. My testing showed about the same level of pass/fail on both branches, so I assumed my branch passing at the same rate as the master branch meant that I could check the passing tests box.

[EdgeJ]

Looks like my changes broke testing in Travis-CI, though. So, I'll have to go back and look through those to see what's busted.

[EdgeJ]

Now all tests pass, except that Travis is having some issue with Bundler in the Ruby 2.4 Rubocop tests:

-------------------------------
Running rubocop tests
-------------------------------
bootstrap.0
Starting script/bootstrap
Running bundler
Resolving dependencies...
Installing rake 11.2.2
Installing ast 2.3.0
Using bundler 1.16.3
Installing diff-lcs 1.3
Installing diffy 3.2.0
Installing docile 1.1.5
Installing facter 2.5.1
Installing fast_gettext 1.1.2
Installing hashdiff 0.3.7
Installing hiera 3.4.2
Installing multi_xml 0.6.0
Installing httparty 0.15.6
Installing json 2.1.0 with native extensions
Installing locale 2.1.2
Installing parallel 1.12.0
Installing rugged 0.26.0 with native extensions
Using octocatalog-diff 1.5.3 from source at `.`
Installing parallel_tests 2.7.1
Installing parser 2.4.0.0
Installing powerpack 0.1.1
Installing puppet 5.4.0
Installing rainbow 2.2.2 with native extensions
Installing rspec-support 3.4.1
Installing rspec-core 3.4.4
Installing rspec-expectations 3.4.0
Installing rspec-mocks 3.4.1
Installing rspec 3.4.0
Installing rspec-retry 0.5.0
Installing ruby-progressbar 1.9.0
Installing unicode-display_width 1.3.0
Installing rubocop 0.48.1
Installing simplecov-html 0.10.2
Installing simplecov 0.14.1
Installing simplecov-erb 0.1.1
Updating files in vendor/cache
Bundle complete! 9 Gemfile dependencies, 34 gems now installed.
Bundled gems are installed into `./vendor/bundle`
Post-install message from httparty:
When you HTTParty, you must party hard!
Completed script/bootstrap successfully
real	0m58.243s
user	0m49.388s
sys	0m7.273s
/home/travis/build/github/octocatalog-diff/vendor/bundle/ruby/2.4.0/gems/rake-11.2.2/lib/rake/ext/fixnum.rb:4: warning: constant ::Fixnum is deprecated
/home/travis/.rvm/gems/ruby-2.4.1/bin/bundle:23:in `load': cannot load such file -- /home/travis/.rvm/rubies/ruby-2.4.1/lib/ruby/gems/2.4.0/gems/bundler-1.16.3/exe/bundle (LoadError)
	from /home/travis/.rvm/gems/ruby-2.4.1/bin/bundle:23:in `<main>'
	from /home/travis/.rvm/gems/ruby-2.4.1/bin/ruby_executable_hooks:15:in `eval'
	from /home/travis/.rvm/gems/ruby-2.4.1/bin/ruby_executable_hooks:15:in `<main>'
real	0m0.691s
user	0m0.589s
sys	0m0.098s

[kpaulisse]

Thanks for submitting this. I'll see if I can get Travis passing, perhaps by updating rake or something.

[kpaulisse]

Hey @EdgeJ I think I got this figured out - it looks like the error you ran in to was widespread and well known across the platform a while back. The thing that ultimately got the build passing in my branch was to remove this line from .travis.yml:

before_install: gem install bundler

Want to give that a try in your branch to see if that gets you ✅ ?

[EdgeJ]

@kpaulisse that did the trick!

[kpaulisse]

Cool, I'll get to reviewing the code soon then. Glad to hear it worked.

[BarnumD]

I would suggest you change -n, --hostname HOSTNAME in doc/optionsref.md to --hostname AND --hostnames or something to that effect.

[BarnumD]

@EdgeJ I'm attempting to try your changes. I've built the ruby gem from your git repository
gem 'octocatalog-diff', :git => 'https://github.com/EdgeJ/octocatalog-diff.git', :branch => 'add_multiple_node_option'
Using octocatalog-diff 1.5.3 from https://github.com/EdgeJ/octocatalog-diff.git (at add_multiple_node_option@ffe219b)

When I run oc-d from the command line with a single node it works as expected
octocatalog-diff --catalog-only --bootstrap-current --no-header --debug -o /dev/null -n server1.domain.org

However, when I add a second comma separated node, I receive
octocatalog-diff --catalog-only --bootstrap-current --no-header --debug -o /dev/null -n server1.domain.org,server2.domain.org
...
... computed.rb:33:in 'initialize': Node name must be passed to OctocatalogDiff::Catalog::Computed (ArgumentError)

Any ideas?

[EdgeJ]

@BarnumD I see what the issue is. Before the Parallel block, the :node key is deleted from the options hash and iterated on in that block. However, only the diffing is wrapped up in the Parallel block. The catalog_only and bootstrap_then_exit functions are called before the Parallel block. So, when a single node is set in the command line, it's correctly passed to those methods as a String.
When multiple nodes are passed, the methods don't know what to do with the Array. I'm not certain whether it would be better to document that the multiple node option only works with diffing or whether I should wrap that whole section of code in the Parallel block. Either way, you're right that I need to update the documentation. Hadn't done that before, as I wanted to get some feedback about the changes and ensure no major changes to my code were needed before I did the documentation.

[kpaulisse]

Thank you for submitting this PR. I apologize that it took so long to be reviewed. This will be included in octocatalog-diff release 1.5.4 later this week. (I made a follow-up commit on that branch to make it so that parallel doesn't get used when only a single hostname is supplied, but of course when there are 2 or more hosts specified, the parallel logic you wrote will be invoked.)