BUG, ENH, PERF: Debian/Ubuntu network configuration

[westurner]

Initially: "debian_ip ifupdown /etc/network/interfaces options for tunnels, ipv6, and dns-search"

[EDIT]

Documentation

Ubuntu ifupdown Docs

Here are the (ubuntu) docs for /etc/network/interfaces (pkg: ifupdown).

• http://manpages.ubuntu.com/manpages/trusty/en/man5/interfaces.5.html
• http://manpages.ubuntu.com/manpages/precise/en/man5/interfaces.5.html
• http://manpages.ubuntu.com/manpages/lucid/en/man5/interfaces.5.html

Salt Docs

• http://docs.saltstack.com/en/latest/topics/releases/2014.1.0.html#network-management-for-debian-ubuntu
• http://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.debian_ip.html#module-salt.modules.debian_ip
• https://github.com/saltstack/salt/pull/9662 (iptables ipv6)

Sources

• https://github.com/saltstack/salt/blob/develop/salt/modules/debian_ip.py
• https://github.com/saltstack/salt/blob/develop/salt/templates/debian_ip/debian_eth.jinja
• https://github.com/westurner/salt/compare/debian_network_config

Tickets

I don't see any other tickets for e.g. tunnels, ipv6, or dns-search (as utilized by resolvconf) in debian_ip:

---

[EDIT]

BUG:

• [x] Unclosed file handles (-> file.open -> with salt.utils.flopen)
• [x] Don't write /etc/default/networking on Ubuntu >= 12.04
  • get_network_settings returns templated output to compare with templated output with difflib.unified_diff (in https://github.com/saltstack/salt/blob/develop/salt/states/network.py) and thus runs on every salt invocation w/ Ubuntu <= 12.04
  • https://github.com/saltstack/salt/commit/c70c2057b8dcf56d0948d1e61d94775e6dda992b
• * /etc/resolv.conf is overwritten when resolvconf is installed (this may be a documentation issue)

PERF:

• [x] Simplify /etc/network/interfaces parser logic

TST:

• [ ] Handle 'TODOs'
• [ ] Write tests
• [ ] Further (manual) testing on other platforms
  • I've run this manually on one platform a bunch of times.

ENH:

• [x] Add (rework/complete) debian ipv6 options
• [x] Add Adds dns-search option
• [x] Add wireless-* options
• [x] Add wpa-* options
• [x] Add basic seting validations

RLS:

• [x] Merge: https://github.com/garethgreenaway/salt/commit/d7884d5faa1ba2e85a78dacbf1857607dc959768
• [x] Release: https://github.com/saltstack/salt/releases/tag/v2014.7:
  • https://github.com/saltstack/salt/commits/2014.7/salt/modules/debian_ip.py
  • https://github.com/saltstack/salt/blob/develop/salt/templates/debian_ip/debian_eth.jinja

[westurner]

... @garethgreenaway et al: is this a unique issue?

[garethgreenaway]

Sorry, not sure what the issue is.

[basepi]

@westurner I'm unsure of what the purpose of this issue is. You linked some docs. What's the feature/bug you're looking to get fixed?

[westurner]

It is not possible to:

• configure IP tunnels
• configure IPV6
• configure dns-search for resolvconf

https://github.com/westurner/salt/tree/debian_network_config

Wes Turner On Apr 24, 2014 2:26 PM, "Colton Myers" notifications@github.com wrote:

@westurner https://github.com/westurner I'm unsure of what the purpose of this issue is. You linked some docs. What's the feature/bug you're looking to get fixed?

— Reply to this email directly or view it on GitHubhttps://github.com/saltstack/salt/issues/12216#issuecomment-41321445 .

[westurner]

[EDIT] BUG

Also, there are unclosed file handles

Wes Turner On Apr 24, 2014 2:26 PM, "Colton Myers" notifications@github.com wrote:

@westurner https://github.com/westurner I'm unsure of what the purpose of this issue is. You linked some docs. What's the feature/bug you're looking to get fixed?

— Reply to this email directly or view it on GitHubhttps://github.com/saltstack/salt/issues/12216#issuecomment-41321445 .

[westurner]

From [1], I understand that I can test this by copying my modified debian_ip.py module into /srv/salt/_modules and then calling saltutil.sync_modules with a master/minion setup.

• Is there developer documentation on testing (changes to; new) modules?
• Where do I copy this modified template?

[1] http://docs.saltstack.com/en/latest/ref/modules/#modules-are-easy-to-write

[westurner]

https://github.com/westurner/salt/compare/debian_network_config

This:

• Uses a with context manager to ensure that files are closed even in the event of an exception (try/except/finally file.close)
• Updates the /etc/network/interfaces parser logic (I haven't looked at the ifupdown code, just the man interfaces docs)
• Renames context -> method (as in the docs)
• Renames inet_type to addrfam (as in the docs)
• Uses some of the constants defined at the top of the file
• Adds ipv6 options
• Adds dns-search option
• Adds wireless-* options
• Adds wpa-* options
• Adds basic seting validations
• Has not been tested

About the validations:

• Wrap salt.utils.validate.net where possible
• Return (valid: True/False, value: casted/transformed value, errmsg)
• Named e.g. __ipv4_quad pending a proper placement
• Could be added to salt.utils.validate.???

[garethgreenaway]

@westurner Some good additions :) I find it easier to run the installer from where ever you're code is, then run a master and a minion on the same host for testing. As you're testing, etc. run the installer and restart the minion.

[westurner]

@westurner Some good additions :)

