Merge lp:~nuclearbob/utah/desktop-resolvconf into lp:utah

[Max Brustkern]

This change makes all desktop installs copy resolv.conf from the installer environment into the target environment before running chroot commands. We've been doing this for a while on physical machine installs, and it works well to allow package installation to take place as part of the latecommand.

Unfortunately, that is not the only problem with the current code. Now that we have a utah-common package, the client installation fails since we don't install that as well. I'll be submitting a separate proposal for that shortly. We could test them together, but even in the absence of that fix, this one should get ssh installed and fail on client installation, unlike the current code on virtual machine desktop installs, which fails to connect to the machine after it reboots following the install.

[Javier Collado (javier.collado)]

I've tested the code and it works fine. Anyway, I've got a couple of questions:

Could the `/etc/resolv.conf` file in the host machine be broken in some way
that it's unusable in the guest machine?

Should we remove the lines that append `apt-get install` commands to
`/etc/rc.local`?

review: Needs Information

[Max Brustkern (nuclearbob)]

If the resolv.conf file in the installation environment is broken, then the package install will fail, so I think having the install in rc.local is a good safeguard against that. I'm not sure what would cause it to be broken, as I haven't seen that happen before.

[Javier Collado (javier.collado)]

Thanks for the response. Merging now.

review: Approve

[Javier Collado (javier.collado)]

By the way, thanks for fixing pep8 warnings as well.

[Dimitri John Ledkov (xnox)]

resolv.conf should always be a symlink to a file in /run generated by resolvconf daemon.
Instead of overriding a symlink with static config, /run should be bindmounted instead.
I will propose an update.

$ ls -la /etc/resolv.conf
lrwxrwxrwx 1 root root 29 Feb 3 2012 /etc/resolv.conf -> ../run/resolvconf/resolv.conf

review: Disapprove

[Javier Collado (javier.collado)]

It's a shame I already merged, but I'll be happy to review your fix and test
it.

[Max Brustkern (nuclearbob)]

His branch removes this, but this does work on the current raring ISOs. Since this is already merged, we can use it to get smoke testing working while we test the better replacement. I'm testing it on physical hardware in magners now.

=== modified file 'utah/provisioning/baremetal/bamboofeeder.py'
--- utah/provisioning/baremetal/bamboofeeder.py	2012-11-07 19:49:03 +0000
+++ utah/provisioning/baremetal/bamboofeeder.py	2012-12-05 16:09:25 +0000
@@ -93,7 +93,7 @@
         self.imagedir = os.path.join(self.tmpdir, 'image.d')
         if not os.path.isdir(self.imagedir):
             os.makedirs(self.imagedir)
-        self.sector = int(subprocess.check_output(['file', 
+        self.sector = int(subprocess.check_output(['file',
                 self.wwwimage]
                 ).strip().split(';')[1].split(',')[3].split()[1])
         self.logger.debug('Image start sector is ' + str(self.sector))
@@ -101,7 +101,7 @@
         self.logger.debug('Mounting ' + self.wwwimage + ' at ' +
                           self.imagedir)
         self._runargs(['sudo', 'mount', self.wwwimage, self.imagedir,
-                       '-o', 'loop', '-o', 'offset=' + str(self.sector*512),
+                       '-o', 'loop', '-o', 'offset=' + str(self.sector * 512),
                        '-o', 'uid=' + config.user])
 
     def _setupconsole(self):
@@ -136,7 +136,7 @@
         pipe.append('cpio -ivd 2>/dev/null', '-.')
         # TODO: suppress dd output
         pipe.copy(self.initrd, '/dev/null')
-        
+
     def _repackinitrd(self):
         """
         Repack the uInitrd file from the initrd.d directory.
@@ -232,7 +232,7 @@
         self.cleanfile(preenvfile)
         self.logger.debug('Writing preenv setup to ' + preenvfile)
         open(preenvfile, 'w').write(self.preenv)
-        
+
     def _create(self):
         """
         Install the OS on the system.
@@ -258,7 +258,6 @@
             myfile.write(
 """
 chroot /target sh -c 'sed "/eth[0-9]/d" -i /etc/network/interfaces'
-cp /etc/resolv.conf /target/etc/resolv.conf
 """)
             myfile.close()
         self._setuppreseed()
@@ -293,7 +292,7 @@
         self.cleanfunction(self.run, ('dd', 'bs=512k', 'count=10',
                 'if=/dev/zero', 'of=/dev/mmcblk0'), root=True)
         return True
-        
+
     def _depcheck(self):
         """
         Check for dependencies that are in Recommends or Suggests.
 
=== modified file 'utah/provisioning/baremetal/cobbler.py'
--- utah/provisioning/baremetal/cobbler.py	2012-11-15 20:02:41 +0000
+++ utah/provisioning/baremetal/cobbler.py	2012-12-05 16:09:25 +0000
@@ -94,7 +94,6 @@
             myfile.write(
 """
 chroot /target sh -c 'sed "/eth[0-9]/d" -i /etc/network/interfaces'
-cp /etc/resolv.conf /target/etc/resolv.conf
 """)
             myfile.close()
         self._setuppreseed()
@@ -165,7 +164,8 @@
                       + ['--' + arg + '=' + self.cargs[arg]
                          for arg in self.cargs])
 
-        # Things seem to be working fine without this, but sometimes things still get broken
+        # Things seem to be working fine without this,
+        # but sometimes things still get broken
         #self.logger.info('Syncing cobbler')
         #self._cobble(['sync'])
         try:
@@ -253,7 +253,7 @@
         except UTAHBMProvisioningException:
             self._runargs(self.powercommand() + ['off'])
         self.active = False
-        
+
     def __del__(self):
         """
         If the machine has an inventory, tell the inventory to release the
@@ -273,7 +273,7 @@
                        '/d', '-i', config.nfsconfigfile])
         self.logger.debug('Reloading NFS config')
         self._runargs(config.nfscommand)
-        
+
     def _cleanuptmpdir(self):
         if os.path.isdir(self.tmpdir):
             # Cribbed from http://svn.python.org
 
=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-11-23 13:08:36 +0000
+++ utah/provisioning/provisioning.py	2012-12-05 16:09:25 +0000
@@ -1069,8 +1069,9 @@
                 .format(install_commands)
             )
             chroot_command = ' '.join([install_commands, postinstall_commands])
-            question.value.prepend('chroot /target sh -c \'{}\'; '
-                                   .format(chroot_command))
+            question.value.prepend('cp /etc/resolv.conf '
+                '/target/etc/resolv.conf ; chroot /target sh -c \'{}\'; '
+                .format(chroot_command))
             question.prepend("ubiquity ubiquity/summary note")
             question.prepend("ubiquity ubiquity/reboot boolean true")
         else:
@@ -1093,7 +1094,7 @@
         """
         question = preseed['pkgsel/include']
         packages = question.value.text.split()
-        for pkgname in ('openssh-server', 'gdebi-core'):
+        for pkgname in config.installpackages:
             if pkgname not in packages:
                 self.logger.info('Adding {} to preseeded packages'
                                  .format(pkgname))