Merge lp:~oreilldf/wicd/resolvconf2 into lp:wicd/1.6

[Dan O'Reilly]

[Dan O'Reilly (oreilldf)]

Adds support for using resolvconf to set DNS settings instead of directly editing resolv.conf.

[David Paleino (dpaleino)]

I suggest changing the last chunk of the patch as follows:

---8<---
@@ -518,7 +496,51 @@
             return
         if self.verbose: print cmd
         misc.Run(cmd)
-
+
+ def SetDNS(self, dns1=None, dns2=None, dns3=None,
+ dns_dom=None, search_dom=None):
+ """ Set the DNS of the system to the specified DNS servers.
+
+ Opens up resolv.conf and writes in the nameservers.
+
+ Keyword arguments:
+ dns1 -- IP address of DNS server 1
+ dns2 -- IP address of DNS server 2
+ dns3 -- IP address of DNS server 3
+ dns_dom -- DNS domain
+ search_dom -- DNS search domain
+
+ """
+ resolv_params = ""
+ if dns_dom:
+ resolv_params = ''.join([resolv_params, 'domain ', dns_dom, '\n'])
+ if search_dom:
+ resolv_params = ''.join([resolv_params, 'search ', search_dom,
+ '\n'])
+ valid_dns_list = []
+ for dns in [dns1, dns2, dns3]:
+ if dns:
+ if misc.IsValidIP(dns):
+ print 'Setting DNS : ' + dns
+ valid_dns_list.append(dns)
+ else:
+ print 'DNS IP %s is not a valid IP address, skipping' % dns
+
+ if self.resolvconf_cmd:
+ # Make sure we have at least one DNS in the list.
+ if valid_dns_list:
+ resolv_params += 'nameserver ' + ' '.join(valid_dns_list) + '\n'
+
+ print "running resolvconf"
+ p = misc.Run(' '.join([self.resolvconf_cmd, '-a', self.iface]),
+ include_stderr=True, return_obj=True)
+ p.communicate(input=resolv_params)[0]
+ else:
+ resolv = open("/etc/resolv.conf", "w")
+ resolv.write(resolv_params + "\n")
+ for dns in valid_dns_list:
+ resolv.write('nameserver %s\n' % dns)
+ resolv.close()

     def FlushRoutes(self):
         """ Flush network routes for this device. """
--->8---

Let's go step-by-step.
Here:

+ else:
+ print 'DNS IP is not a valid IP address, skipping'

It would be rather useful printing *which* DNS is not a valid IP address.

Also, later we have:

In the same file:

+ # Make sure we have more than just 'nameserver' in the list.
+ if len(valid_dns_list) > 1:
+ resolv_params += ' '.join(valid_dns_list) + '\n'
+
+ if self.resolvconf_cmd:
+ print "running resolvconf"
+ p = misc.Run(' '.join([self.resolvconf_cmd, '-a', self.iface]),
+ include_stderr=True, return_obj=True)
+ p.communicate(input=resolv_params)[0]
+ else:
+ resolv = open("/etc/resolv.conf", "w")
+ resolv.write(resolv_params + "\n")
+ resolv.close()

This is wrong. /etc/resolv.conf accepts maximum MAXNS (currently 3, see <resolv.h>) IPs listed after "nameserver", while resolvconf gracefully handles that by putting each IP on a separate line (i.e. "nameserver A B" used with resolvconf would actually give "nameserver A\nnameserver B" in /etc/resolv.conf).

O...

review: Needs Fixing

[lp:~oreilldf/wicd/resolvconf2]

326. By Dan O'Reilly on 2009-03-14

Fix how we set nameserver entries after code review.

327. By Dan O'Reilly on 2009-03-15

Only print "Setting DNS" message if debug mode is on.

Unmerged revisions

=== modified file 'wicd/misc.py'
--- wicd/misc.py	2009-03-09 03:39:41 +0000
+++ wicd/misc.py	2009-03-09 04:07:39 +0000
@@ -95,6 +95,10 @@
     else:
         err = None
         fds = False
+    if return_obj:
+        std_in = PIPE
+    else:
+        std_in = None
     
     # We need to make sure that the results of the command we run
     # are in English, so we set up a temporary environment.
@@ -105,8 +109,8 @@
     tmpenv["LANG"] = __LANG
     
     try:
-        f = Popen(cmd, shell=False, stdout=PIPE, stderr=err, close_fds=fds,
-                  cwd='/', env=tmpenv)
+        f = Popen(cmd, shell=False, stdout=PIPE, stdin=std_in, stderr=err,
+                  close_fds=fds, cwd='/', env=tmpenv)
     except OSError, e:
         print "Running command %s failed: %s" % (str(cmd), str(e))
         return ""
 
=== modified file 'wicd/networking.py'
--- wicd/networking.py	2009-03-09 03:39:41 +0000
+++ wicd/networking.py	2009-03-09 04:07:39 +0000
@@ -427,7 +427,7 @@
                 return
         
     @abortable
-    def set_dns_addresses(self):
+    def set_dns_addresses(self, iface):
         """ Set the DNS address(es).
 
         If static DNS servers or global DNS servers are specified, set them.
@@ -435,19 +435,19 @@
         
         """
         if self.network.get('use_global_dns'):
-            BACKEND.SetDNS(misc.Noneify(self.global_dns_1),
-                           misc.Noneify(self.global_dns_2), 
-                           misc.Noneify(self.global_dns_3),
-                           misc.Noneify(self.global_dns_dom),
-                           misc.Noneify(self.global_search_dom))
+            iface.SetDNS(misc.Noneify(self.global_dns_1),
+                         misc.Noneify(self.global_dns_2), 
+                         misc.Noneify(self.global_dns_3),
+                         misc.Noneify(self.global_dns_dom),
+                         misc.Noneify(self.global_search_dom))
         elif self.network.get('use_static_dns') and (self.network.get('dns1') or
                     self.network.get('dns2') or self.network.get('dns3')):
             self.SetStatus('setting_static_dns')
-            BACKEND.SetDNS(self.network.get('dns1'),
-                           self.network.get('dns2'),
-                           self.network.get('dns3'),
-                           self.network.get('dns_domain'),
-                           self.network.get('search_domain'))
+            iface.SetDNS(self.network.get('dns1'),
+                         self.network.get('dns2'),
+                         self.network.get('dns3'),
+                         self.network.get('dns_domain'),
+                         self.network.get('search_domain'))
 
     @abortable
     def release_dhcp_clients(self, iface):
@@ -837,7 +837,7 @@
         # Set up gateway, IP address, and DNS servers.
         self.set_broadcast_address(wiface)
         self.set_ip_address(wiface)
-        self.set_dns_addresses()
+        self.set_dns_addresses(wiface)
         
         # Run post-connection script.
         self.run_global_scripts_if_needed(wpath.postconnectscripts)
@@ -1025,7 +1025,7 @@
         # Set gateway, IP adresses, and DNS servers.
         self.set_broadcast_address(liface)
         self.set_ip_address(liface)
-        self.set_dns_addresses()
+        self.set_dns_addresses(liface)
         
         # Run post-connection script.
         self.run_global_scripts_if_needed(wpath.postconnectscripts)
 
=== modified file 'wicd/wnettools.py'
--- wicd/wnettools.py	2009-03-09 03:09:12 +0000
+++ wicd/wnettools.py	2009-03-09 04:07:39 +0000
@@ -74,7 +74,7 @@
 blacklist_norm = ";`$!*|><&\\"
 blank_trans = maketrans("", "")
 
-__all__ = ["SetDNS", "GetDefaultGateway", "GetWiredInterfaces",
+__all__ = ["GetDefaultGateway", "GetWiredInterfaces",
            "GetWirelessInterfaces", "IsValidWpaSuppDriver"]
 
 def _sanitize_string(string):
@@ -89,33 +89,6 @@
     else:
         return string
 
-def SetDNS(dns1=None, dns2=None, dns3=None, dns_dom=None, search_dom=None):
-    """ Set the DNS of the system to the specified DNS servers.
-
-    Opens up resolv.conf and writes in the nameservers.
-
-    Keyword arguments:
-    dns1 -- IP address of DNS server 1
-    dns2 -- IP address of DNS server 2
-    dns3 -- IP address of DNS server 3
-    dns_dom -- DNS domain
-    search_dom -- DNS search domain
-
-    """
-    resolv = open("/etc/resolv.conf", "w")
-    if dns_dom:
-        resolv.write("domain %s\n" % dns_dom)
-    if search_dom:
-        resolv.write('search %s\n' % search_dom)
-    for dns in [dns1, dns2, dns3]:
-        if dns:
-            if misc.IsValidIP(dns):
-                print 'Setting DNS : ' + dns
-                resolv.write('nameserver ' + dns + '\n')
-            else:
-                print 'DNS IP is not a valid IP address, not writing to resolv.conf'
-    resolv.close()
-
 def GetDefaultGateway():
     """ Attempts to determine the default gateway by parsing route -n. """
     route_info = misc.Run("route -n")
@@ -291,6 +264,11 @@
         self.CheckWirelessTools()
         self.CheckSudoApplications()
         self.CheckRouteFlushTool()
+        self.CheckResolvConf()
+
+    def CheckResolvConf(self):
+        """ Checks for the existence of resolvconf."""
+        self.resolvconf_cmd = self._find_program_path("resolvconf")
         
     def CheckDHCP(self):
         """ Check for the existence of valid DHCP clients. 
@@ -518,7 +496,48 @@
             return 
         if self.verbose: print cmd
         misc.Run(cmd)
-            
+
+    def SetDNS(self, dns1=None, dns2=None, dns3=None, 
+               dns_dom=None, search_dom=None):
+        """ Set the DNS of the system to the specified DNS servers.
+
+        Opens up resolv.conf and writes in the nameservers.
+
+        Keyword arguments:
+        dns1 -- IP address of DNS server 1
+        dns2 -- IP address of DNS server 2
+        dns3 -- IP address of DNS server 3
+        dns_dom -- DNS domain
+        search_dom -- DNS search domain
+
+        """
+        resolv_params = ""
+        if dns_dom:
+            resolv_params = ''.join([resolv_params, 'domain ', dns_dom, '\n'])
+        if search_dom:
+            resolv_params = ''.join([resolv_params, 'search ', search_dom,
+                                     '\n'])
+        valid_dns_list = ['nameserver']
+        for dns in [dns1, dns2, dns3]:
+            if dns:
+                if misc.IsValidIP(dns):
+                    print 'Setting DNS : ' + dns
+                    valid_dns_list.append(dns)
+                else:
+                    print 'DNS IP is not a valid IP address, skipping'
+        # Make sure we have more than just 'nameserver' in the list.
+        if len(valid_dns_list) > 1:
+            resolv_params +=  ' '.join(valid_dns_list) + '\n'
+
+        if self.resolvconf_cmd:
+            print "running resolvconf"
+            p = misc.Run(' '.join([self.resolvconf_cmd, '-a', self.iface]),
+                         include_stderr=True, return_obj=True)
+            p.communicate(input=resolv_params)[0]
+        else:
+            resolv = open("/etc/resolv.conf", "w")
+            resolv.write(resolv_params + "\n")
+            resolv.close()
         
     def FlushRoutes(self):
         """ Flush network routes for this device. """