fwv

[jhoblitt]

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

[jhoblitt]

Questions:

• Are the parameter names reasonable? hosts::hosts from hiera looks a bit awkward. Would hosts::host (without the s) be any better? Maybe hosts::entry or hosts::entries would be better?
• Is the default set of /etc/hosts entries for loopback/etc. reasonable?
• Should we use the core hosts type? It does not correctly handle duplicate ip address entries.

[jcpunk]

I've got a number of dual stack hosts, how would I use this module for them?

[jhoblitt]

@jcpunk are you creating /etc/hosts entries for networking.ip6 ?

[jhoblitt]

How about hosts::host_entries ?

[jhoblitt]

I'm not really sure how to build dual stack support while using puppetlabs/host_core, as that module's parser doesn't seem to fully handle the nuances of mixed ipv4 and ipv6 addresses.

I think for the moment I'm going to going to leave the default behavior as ipv4 only, so something can be shipped, as I believe the only way forward for dual-stack to replace puppetlabs/host_core and that's more effort than I can I can afford to invest at the moment.

[bastelfreak]

please don't add the core module. They are supposed to be always available so we never add them to metadata.json.

[jhoblitt]

I deeply disagree with this. For starters, it isn't always available. It is only available when using the AIO packaging and AFAIK, there is no official vox policy of only support AIO. puppetlabs/host_core is a dependency and it has a version string. If we don't put it in the metadata.json then we have to do extra work to setup the acceptance tests, which is just shooting ourselves by causing extra work for no benefit.

[bastelfreak]

Even non AIO packages should include those core modules or pull them in as a dependency. Debian for example does that: https://packages.debian.org/bookworm/puppet-agent. Same on ubuntu: https://packages.ubuntu.com/lunar/puppet-agent. Arch vendors them into the puppet package: https://gitlab.archlinux.org/archlinux/packaging/packages/puppet/-/blob/main/PKGBUILD?ref_type=heads#L38-46

because of that, we don't need to explicitly install them in our acceptance tests. If a distro builds their own package but doens't pull in the core modules, the package is broken (that's a statement from Puppet, but I agree with this).

[jhoblitt]

These are modules that have versions. They are released on the forge so they can be updated independently from the core.

[jhoblitt]

Let me turn this around, what is broken by this module's metadata declaring its full dependency chain?

[jhoblitt]

Also, it needs to be documented that this module has a dep on host_core as when someone copies into into the Puppetfile for the control repo, host_core also needs to be declared in the Puppetfile for rspec-puppet to work.

[jhoblitt]

I'm going to ship this as-is as I'm in a rush. We can continue this discussion if needed.