`augeas` should not be the default provider for `sshkey`

[alexjfisher]

I started using this module for the ssh_config type, but meanwhile it became the default provider for sshkey (a type from the sshkeys_core module). This has had a hideous impact on performance.

In my testing, 2000 resources take 330 seconds to enforce (even with no changes), compared to 0.91 seconds if I force the provider back to parsed.

It's not uncommon to manage a large number of sshkey resources, so I think using this module to manage sshkey resources should be opt-in, (or the sshkey provider moved into its own module).

[alexjfisher]

When I look at just the sshkey execution time as displayed in reports sent to Foreman, I have seen a 2000x speed difference on a host with about 2.5k sshkey resources.

[binford2k]

My opinion is that 300-2000x performance decrease should not ever be the default. If people want additional features, they should have to opt in to it.

@joshcooper even if we remove the defaultfor, it will still be selected because it's got a confine and is more suitable, correct? Do you know why parsed isn't the default?

[alexjfisher]

According to https://github.com/voxpupuli/puppet-augeasproviders_ssh/issues/55 this provider is currently broken; which is odd as it didn't explode for me, (but it wasn't making any changes so maybe I missed the broken code path.)

Also quite scary how it monkey patches the original type... https://github.com/voxpupuli/puppet-augeasproviders_ssh/blob/3a16b2a3f001f2aff079d86ace569682ace177cc/lib/puppet/provider/sshkey/augeas.rb#L18

Maybe this made slightly more sense at the time, when the type was in puppet and not split into its own module.

[joshcooper]

it will still be selected because it's got a confine and is more suitable, correct?

If there are no default providers,, then puppet magically picks the provider with the "most specificity" https://github.com/puppetlabs/puppet/blob/a423af825d1e87a3158cd68d630e77352e9a5454/lib/puppet/provider.rb#L336-L345

The whole specificity thing is fragile and dumb. Refactoring a provider to use a new base provider can cause the specificity to change, thereby causing a different provider to be chosen.

Do you know why parsed isn't the default?

Probably because it used to be in core and I'm guessing predated this provider? TBH I don't think either provider should be the defaultfor because they don't support composite namevars in the same way. Instead require users to specify which provider they want to use.

[joshcooper]

300-2000x performance decrease

This is also strange. I would expect augeas to be much faster than parsed file, unless augeas is accidentally loading the entire /etc/ directory. Maybe the provider is not setting the incl filter correctly? https://augeas.net/docs/builtins.html

[alexjfisher]

300-2000x performance decrease

This is also strange. I would expect augeas to be much faster than parsed file, unless augeas is accidentally loading the entire /etc/ directory. Maybe the provider is not setting the incl filter correctly? https://augeas.net/docs/builtins.html

https://github.com/voxpupuli/puppet-augeasproviders_ssh/blob/5d20ceed9e96c51eeed9c2b9671bb57749f291dc/lib/puppet/provider/sshkey/augeas.rb#L82-L97 is crazy expensive (even if you're not using hashed hostnames) with a large number of resources. Looks like it gets called 3 times per sshkey catalog resource.

With 1000 sshkey resources that's ~ 1.5 MILLION calls to aug.get as it iterates over non-comment entries until it finds the resource.