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

Proposed by Max Brustkern
Status: Merged
Approved by: Javier Collado
Approved revision: 771
Merged at revision: 770
Proposed branch: lp:~nuclearbob/utah/desktop-resolvconf
Merge into: lp:utah
Diff against target: 121 lines (+13/-13)
3 files modified
utah/provisioning/baremetal/bamboofeeder.py (+5/-6)
utah/provisioning/baremetal/cobbler.py (+4/-4)
utah/provisioning/provisioning.py (+4/-3)
To merge this branch: bzr merge lp:~nuclearbob/utah/desktop-resolvconf
Reviewer Review Type Date Requested Status
Dimitri John Ledkov (community) Disapprove
Javier Collado (community) Approve
Review via email: mp+138236@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Javier Collado (javier.collado) wrote :

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
Revision history for this message
Max Brustkern (nuclearbob) wrote :

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.

Revision history for this message
Javier Collado (javier.collado) wrote :

Thanks for the response. Merging now.

review: Approve
Revision history for this message
Javier Collado (javier.collado) wrote :

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

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

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
Revision history for this message
Javier Collado (javier.collado) wrote :

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

Revision history for this message
Max Brustkern (nuclearbob) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'utah/provisioning/baremetal/bamboofeeder.py'
2--- utah/provisioning/baremetal/bamboofeeder.py 2012-11-07 19:49:03 +0000
3+++ utah/provisioning/baremetal/bamboofeeder.py 2012-12-05 16:09:25 +0000
4@@ -93,7 +93,7 @@
5 self.imagedir = os.path.join(self.tmpdir, 'image.d')
6 if not os.path.isdir(self.imagedir):
7 os.makedirs(self.imagedir)
8- self.sector = int(subprocess.check_output(['file',
9+ self.sector = int(subprocess.check_output(['file',
10 self.wwwimage]
11 ).strip().split(';')[1].split(',')[3].split()[1])
12 self.logger.debug('Image start sector is ' + str(self.sector))
13@@ -101,7 +101,7 @@
14 self.logger.debug('Mounting ' + self.wwwimage + ' at ' +
15 self.imagedir)
16 self._runargs(['sudo', 'mount', self.wwwimage, self.imagedir,
17- '-o', 'loop', '-o', 'offset=' + str(self.sector*512),
18+ '-o', 'loop', '-o', 'offset=' + str(self.sector * 512),
19 '-o', 'uid=' + config.user])
20
21 def _setupconsole(self):
22@@ -136,7 +136,7 @@
23 pipe.append('cpio -ivd 2>/dev/null', '-.')
24 # TODO: suppress dd output
25 pipe.copy(self.initrd, '/dev/null')
26-
27+
28 def _repackinitrd(self):
29 """
30 Repack the uInitrd file from the initrd.d directory.
31@@ -232,7 +232,7 @@
32 self.cleanfile(preenvfile)
33 self.logger.debug('Writing preenv setup to ' + preenvfile)
34 open(preenvfile, 'w').write(self.preenv)
35-
36+
37 def _create(self):
38 """
39 Install the OS on the system.
40@@ -258,7 +258,6 @@
41 myfile.write(
42 """
43 chroot /target sh -c 'sed "/eth[0-9]/d" -i /etc/network/interfaces'
44-cp /etc/resolv.conf /target/etc/resolv.conf
45 """)
46 myfile.close()
47 self._setuppreseed()
48@@ -293,7 +292,7 @@
49 self.cleanfunction(self.run, ('dd', 'bs=512k', 'count=10',
50 'if=/dev/zero', 'of=/dev/mmcblk0'), root=True)
51 return True
52-
53+
54 def _depcheck(self):
55 """
56 Check for dependencies that are in Recommends or Suggests.
57
58=== modified file 'utah/provisioning/baremetal/cobbler.py'
59--- utah/provisioning/baremetal/cobbler.py 2012-11-15 20:02:41 +0000
60+++ utah/provisioning/baremetal/cobbler.py 2012-12-05 16:09:25 +0000
61@@ -94,7 +94,6 @@
62 myfile.write(
63 """
64 chroot /target sh -c 'sed "/eth[0-9]/d" -i /etc/network/interfaces'
65-cp /etc/resolv.conf /target/etc/resolv.conf
66 """)
67 myfile.close()
68 self._setuppreseed()
69@@ -165,7 +164,8 @@
70 + ['--' + arg + '=' + self.cargs[arg]
71 for arg in self.cargs])
72
73- # Things seem to be working fine without this, but sometimes things still get broken
74+ # Things seem to be working fine without this,
75+ # but sometimes things still get broken
76 #self.logger.info('Syncing cobbler')
77 #self._cobble(['sync'])
78 try:
79@@ -253,7 +253,7 @@
80 except UTAHBMProvisioningException:
81 self._runargs(self.powercommand() + ['off'])
82 self.active = False
83-
84+
85 def __del__(self):
86 """
87 If the machine has an inventory, tell the inventory to release the
88@@ -273,7 +273,7 @@
89 '/d', '-i', config.nfsconfigfile])
90 self.logger.debug('Reloading NFS config')
91 self._runargs(config.nfscommand)
92-
93+
94 def _cleanuptmpdir(self):
95 if os.path.isdir(self.tmpdir):
96 # Cribbed from http://svn.python.org
97
98=== modified file 'utah/provisioning/provisioning.py'
99--- utah/provisioning/provisioning.py 2012-11-23 13:08:36 +0000
100+++ utah/provisioning/provisioning.py 2012-12-05 16:09:25 +0000
101@@ -1069,8 +1069,9 @@
102 .format(install_commands)
103 )
104 chroot_command = ' '.join([install_commands, postinstall_commands])
105- question.value.prepend('chroot /target sh -c \'{}\'; '
106- .format(chroot_command))
107+ question.value.prepend('cp /etc/resolv.conf '
108+ '/target/etc/resolv.conf ; chroot /target sh -c \'{}\'; '
109+ .format(chroot_command))
110 question.prepend("ubiquity ubiquity/summary note")
111 question.prepend("ubiquity ubiquity/reboot boolean true")
112 else:
113@@ -1093,7 +1094,7 @@
114 """
115 question = preseed['pkgsel/include']
116 packages = question.value.text.split()
117- for pkgname in ('openssh-server', 'gdebi-core'):
118+ for pkgname in config.installpackages:
119 if pkgname not in packages:
120 self.logger.info('Adding {} to preseeded packages'
121 .format(pkgname))

Subscribers

People subscribed via source and target branches