Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all facts have networking.ip6 fact #255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bastelfreak
Copy link
Member

Currently not all factsets have networking.ip6. I'm wondering if we should enforce this.

@bastelfreak bastelfreak self-assigned this Jul 19, 2022
@ekohl
Copy link
Member

ekohl commented Jul 20, 2022

I'm wondering if we should enforce this.

I can see both advantages and disadvantages. On the one hand, not having an IPv6 address is completely possible and valid. On the other hand, you sometimes do want to test with it.

In cases where it matters I've always explicitly overridden it and not relied on the basic facts.

@bastelfreak
Copy link
Member Author

yes, that's why I'm unsure as well. I did say that usual linux boxes at least have a link local address so it would be nice to rely on that during testing as well. I run into this because some of our factsets have ipv6 addresses and some don't . also overwriting the networking hash is a bit ugly I think.

@ekohl
Copy link
Member

ekohl commented Jul 20, 2022

AFAIK link local never shows up as networking.ip6. I at least hope not because it's usually useless and unusable.

Copy link
Contributor

@baurmatt baurmatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me but I think you have to touch a couple more factsets ;)

@smortex
Copy link
Member

smortex commented Aug 2, 2022

AFAIK link local never shows up as networking.ip6. I at least hope not because it's usually useless and unusable.

That makes no sense but they do 🤷 :

romain@desktop-fln40kq ~/Projects/puppet-bacula % facter networking.ip6
fe80::aa5e:45ff:feb7:3a51
romain@desktop-fln40kq ~/Projects/puppet-bacula % facter --version
4.2.10

If it makes more sense, maybe we can expect an IPv6 reserved for documentation (i.e. in the 2001:DB8::/32 range, see RFC 3849) in the test suite and require the factsets to be adequately tuned (by hand) when submitting a new OS?

@ekohl
Copy link
Member

ekohl commented Aug 3, 2022

In Foreman we do filter out reported link local addresses. I recall at some point they even reported the interface in the link local address!

Also note that there's rodjek/rspec-puppet#772. So at least there is some legacy fact set.

It should also be noted that in Linux when you remove the ipv6 kernel module you don't have a link local address at all, but I think we can consider that edge case unsupported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants