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

spec_helper_acceptance: add abbility to enable Ipv6 in beaker hosts #16

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

Conversation

b4ldr
Copy link
Member

@b4ldr b4ldr commented Dec 22, 2020

Enabled by default
Also see: voxpupuli/modulesync_config#694

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Does this work in Docker?

@b4ldr
Copy link
Member Author

b4ldr commented Dec 22, 2020

Yes its been in use in the unbound module for some time and works as expected. I have also used it in other projects but that was some time ago

@@ -61,6 +61,7 @@ def configure_beaker(modules: :metadata, configure_facts_from_env: true, &block)
end

write_beaker_facts_on(hosts) if configure_facts_from_env
on(hosts, 'echo 0 > /proc/sys/net/ipv6/conf/all/disable_ipv6') if enable_ipv6
Copy link
Member Author

Choose a reason for hiding this comment

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

i originally used sysctl net.ipv6.conf.all.disable_ipv6=0 but sysctl isn't installed on fedora by default.

@b4ldr
Copy link
Member Author

b4ldr commented Dec 22, 2020

Does this work in Docker?

Tested and working however when i tested locally (with a plain docker image) i noticed that /proc/sys is mounted readonly. As such im gussing beaker takes care of making sure this is mounted with rw?

@b4ldr
Copy link
Member Author

b4ldr commented Dec 22, 2020

i noticed that /proc/sys is mounted readonly. As such im gussing beaker takes care of making sure this is mounted with rw?

confirmed beaker starts the container with --privileged which dosn't have this issue

@ekohl
Copy link
Member

ekohl commented Dec 22, 2020

I think that does mean you modify sysctl from the host, which can be nasty as well. I'll submit a PR to unbound to use some ULA subnet instead of a global one to see if that passes.

@b4ldr
Copy link
Member Author

b4ldr commented Dec 22, 2020

I think that does mean you modify sysctl from the host, which can be nasty as well.

that's not correct for this parameter and most kernel parameters they have there own cgroup/namespace see below:

~/git/puppet $ cat /proc/sys/net/ipv6/conf/all/disable_ipv6                            
0
~/git/puppet $ docker run --privileged --rm -it fedora:32 /bin/bash                       
[root@98a76107e9d8 /]# cat /proc/sys/net/ipv6/conf/all/disable_ipv6 
1

I'll submit a PR to unbound to use some ULA subnet instead of a global one to see if that passes.

hmm setting the following "fixed-cidr-v6": "fd00::/8" in docker.yaml may work as well however if one really just needs the ::1 loopback which i suspect is the case in most situations then i think i rather lean towards this solution

*fyi which ever fix needs to be made in modsync_config not puppet-unbound

@ekohl
Copy link
Member

ekohl commented Dec 22, 2020

I thought a bit more about it. One thing I like about doing it via a sysctl is that you are less dependent on the environment so you can reproduce it locally. Later I'll see about merging your PRs but now it's time to call it a day.

@ekohl
Copy link
Member

ekohl commented May 21, 2021

I forgot about this, but I've since taken a difference approach and use RSpec.add_setting instead of adding parameters to configure_beaker to tune behavior. I also pushed the setup_acceptance_node.pp pattern into the module which makes things easier to use. That means it now needs a rebase.

I'd also be ok with not solving it in this library but rather keep it up to the modules to fix it.

@b4ldr
Copy link
Member Author

b4ldr commented Jun 4, 2021

Sorry i missed this, have rebased now

I'd also be ok with not solving it in this library but rather keep it up to the modules to fix it.

On the one hand this seems to only affect a small subset of modules so perhaps not worth it. On the other hand its a very minor addition and could save someone from scratching there head when implementing acceptance test. as such i would favour adding it

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.

2 participants