fixes #16844 - remove discovery image installation

[mmoll]

This should (and is) handled in puppet-foreman_proxy. If needed, there could be even a migration to transfer the values to foreman_proxy parameters, when it was enabled for foreman.

[domcleal]

Isn't this here too because the plugin supports either installation with or without a smart proxy plugin?

[mmoll]

@lzap could you have a look here?
@domcleal on the one hand yes, but on the other, are there any usecases where one has that image on the foreman server?

[lzap]

I don't understand the report - why puppet fails with the duplicate declaration now? Is this some new kind of check in newer Puppet version?

Anyway, discovery works just fine without any smart-proxy, it is not required at all. So this is the reason why it's here twice. Shall we rename the resource name or something to prevent the error?

[domcleal]

It probably fails if install_images is enabled in both the foreman::plugin::discovery and foreman_proxy::plugin::discovery classes, which is correct. Renaming the resources is somewhat pointless if they're managing the same files.

@domcleal on the one hand yes, but on the other, are there any usecases where one has that image on the foreman server?

It might be that the Foreman and proxy are on the same host, so you'd only enable the foreman::plugin::discovery class, in which case you'd use this code.

It may just need a better error message.

[mmoll]

Maybe it would be better to have the discovery image installation centralized and seperated from the core and proxy plugins somewhere?

[mmoll]

related: http://projects.theforeman.org/issues/9543

[lzap]

Feel free to move it, I have no opinion on that. I'd rather stay away from Puppet.

[ekohl]

@mmoll what's the plan for this?

[mmoll]

There was no real consensus reached, the current situation is not optimal, but the error described in the issue only occurs when setting things manually.

[tuxmea]

theforeman/puppet-foreman_proxy#367 is not directly related to this issue.
This issue is due to duplicate resource declaration.

We can use the same pattern: switch from foreman::remote_file { <title>: ...} to ensure_resource('foreman::remote_file', ...)
On the other hand: why not make use of voxpupuli-archive?

[mmoll]

The relation is that discovery (image) related things are in puppet-foreman and puppet-foreman_proxy

[tuxmea]

Yes. we have two options:

1. use ensure_resource also on the define foreman::remote_file in both discovery classes
2. move the discovery.pp to a separate place and include the class in puppet-foreman and puppet-foreman_proxy

Which option do you prefer?
Personally I would prefer option 2 as this removes duplicate code.

[mmoll]

@lzap http://projects.theforeman.org/issues/9543 - what did you have exactly in mind there?

[lzap]

See #253 (comment)

[ekohl]

Personally I still think the discovery plugin should only handle images on a proxy host and not the foreman (only) host. That's this PR and I think we should revisit this.