sgnl05 / sgnl05-sssd

Puppet module for SSSD
https://forge.puppet.com/sgnl05/sssd
GNU General Public License v3.0
17 stars 77 forks source link

Stop sorting config keys and remove extraneous whitespace. #12

Closed gizmoguy closed 8 years ago

gizmoguy commented 8 years ago

Here you go :)

edestecd commented 8 years ago

We need that sort on boxes that use ruby 1.8.7. Hashes are not ordered prior to ruby 1.9.3, so consecutive puppet runs keep changing the file over and over and you never get to a state of no change.

We would need to drop support for puppet 3.x to accept this, which I am not ready to do at this point.

sgnl05 commented 8 years ago

Thanks for your PR gizmoguy! Too bad it's not as simple as I was hoping.

@edestecd maybe there's a way to check what ruby-version's running within the template. If running 1.9.3 or newer we can do without sorting, and if it's less we sort. I'll look into it.

edestecd commented 8 years ago

There is a rubyversion fact, I believe.

sgnl05 commented 8 years ago

Yes you're right.

We could use two templates, one with sort and one without. Then compare versions with "versioncmp($::rubyversion, '1.9.3') >= 0" to select what template to use. Any thoughts about this?

edestecd commented 8 years ago

I'd double check that the rubyversion fact works in puppet 4, with the new packaging of ruby in /opt... They may have two facts now. One for system ruby and one for puppet ruby. I seem to remember something like that.

sgnl05 commented 8 years ago

I've checked this by installing centos 7 and latest puppet from Puppetlabs repo.

[root@localhost ~]# /opt/puppetlabs/puppet/bin/puppet --version
4.3.2
[root@localhost ~]# /opt/puppetlabs/puppet/bin/facter --version
3.1.4 (commit c08a5ed7606b6cd8be3a146b247c03bf7213c445)
[root@localhost ~]# /opt/puppetlabs/puppet/bin/ruby --version
ruby 2.1.8p440 (2015-12-16 revision 53160) [x86_64-linux]
[root@localhost ~]# ruby --version
ruby 2.0.0p598 (2014-11-13) [x86_64-linux]
[root@localhost ~]# /opt/puppetlabs/puppet/bin/facter rubyversion
2.1.8

So 'rubyversion' fact returns the version used by Puppet. As far as I can tell, we're good to go? :)

edestecd commented 8 years ago

Looks good to me.

sgnl05 commented 8 years ago

@gizmoguy would you like to update your PR with the proposed solution? Just make sure the tests pass :)