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

fix network bridge modules and sysctl #103

Closed
wants to merge 2 commits into from

Conversation

rdxmb
Copy link
Contributor

@rdxmb rdxmb commented Apr 21, 2021

fix #92

@rdxmb
Copy link
Contributor Author

rdxmb commented Apr 21, 2021

if this looks good to you I can create another PR with sqashed commits.

@rdxmb
Copy link
Contributor Author

rdxmb commented Apr 22, 2021

This could also be part of the containerd-role, like it is described here: https://kubernetes.io/docs/setup/production-environment/container-runtimes/#containerd .

What do you think?

@gleichda
Copy link

@rdxmb @geerlingguy what about this PR tested it on Ubuntu 21.04 Server on a raspberry Pi and can confirm it works

@@ -8,14 +8,40 @@
or ansible_distribution_major_version | int < 10

# See: https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#letting-iptables-see-bridged-traffic
- name: Let iptables see bridged traffic.
sysctl:
Copy link
Owner

Choose a reason for hiding this comment

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

Is there some reason the sysctl module can't be used for these modifications (and lineinfile used instead)?

Copy link
Contributor Author

@rdxmb rdxmb Jun 24, 2021

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

This is immediately applied, but will only last 'till the next boot, so it is not a permanent change.
to make sysctl changes permanently write token = value formatted file into below dirs.

FILES
       /run/sysctl.d/*.conf
       /etc/sysctl.d/*.conf
       /usr/local/lib/sysctl.d/*.conf
       /usr/lib/sysctl.d/*.conf
       /lib/sysctl.d/*.conf
       /etc/sysctl.conf
              The paths where sysctl preload files usually exist.  See also sysctl option --system.

@terryzwt
Copy link

I apply the patch mannually, works for me.

@stale
Copy link

stale bot commented Nov 20, 2021

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Nov 20, 2021
@rdxmb
Copy link
Contributor Author

rdxmb commented Nov 22, 2021

/no-stale-please

@stale
Copy link

stale bot commented Nov 22, 2021

This issue is no longer marked for closure.

@stale stale bot removed the stale label Nov 22, 2021
@gleichda
Copy link

gleichda commented Dec 21, 2021

See the original issue or https://stackoverflow.com/a/63692277 where @geerlingguy answered the question already on stackoverflow. Validated today with Ubuntu LTS 20.04 on a RPI 4

Edit the net.ipv4.ip_forward = 1 is still required. So my pre tasks currently look:

- name: Ensure br_netfilter is enabled.
  modprobe:
    name: br_netfilter
    state: present
  become: true

- name: Let iptables see bridged traffic.
  sysctl:
    name: net.ipv4.ip_forward
    value: '1'
    state: present
  become: true

I would suggest to include these tasks into the module. Whats your opinion on that @geerlingguy @rdxmb

@rdxmb
Copy link
Contributor Author

rdxmb commented Dec 21, 2021

If this works, this sounds good.
However, I prefer using a config-file under /etc/sysctl.d , which can also be done via ansible's sysctl.

https://docs.ansible.com/ansible/latest/collections/ansible/posix/sysctl_module.html#parameter-sysctl_file

This way a later change / cleanup can be done through ansible by deleting this file.

@gleichda
Copy link

Sounds valid no objections there. Will you rewrite your PR or should I create a new one!

@rdxmb
Copy link
Contributor Author

rdxmb commented Dec 28, 2021

feel free to create a new one. Rewrite is bad because of missing squash. Thanks.

@stale
Copy link

stale bot commented Apr 7, 2022

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Apr 7, 2022
@rdxmb
Copy link
Contributor Author

rdxmb commented Apr 7, 2022

no-stale

@gleichda will you continue with your pr?

@stale
Copy link

stale bot commented Apr 7, 2022

This issue is no longer marked for closure.

@gleichda
Copy link

@rdxmb unfortunately not at the moment due to missing time. But as soon as I find some spare time I will.
In the meantime if you want to and have time pls go ahead

@stale
Copy link

stale bot commented Apr 12, 2022

This issue is no longer marked for closure.

@stale stale bot removed the stale label Apr 12, 2022
@stale
Copy link

stale bot commented Jul 12, 2022

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Jul 12, 2022
@stale
Copy link

stale bot commented Aug 12, 2022

This pull request has been closed due to inactivity. If you feel this is in error, please reopen the pull request or file a new PR with the relevant details.

@stale stale bot closed this Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sysctl-setup.yml doesn't recognize Ubuntu 20.04
5 participants