interfaces/network-control: remove incorrect rules for tun #4031

Merged
merged 3 commits into from Oct 12, 2017

Conversation

Projects
None yet
4 participants
Contributor

jdstrand commented Oct 12, 2017

We had two udev rules to tag tun: tun and tun[0-9]*. This results in
'udev_enumerate_scan failed' errors. This is because udev ships this rule

KERNEL=="tun", MODE="0666", OPTIONS+="static_node=net/tun"

so we don't need the tun[0-9]* or tap[0-9]* rules. Removing it resolves the issue. This PR is safe because tun[0-9]* and tap[0-9]* are virtual devices (like other network devices) and aren't represented in /dev, and therefore don't need to be added to the device cgroup and therefore don't need to be udev tagged.

This PR also revert 65b7ef7 to remove the unneeded apparmor rules (the devices don't show up in /dev-- these rules weren't ever needed and mistakenly added proactively).

jdstrand added some commits Oct 12, 2017

interfaces/network-control: remove incorrect rule for tun
We had two udev rules to tag tun: 'tun' and 'tun[0-9]*'. This results in
'udev_enumerate_scan failed' errors. This is because udev ships this rule

'KERNEL=="tun", MODE="0666", OPTIONS+="static_node=net/tun"

so we don't need the 'tun[0-9]*' rule. Removing it resolves the issue

@mvo5 mvo5 added this to the 2.28 milestone Oct 12, 2017

@jdstrand jdstrand changed the title from interfaces/network-control: remove incorrect rule for tun to interfaces/network-control: remove incorrect rules for tun Oct 12, 2017

Contributor

zyga commented Oct 12, 2017

@jdstrand the unit tests are failing after those changes

zyga approved these changes Oct 12, 2017

+1

Contributor

jdstrand commented Oct 12, 2017

Please note that I plan to add a spread test for tun/tap devices in particular, but due to time constraints it needs to be in a followup PR. Also due to said time constraints, I've only tested that this fixes the udev_enumerate_scan failure. This PR on its own should be safe for 2.28 because it is no worse than what exists now (indeed, it is a lot better-- anything with network-control connected will at least now start).

Codecov Report

Merging #4031 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4031   +/-   ##
=======================================
  Coverage   75.88%   75.88%           
=======================================
  Files         431      431           
  Lines       36915    36915           
=======================================
  Hits        28014    28014           
  Misses       6948     6948           
  Partials     1953     1953
Impacted Files Coverage Δ
interfaces/builtin/network_control.go 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ea3421...7a6e135. Read the comment docs.

mvo5 approved these changes Oct 12, 2017

@mvo5 mvo5 merged commit 2ee4fa0 into snapcore:master Oct 12, 2017

6 of 7 checks passed

artful-i386 autopkgtest running
Details
artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@jdstrand jdstrand deleted the jdstrand:fix-udev-network-control branch Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment