Fixes razorsedge#104 vlan in device config

matschundbrei · Nov 29, 2017 · 448680f · 448680f
1 parent 106d300
commit 448680f
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 39 deletions.
diff --git a/README.md b/README.md
@@ -247,26 +247,21 @@ Static interface routes (host route):
 
 Normal interface - VLAN - no IP:
 
-    class { 'network::global':
-      vlan => 'yes',
-    }
-
     network::if { 'eth0.330':
       ensure => 'up',
-    }
+      vlan => true,
+   }
 
 Normal interface - VLAN - static IPv4:
 
-    class { 'network::global':
-      vlan => 'yes',
-    }
-
     network::if::static { 'eth0.330':
       ensure    => 'up',
       ipaddress => '10.2.3.248',
       netmask   => '255.255.255.0',
+      vlan => true,
     }
 
+
 Promiscuous interface:
     To set a static or dynamic interface to promiscuous mode (RedHat only), add:
       promisc => true
@@ -333,7 +328,6 @@ Notes
 * It is assumed that if you create a bond that you also create the slave interface(s).
 * It is assumed that if you create an alias that you also create the parent interface.
 * network::route requires the referenced device to also be defined via network::if or network::bond.
-* For VLANs to work, `Class['network::global']` must have parameter `vlan` set to `yes`.
 * To enable IPv6 you have to set both `ipv6networking` in `Class['network::global']` to `true` and `ipv6init` in `network::if::static` to `true`.
 
 Issues

diff --git a/manifests/global.pp b/manifests/global.pp
@@ -20,7 +20,6 @@
 #   $ipv6defaultdev - optional - Determines the device to use as the default gateway
 #                                for IPV6 traffic.
 #   $nisdomain      - optional - Configures the NIS domainname.
-#   $vlan           - optional - yes|no to enable VLAN kernel module
 #   $ipv6networking - optional - enables / disables IPv6 globally
 #   $nozeroconf     - optional
 #   $restart        - optional - defaults to true
@@ -43,7 +42,6 @@
 #     ipv6gateway    => '123:4567:89ab:cdef:123:4567:89ab:1',
 #     ipv6defaultdev => 'eth0',
 #     nisdomain      => 'domain.tld',
-#     vlan           => 'yes',
 #     ipv6networking => true,
 #     nozeroconf     => 'yes',
 #     requestreopen  => false,
@@ -65,10 +63,10 @@
   $hostname       = undef,
   $gateway        = undef,
   $gatewaydev     = undef,
+  $vlan           = false,
   $ipv6gateway    = undef,
   $ipv6defaultdev = undef,
   $nisdomain      = undef,
-  $vlan           = undef,
   $ipv6networking = false,
   $nozeroconf     = undef,
   $restart        = true,
@@ -82,16 +80,11 @@
     if ! is_ip_address($ipv6gateway) { fail("${ipv6gateway} is not an IPv6 address.") }
   }
 
+  validate_bool($vlan)
   validate_bool($ipv6networking)
   validate_bool($restart)
   validate_bool($requestreopen)
 
-  # Validate our regular expressions
-  if $vlan {
-    $states = [ '^yes$', '^no$' ]
-    validate_re($vlan, $states, '$vlan must be either "yes" or "no".')
-  }
-
   include '::network'
 
   case $::operatingsystem {

diff --git a/manifests/if/static.pp b/manifests/if/static.pp
@@ -29,6 +29,7 @@
 #   $defroute       - optional
 #   $restart        - optional - defaults to true
 #   $arpcheck       - optional - defaults to true
+#   $vlan           - optional - defaults to false
 #
 # === Actions:
 #
@@ -82,6 +83,7 @@
   $metric = undef,
   $restart = true,
   $arpcheck = true,
+  $vlan = false,
 ) {
   # Validate our data
   if $ipaddress {
@@ -118,6 +120,7 @@
   validate_bool($manage_hwaddr)
   validate_bool($flush)
   validate_bool($arpcheck)
+  validate_bool($vlan)
 
   network_if_base { $title:
     ensure          => $ensure,
@@ -148,5 +151,6 @@
     metric          => $metric,
     restart         => $restart,
     arpcheck        => $arpcheck,
+    vlan            => $vlan,
   }
 } # define network::if::static
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -79,6 +79,7 @@
 #   $promisc         - optional - defaults to false
 #   $restart         - optional - defaults to true
 #   $arpcheck        - optional - defaults to true
+#   $vlan            - optional - defaults to false
 #
 # === Actions:
 #
@@ -140,6 +141,7 @@
   $promisc         = false,
   $restart         = true,
   $arpcheck        = true,
+  $vlan            = false,
 ) {
   # Validate our booleans
   validate_bool($noaliasrouting)
@@ -155,6 +157,7 @@
   validate_bool($promisc)
   validate_bool($restart)
   validate_bool($arpcheck)
+  validate_bool($vlan)
   # Validate our regular expressions
   $states = [ '^up$', '^down$' ]
   validate_re($ensure, $states, '$ensure must be either "up" or "down".')
@@ -192,7 +195,6 @@
     }
     $iftemplate = template('network/ifcfg-eth.erb')
   }
-
   if $flush {
     exec { 'network-flush':
       user        => 'root',

diff --git a/spec/classes/network_global_spec.rb b/spec/classes/network_global_spec.rb
@@ -82,7 +82,7 @@
       :gateway    => '1.2.3.4',
       :gatewaydev => 'eth2',
       :nisdomain  => 'myNisDomain',
-      :vlan       => 'yes',
+      :vlan       => true,
       :nozeroconf => 'yes',
     }
     end
@@ -101,7 +101,6 @@
         'GATEWAY=1.2.3.4',
         'GATEWAYDEV=eth2',
         'NISDOMAIN=myNisDomain',
-        'VLAN=yes',
         'NOZEROCONF=yes',
         'RES_OPTIONS="single-request-reopen"',
       ])
@@ -156,24 +155,14 @@
       end
     end
 
-    context 'vlan = foo' do
-      let(:params) {{ :vlan => 'foo' }}
-
-      it 'should fail' do
-        expect {
-          should raise_error(Puppet::Error, /$vlan must be either "yes" or "no"./)
-        }
-      end
-    end
-
     context 'ipv6networking = foo' do
       let(:params) {{ :ipv6networking => 'foo' }}
-
+  
       it 'should fail' do
         expect { 
           should raise_error(Puppet::Error, /$ipv6networking is not a boolean.  It looks to be a String./)
         }
-      end
+        end
     end
 
     context 'manage_hwaddr = foo' do
@@ -204,7 +193,6 @@
       :gateway        => '1.2.3.4',
       :gatewaydev     => 'eth2',
       :nisdomain      => 'myNisDomain',
-      :vlan           => 'yes',
       :nozeroconf     => 'yes',
       :ipv6networking => true,
       :ipv6gateway    => '123:4567:89ab:cdef:123:4567:89ab:1',
@@ -229,7 +217,6 @@
         'GATEWAY=1.2.3.4',
         'GATEWAYDEV=eth2',
         'NISDOMAIN=myNisDomain',
-        'VLAN=yes',
         'NOZEROCONF=yes',
       ])
     end

diff --git a/spec/defines/network_if_static_spec.rb b/spec/defines/network_if_static_spec.rb
@@ -169,6 +169,7 @@
       :metric       => '10',
       :zone         => 'trusted',
       :arpcheck     => false,
+      :vlan         => true,
     }
     end
     let :facts do {
@@ -212,7 +213,8 @@
         'DEFROUTE=yes',
         'ZONE=trusted',
         'METRIC=10',
-	'ARPCHECK=no',
+      	'ARPCHECK=no',
+        'VLAN=yes',
         'NM_CONTROLLED=no',
       ])
     end
@@ -225,6 +227,7 @@
       :ensure    => 'up',
       :ipaddress => '1.2.3.4',
       :netmask   => '255.255.255.0',
+      :vlan      => true,
     }
     end
     let :facts do {
@@ -250,6 +253,7 @@
         'TYPE=Ethernet',
         'IPADDR=1.2.3.4',
         'NETMASK=255.255.255.0',
+        'VLAN=yes',
         'NM_CONTROLLED=no',
       ])
     end

diff --git a/templates/ifcfg-eth.erb b/templates/ifcfg-eth.erb
@@ -70,4 +70,6 @@ check_link_down() {
 <% end -%>
 <% if !@arpcheck %>ARPCHECK=no
 <% end -%>
+<% if @vlan %>VLAN=yes
+<% end -%>
 NM_CONTROLLED=no
diff --git a/templates/network.erb b/templates/network.erb
@@ -18,7 +18,7 @@ NETWORKING=yes
 <% end -%>
 <% if @nisdomain %>NISDOMAIN=<%= @nisdomain %>
 <% end -%>
-<% if @vlan %>VLAN=<%= @vlan %>
+<% if @vlan %>VLAN=yes
 <% end -%>
 <% if @nozeroconf %>NOZEROCONF=<%= @nozeroconf %>
 <% end -%>

diff --git a/tests/global.pp b/tests/global.pp
@@ -1,7 +1,7 @@
 class { 'network::global':
   gateway        => '1.2.3.1',
   gatewaydev     => 'eth0',
-  vlan           => 'yes',
+  vlan           => true,
   nozeroconf     => 'yes',
   ipv6networking => true,
   ipv6gateway    => '123:4567:89ab:cdef:123:4567:89ab:1',