Skip to content

Added macvlan network to allow for pi-hole DHCP #10

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

Closed
wants to merge 5 commits into from

Conversation

Duckle29
Copy link
Contributor

Because DHCP is dependent on network broadcasts, the default bridged network won't work.

The easiest method would probably just be to set the network mode to host, but then we risk running into port conflicts.
because of this, and because the pi-hole server needs a static IP if it is to act as DHCP server, I decided to instead go for the macvlan network driver, which gives the pi-hole container a static IP on the same network as the pi.

I'd like some feedback on how to handle this.
As it stands you need pip install netaddr on the ansible controller, as I have the pi-hole task calculate the CIDR mask from the netmask. That could alternatively be provided by the user in the config?

Maybe it should also be optional, and just let people run the default bridged mode if they don't intend to use the DHCP functionality?

@geerlingguy
Copy link
Owner

See also: #8

@Duckle29 Duckle29 marked this pull request as ready for review June 23, 2021 15:53
@Duckle29
Copy link
Contributor Author

I've been running this for a while now and it hasn't broken yet :)

# More info at https://github.com/pi-hole/docker-pi-hole/ and https://docs.pi-hole.net/
services:
pihole:
container_name: pihole
image: pihole/pihole:latest
hostname: '{{ pihole_hostname }}'
networks:
pihole_network:
ipv4_address: '{{ pihole_ipaddr }}'
Copy link
Owner

Choose a reason for hiding this comment

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

Where is pihole_ipaddr being defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's defined in the config.yml I'll add it to the example file but I feel like it might need more explanation, maybe in the readme?


- name: CIDR subnet is
ansible.builtin.debug:
msg: 'Subnet: {{ mask_cidr }}'
Copy link
Owner

Choose a reason for hiding this comment

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

Are these two debug tasks needed? Or were they just for testing/debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The debug message isn't needed. I've removed that, however mask_cidr is used in the docker compose file

@sinistro14
Copy link

@geerlingguy, anything else required before merging ?

@dturaev
Copy link

dturaev commented Aug 1, 2021

I wanted to solve the same problem using a DHCP relay as described here, just for fun. The relay runs in an additional container, and is enabled/disabled from the config file. Let me know if you're interested in a pull request.

@stale
Copy link

stale bot commented Oct 31, 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 Oct 31, 2021
@sinistro14
Copy link

@dturaev, would be a nice addition. Let me know if you have it ready on your end. I've been wanting to implement it for long.

@stale
Copy link

stale bot commented Nov 4, 2021

This issue is no longer marked for closure.

@stale stale bot removed the stale label Nov 4, 2021
@stale
Copy link

stale bot commented Feb 3, 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 Feb 3, 2022
@stale
Copy link

stale bot commented Mar 6, 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.

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.

4 participants