Thx!

I find it easier to run the installer from where ever you're code is, then run a master and a minion on the same host for testing. As you're testing, etc. run the installer and restart the minion.

• As root?
• I will look into automating these crucial tests

[garethgreenaway]

Yup. both master and minion running as root, installer has to run as root because of file permissions.

[westurner]

Got it. I have a packer/vagrant/virtualbox setup that will help with this. I see the Jenkins setup and tests; is there an easier way with something like salt-cloud and lxc?

Wes Turner On Apr 25, 2014 12:16 AM, "garethgreenaway" notifications@github.com wrote:

Yup. both master and minion running as root, installer has to run as root because of file permissions.

— Reply to this email directly or view it on GitHubhttps://github.com/saltstack/salt/issues/12216#issuecomment-41359613 .

[westurner]

@garethgreenaway :

• When you specify network.system:hostname as a FQDN, is /etc/hostname updated every time?
• Does network_settings:network.system show a diff of a nonexistant file (/etc/default/networking) every time?

(With Ubuntu 12.04)

[westurner]

... [why] does salt.modules.debian_ip.get_network_settings return a single template (when the file doesn't exist on disk)? [1]

[1] https://github.com/saltstack/salt/blob/eade92145e32084379019b039644df7f7cf1064e/salt/modules/debian_ip.py#L1387

[westurner]

resolvconf may or may not be installed on Debian systems.

If resolvconf is not installed, writes to /etc/resolv.conf will persist.

If resolvconf is installed (as it is by default w/ Ubuntu 12.04 (and earlier, IIRC)):

• writes to /etc/resolv.conf are overwritten
• /etc/resolv.conf is generated from:
  • scripts in /etc/resolvconf
  • dns-nameservers and dns-search settings in /etc/network/interfaces (and /etc/network/interfaces.d)
  • data pulled from a DHCP server with e.g. dhclient (cached in /var/lib/dhcp/dhclient.${IFNAME}.leases)

Debian & Ubuntu DNS configuration docs references:

• https://en.wikipedia.org/wiki/Resolv.conf
• https://en.wikipedia.org/wiki/Resolvconf
• http://manpages.ubuntu.com/manpages/precise/en/man8/resolvconf.8.html
• http://www.debian.org/doc/manuals/debian-reference/ch05.en.html#_the_hostname_resolution

[bbinet]

what is the status of this work? these are useful additions: any chance it can be turned into a pull request to be merged?

[westurner]

Unfortunately I've been sidetracked and haven't yet learned/written tests for this sensitive functionality.

If someone has the time and/or resources to get this merged, by all means, please do.

I've only recently learned about the _modules folder.

Wes Turner On Jun 4, 2014 3:47 AM, "Bruno Binet" notifications@github.com wrote:

what is the status of this work? these are useful additions: any chance it can be turned into a pull request to be merged?

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/12216#issuecomment-45066306.

[garethgreenaway]

Going to take a look at getting these changes merged in.

[bbinet]

@garethgreenaway do you think the additions described in https://github.com/saltstack/salt/issues/12216#issuecomment-41358915 will be available in Helium release?

[garethgreenaway]

@bbinet these changes are in 2014.7 (Helium) now.

[bbinet]

Cool! Thanks @garethgreenaway.

[westurner]

@garethgreenaway Any suggestions on how to test the changes in https://github.com/westurner/salt/commit/f98ef01b876377328316d36ef54d7ed5a70ec1b7 ?

(There's an RFC at https://github.com/westurner/salt/commit/f98ef01b876377328316d36ef54d7ed5a70ec1b7#commitcomment-6131081 )

[EDIT]: I see: https://github.com/garethgreenaway/salt/commit/d7884d5faa1ba2e85a78dacbf1857607dc959768

and https://github.com/saltstack/salt/blob/2014.7/salt/modules/debian_ip.py

Thanks again!

TODO:

• [ ] issue desc < this list
• [ ] Rebase
• [ ] Don't write /etc/default/networking on Ubuntu >= 12.04
• [ ] Handle 'TODOs'
• [ ] Write tests
• [ ] Further (manual) testing on other platforms

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.