On Wed, May 06, 2009 at 11:29:59AM -0000, Soren Hansen wrote: > You have been requested to review the proposed merge of lp:~zulcss/vmbuilder/vmbuilder-jaunty-ec2 into lp:vmbuilder. > === added file 'VMBuilder/plugins/ec2/README' > --- VMBuilder/plugins/ec2/README 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/README 2009-05-04 13:58:48 +0000 > @@ -0,0 +1,14 @@ > +This is the command to use to create an ec2 image with vmbuilder: > + > +Note: you must have ec2-ami-tools installed. Yes, ec2-ami-tools are required, but it seems odd to put that in the README. Lots of other packages are needed, and they (like ec2-ami-tools) are listed as dependencies of the package. If you care about people who use vmbuilder outside the context of Ubuntu, you could add a preflight check to make sure the ami tools are installed, and bail out if not. > + > +sudo vmbuilder xen ubuntu --suite={hardy,intrepid} --ec2 \ > + --ec2-cert= \ > + --ec2-key= \ > + --ec2-access-key= \ > + --ec2-secret-key= \ > + --ec2-user= \ > + --ec2-bucket= \ > + --ec2-prefix= \ > + --ec2-version="Description of your EC2 image" \ > + --debug > > === modified file 'VMBuilder/plugins/ec2/__init__.py' > --- VMBuilder/plugins/ec2/__init__.py 2008-11-28 11:18:43 +0000 > +++ VMBuilder/plugins/ec2/__init__.py 2009-05-01 19:24:18 +0000 > @@ -36,6 +36,7 @@ > group.add_option('--ec2-secret-key', metavar='SECRET_ID', help='AWS secret access key.') > group.add_option('--ec2-kernel','--ec2-aki', metavar='AKI', help='EC2 AKI (kernel) to use.') > group.add_option('--ec2-ramdisk','--ec2-ari', metavar='ARI', help='EC2 ARI (ramdisk) to use.') > + group.add_option('--ec2-version',metavar='EC2_VER',help='Specifity the EC2 image version.') Please use spaces instead of tabs. Note: This is far from the only occurrence of this problem. > self.vm.register_setting_group(group) > > def preflight_check(self): > @@ -59,17 +60,45 @@ > > if not self.vm.ec2_kernel: > logging.debug('No ec2-aki choosen setting to default. Use --ec2-kernel to change this') > - if self.vm.arch == 'amd64': > - self.vm.ec2_kernel = 'aki-d314f0ba' > - else: > - self.vm.ec2_kernel = 'aki-af14f0c6' > + if self.vm.suite == 'hardy': > + if self.vm.arch == 'amd64': > + self.vm.ec2_kernel = 'aki-a27493cb' > + else: > + self.vm.ec2_kernel = 'aki-a17493c8' > + elif self.vm.suite == 'intrepid': > + if self.vm.arch == 'amd64': > + self.vm.ec2_kernel = 'aki-20c12649' > + else: > + self.vm.ec2_kernel = 'aki-20c12649' > + elif self.vm.suite == 'jaunty': > + if self.vm.arch == 'amd64': > + self.vm.ec2_kernel = 'aki-d653b4bf' > + else: > + self.vm.ec2_kernel = 'aki-c553b4ac' > + > + logging.info('%s - AKI to be used.' %(self.vm.ec2_kernel)) > + logging.info('WARNING! You might be using an outdated AKI. Please check xxx') aki/ari settings should be in the relevant suite modules of Ubuntu, not in the EC2 plugin. > if not self.vm.ec2_ramdisk: > logging.debug('No ec2-ari choosen setting to default. Use --ec2-ramdisk to change this.') > - if self.vm.arch == 'amd64': > - self.vm.ec2_ramdisk = 'ari-d014f0b9' > - else: > - self.vm.ec2_ramdisk = 'ari-ac14f0c5' > + if self.vm.suite == 'hardy': > + if self.vm.arch == 'amd64': > + self.vm.ec2_ramdisk = 'ari-b87493d1' > + else: > + self.vm.ec2_ramdisk = 'ari-ae7493c7' > + elif self.vm.suite == 'intrepid': > + if self.vm.arch == 'amd64': > + self.vm.ec2_ramdisk = 'ari-0ac12663' > + else: > + self.vm.ec2_ramdisk = 'ari-21c12648' > + elif self.vm.suite == 'jaunty': > + if self.vm.arch == 'amd64': > + self.vm.ec2_ramdisk = 'ari-d753b4be' > + else: > + self.vm.ec2_ramdisk = 'ari-c253b4ab' > + > + logging .info('%s - ARI to be used.'%(self.vm.ec2_ramdisk)) > + logging.info('WARNING! You might be using an outdated AKI. Please check xxx') "xxx"? > > if not self.vm.ec2_bucket: > raise VMBuilderUserError('When building for EC2 you must provide an S3 bucket to hold the AMI') > @@ -80,29 +109,28 @@ > if not self.vm.ec2_secret_key: > raise VMBuilderUserError('When building for EC2 you must provide your AWS secret access key.') > > - > - if not self.vm.addpkg: > - self.vm.addpkg = [] > - > - self.vm.addpkg += ['openssh-server'] > - self.vm.addpkg += ['ec2-init'] > - self.vm.addpkg += ['openssh-server'] > - self.vm.addpkg += ['ec2-modules'] > - self.vm.addpkg += ['server^'] > - self.vm.addpkg += ['standard^'] > - > - if not self.vm.ppa: > - self.vm.ppa = [] > - > - self.vm.ppa += ['ubuntu-ec2'] > + logging.info('Installing common software') > + self.install_common() > + if self.vm.suite == 'hardy': > + self.install_hardy() > + elif self.vm.suite == 'intrepid': > + self.install_intrepid() > + elif self.vm.suite == 'jaunty': > + self.install_jaunty() These look odd in preflight_check. Also, things that are specific to a specific suite does not belong here. Such things belong in the Ubuntu plugin in the respecitive suite's class. Again: spaces instead of tabs. We need to all do it the same way, or reviewing patches will be horrible. > > def post_install(self): > if not self.vm.ec2: > return > > logging.info("Running ec2 postinstall") > - self.install_from_template('/etc/event.d/xvc0', 'upstart') > - self.run_in_target('passwd', '-l', self.vm.user) > + logging.info("Running common post install") > + if self.vm.suite == 'hardy': > + self.postinstall_hardy() > + elif self.vm.suite == 'intrepid': > + self.postinstall_intrepid() > + elif self.vm.suite == 'jaunty': > + self.postinstall_jaunty() > + self.postinstall_common() It will look very confusing in the debug output that it says "Running common post install" and the next thing that happens in the post-install specific to the suite. This devaluates the debug log. Again: Spaces instead of tabs, and suite specific things go in their respective classes in the Ubuntu plugin. > > def deploy(self): > if not self.vm.ec2: > @@ -121,4 +149,112 @@ > > return True > > + def install_common(self): > + if not self.vm.ec2: > + return False > + > + if not self.vm.addpkg: > + self.vm.addpkg = [] > + > + logging.info('Installing common software.') > + self.vm.addpkg += ['openssh-server', > + 'ec2-init', > + 'standard^', > + 'ec2-ami-tools', > + 'update-motd', > + 'curl', > + 'screen', > + 'screen-profiles'] Do you remember how/when it was decided that screen-profiles should be installed by default? > + > + if not self.vm.ppa: > + self.vm.ppa = [] > + > + self.vm.ppa += ['ubuntu-on-ec2'] Won't this cause "apt-get update" et al on EC2 instances to complain about GPG keys? > + > + def install_hardy(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Installing software for hardy.') > + self.vm.addpkg += ['ruby', > + 'libc6-xen', > + 'libopenssl-ruby'] > + > + > + def postinstall_hardy(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Running post-install for hardy') > + # work around for libc6/xen bug in hardy. > + self.install_from_template('/etc/ld.so.conf.d/libc6-xen.conf', 'xen-ld-so-conf') > + self.run_in_target('apt-get', 'remove', '-y', 'libc6-i686') Replace with self.vm.addpkg += ['libc6-i686-'] > + > + self.install_from_template('/etc/update-motd.d/51_update-motd', '51_update-motd-hardy') > + self.install_from_template('/etc/event.d/xvc0', 'xvc0') > + self.run_in_target('update-rc.d', '-f', 'hwclockfirst.sh', 'remove') Why remove hwclockfirst.sh? > + > + def install_intrepid(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Installing software for intrepid') > + self.vm.addpkg += ['ec2-modules', > + 'ruby', > + 'libopenssl-ruby', > + 'server^', > + 'policykit', > + 'landscape-common', > + 'landscape-client'] Why ruby and libopenssl-ruby? Why policykit? Why landscape-client? > + > + def postinstall_intrepid(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Running post-install for intrepid') > + self.install_from_template('/etc/update-motd.d/51_update-motd', '51_update-motd-intrepid') > + self.install_from_template('/etc/default/landscape-client', 'landscape_client') > + > + def install_jaunty(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Installing software for jaunty') > + self.vm.addpkg += ['ec2-modules', > + 'ruby1.8', > + 'server^', > + 'libopenssl-ruby1.8', > + 'landscape-common', > + 'landscape-client'] Same here. Why ruby1.8, libopenssl-ruby1.8, and landscape-client? > + > + def postinstall_jaunty(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Running post-install for jaunty') > + self.install_from_template('/etc/update-motd.d/51_update-motd', '51_update-motd-intrepid') > + self.install_from_template('/etc/default/landscape-client', 'landscape_client') > + self.install_from_template('/etc/event.d/hvc0', 'hvc0') > + > + def postinstall_common(self): > + if not self.vm.ec2: > + return False > + > + logging.info('Running common post-install') > + self.install_from_template('/etc/ssh/sshd_config', 'sshd_config') > + self.run_in_target('chpasswd', '-e', stdin='ubuntu:!\n') Why do this here? VMBuilder should already be doing that. > + # this makes my skin crawl > + self.install_from_template('/etc/sudoers', 'sudoers') > + # this doesnt > + self.run_in_target('chmod', '755', '/etc/update-motd.d/51_update-motd') > + self.install_from_template('/etc/ec2_version', 'ec2_version', { 'version' : self.vm.ec2_version }) > + > + self.run_in_target('rm', '-f', '/etc/localtime') > + self.run_in_target('ln', '-s', '/usr/share/zoneinfo/UTC', '/etc/localtime') /etc/localtime should be a copy of /usr/share/zoneinfo/UTC, not a symlink to it. That's how we do it everywhere else. > + self.run_in_target('usermod', '-u', '135', 'ubuntu') How was 135 chosen? > + self.run_in_target('chown', '-R', 'ubuntu', '/home/ubuntu') According to usermod's documention, this happens automatically. Is that not the case? > + > + self.run_in_target('update-rc.d', '-f', 'hwclock.sh', 'remove') Why? > register_plugin(EC2) > > === added file 'VMBuilder/plugins/ec2/templates/51_update-motd-hardy.tmpl' > --- VMBuilder/plugins/ec2/templates/51_update-motd-hardy.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/51_update-motd-hardy.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,10 @@ > +#!/bin/sh > + > +echo "---------------------------------------------------------------------" > +echo "At the moment, only the core of the system is installed. To tune the " > +echo "system to your needs, you can choose to install one or more " > +echo "predefined collections of software by running the following " > +echo "command: " > +echo " " > +echo " sudo tasksel " > +echo "---------------------------------------------------------------------" > > === added file 'VMBuilder/plugins/ec2/templates/51_update-motd-intrepid.tmpl' > --- VMBuilder/plugins/ec2/templates/51_update-motd-intrepid.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/51_update-motd-intrepid.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,10 @@ > +#!/bin/sh > + > +echo "---------------------------------------------------------------------" > +echo "At the moment, only the core of the system is installed. To tune the " > +echo "system to your needs, you can choose to install one or more " > +echo "predefined collections of software by running the following " > +echo "command: " > +echo " " > +echo " sudo tasksel --section server " > +echo "---------------------------------------------------------------------" > > === added file 'VMBuilder/plugins/ec2/templates/51_update-motd-jaunty.tmpl' > --- VMBuilder/plugins/ec2/templates/51_update-motd-jaunty.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/51_update-motd-jaunty.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,10 @@ > +#!/bin/sh > + > +echo "---------------------------------------------------------------------" > +echo "At the moment, only the core of the system is installed. To tune the " > +echo "system to your needs, you can choose to install one or more " > +echo "predefined collections of software by running the following " > +echo "command: " > +echo " " > +echo " sudo tasksel --section server " > +echo "---------------------------------------------------------------------" > > === added file 'VMBuilder/plugins/ec2/templates/ec2_version.tmpl' > --- VMBuilder/plugins/ec2/templates/ec2_version.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/ec2_version.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,1 @@ > +$version > > === added file 'VMBuilder/plugins/ec2/templates/hvc0.tmpl' > --- VMBuilder/plugins/ec2/templates/hvc0.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/hvc0.tmpl 2009-03-26 17:13:31 +0000 > @@ -0,0 +1,16 @@ > +# hvc0 - getty > +# > +# This service maintains a getty on tty1 from the point the system is No, it doesn't :) > +# started until it is shut down again. > + > +start on stopped rc2 > +start on stopped rc3 > +start on stopped rc4 > +start on stopped rc5 > + > +stop on runlevel 0 > +stop on runlevel 1 > +stop on runlevel 6 > + > +respawn > +exec /sbin/getty 38400 hvc0 > > === added file 'VMBuilder/plugins/ec2/templates/landscape_client.tmpl' > --- VMBuilder/plugins/ec2/templates/landscape_client.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/landscape_client.tmpl 2009-04-07 12:51:57 +0000 > @@ -0,0 +1,1 @@ > +CLOUD=1 > > === added file 'VMBuilder/plugins/ec2/templates/sshd_config.tmpl' > --- VMBuilder/plugins/ec2/templates/sshd_config.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/sshd_config.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,77 @@ > +# Package generated configuration file > +# See the sshd(8) manpage for details > + > +# What ports, IPs and protocols we listen for > +Port 22 > +# Use these options to restrict which interfaces/protocols sshd will bind to > +#ListenAddress :: > +#ListenAddress 0.0.0.0 > +Protocol 2 > +# HostKeys for protocol version 2 > +HostKey /etc/ssh/ssh_host_rsa_key > +HostKey /etc/ssh/ssh_host_dsa_key > +#Privilege Separation is turned on for security > +UsePrivilegeSeparation yes > + > +# Lifetime and size of ephemeral version 1 server key > +KeyRegenerationInterval 3600 > +ServerKeyBits 768 > + > +# Logging > +SyslogFacility AUTH > +LogLevel INFO > + > +# Authentication: > +LoginGraceTime 120 > +PermitRootLogin yes > +StrictModes yes > + > +RSAAuthentication yes > +PubkeyAuthentication yes > +#AuthorizedKeysFile %h/.ssh/authorized_keys > + > +# Don't read the user's ~/.rhosts and ~/.shosts files > +IgnoreRhosts yes > +# For this to work you will also need host keys in /etc/ssh_known_hosts > +RhostsRSAAuthentication no > +# similar for protocol version 2 > +HostbasedAuthentication no > +# Uncomment if you don't trust ~/.ssh/known_hosts for RhostsRSAAuthentication > +#IgnoreUserKnownHosts yes > + > +# To enable empty passwords, change to yes (NOT RECOMMENDED) > +PermitEmptyPasswords no > + > +# Change to yes to enable challenge-response passwords (beware issues with > +# some PAM modules and threads) > +ChallengeResponseAuthentication no > + > +# Change to no to disable tunnelled clear text passwords > +PasswordAuthentication no > + > +# Kerberos options > +#KerberosAuthentication no > +#KerberosGetAFSToken no > +#KerberosOrLocalPasswd yes > +#KerberosTicketCleanup yes > + > +# GSSAPI options > +#GSSAPIAuthentication no > +#GSSAPICleanupCredentials yes > + > +X11Forwarding yes > +X11DisplayOffset 10 > +PrintMotd no > +PrintLastLog yes > +TCPKeepAlive yes > +#UseLogin no > + > +#MaxStartups 10:30:60 > +#Banner /etc/issue.net > + > +# Allow client to pass locale environment variables > +AcceptEnv LANG LC_* > + > +Subsystem sftp /usr/lib/openssh/sftp-server > + > +UsePAM yes How and why does this differ from the standard sshd_config? > > === added file 'VMBuilder/plugins/ec2/templates/sudoers.tmpl' > --- VMBuilder/plugins/ec2/templates/sudoers.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/sudoers.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,23 @@ > +# /etc/sudoers > +# > +# This file MUST be edited with the 'visudo' command as root. > +# > +# See the man page for details on how to write a sudoers file. > +# > + > +Defaults env_reset > + > +# Uncomment to allow members of group sudo to not need a password > +# %sudo ALL=NOPASSWD: ALL > + > +# Host alias specification > + > +# User alias specification > + > +# Cmnd alias specification > + > +# User privilege specification > +root ALL=(ALL) ALL > +ubuntu ALL=(ALL) NOPASSWD:ALL > + > +# Members of the admin group may gain root privileges Does anything add "%admin ALL=(ALL) ALL" later? If not, please remove that final comment. > > === removed file 'VMBuilder/plugins/ec2/templates/upstart.tmpl' > --- VMBuilder/plugins/ec2/templates/upstart.tmpl 2008-11-11 18:28:57 +0000 > +++ VMBuilder/plugins/ec2/templates/upstart.tmpl 1970-01-01 00:00:00 +0000 > @@ -1,16 +0,0 @@ > -# tty1 - getty > -# > -# This service maintains a getty on tty1 from the point the system is > -# started until it is shut down again. > - > -start on stopped rc2 > -start on stopped rc3 > -start on stopped rc4 > -start on stopped rc5 > - > -stop on runlevel 0 > -stop on runlevel 1 > -stop on runlevel 6 > - > -respawn > -exec /sbin/getty 38400 xvc0 > > === added file 'VMBuilder/plugins/ec2/templates/xen-ld-so-conf.tmpl' > --- VMBuilder/plugins/ec2/templates/xen-ld-so-conf.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/xen-ld-so-conf.tmpl 2009-03-24 16:33:47 +0000 > @@ -0,0 +1,1 @@ > +hwcap 0 nosegneg > > === added file 'VMBuilder/plugins/ec2/templates/xvc0.tmpl' > --- VMBuilder/plugins/ec2/templates/xvc0.tmpl 1970-01-01 00:00:00 +0000 > +++ VMBuilder/plugins/ec2/templates/xvc0.tmpl 2009-03-26 17:13:31 +0000 > @@ -0,0 +1,16 @@ > +# xvc0 - getty > +# > +# This service maintains a getty on tty1 from the point the system is > +# started until it is shut down again. > + > +start on stopped rc2 > +start on stopped rc3 > +start on stopped rc4 > +start on stopped rc5 > + > +stop on runlevel 0 > +stop on runlevel 1 > +stop on runlevel 6 > + > +respawn > +exec /sbin/getty 38400 xvc0 This is roughly identical to what's in the xvc0 template, isn't it? If so, please only provide one template. > > === modified file 'VMBuilder/plugins/firstscripts/__init__.py' > --- VMBuilder/plugins/firstscripts/__init__.py 2008-09-30 13:01:58 +0000 > +++ VMBuilder/plugins/firstscripts/__init__.py 2009-03-24 13:22:36 +0000 > @@ -41,11 +41,17 @@ > > if self.vm.firstboot: > logging.debug("Checking if firstboot script %s exists" % (self.vm.firstboot,)) > + if not(os.access(self.vm.firstboot, os.X_OK)): > + raise MBuilderUserError("The first-boot script is not executable") > + It's VMBuilderUserError, not MBuilderUserError. Also, it's seems like a bad plan to first test if it's executable or not and /then/ check if it exists at all. > if not(os.path.isfile(self.vm.firstboot)): > raise VMBuilderUserError('The path to the first-boot script is invalid: %s. Make sure you are providing a full path.' % self.vm.firstboot) > > if self.vm.firstlogin: > logging.debug("Checking if first login script %s exists" % (self.vm.firstlogin,)) > + if not(os.access(self.vm.firstlogin, os.X_OK)): > + raise VMBuilderUserError("The first-login script is not executable") > + > if not(os.path.isfile(self.vm.firstlogin)): > raise VMBuilderUserError('The path to the first-login script is invalid: %s. Make sure you are providing a full path.' % self.vm.firstlogin) Same as above. First check if it exists, /then/ check if it's executable. > === modified file 'VMBuilder/plugins/ubuntu/dapper.py' > --- VMBuilder/plugins/ubuntu/dapper.py 2008-12-16 15:29:50 +0000 > +++ VMBuilder/plugins/ubuntu/dapper.py 2009-03-24 13:30:11 +0000 > @@ -81,6 +81,9 @@ > logging.debug("Installing ssh keys") > self.install_authorized_keys() > > + logging.debug("Install xen kernel") > + self.install_xen_kernel() > + Can't this be done using the normal kernel installation code? > logging.debug("Installing extra packages") > self.install_extras() > > > === modified file 'VMBuilder/plugins/ubuntu/hardy.py' > --- VMBuilder/plugins/ubuntu/hardy.py 2008-11-06 15:36:18 +0000 > +++ VMBuilder/plugins/ubuntu/hardy.py 2009-03-24 15:28:46 +0000 > @@ -18,13 +18,30 @@ > # along with this program. If not, see . > # > import suite > +import logging > +import VMBuilder > +from VMBuilder.util import run_cmd > from VMBuilder.plugins.ubuntu.gutsy import Gutsy Please keep imports in alphabetical order. > > class Hardy(Gutsy): > virtio_net = True > > def xen_kernel_path(self): > - return '/boot/vmlinuz-2.6.24-19-xen' > + rcmd = run_cmd('chroot', self.destdir, 'dpkg', '-S', 'xen') > + temp = rcmd[0].split(": ") > + xen_kernel = temp[0].split("linux-image-") > + path = '/boot/vmlinuz-%s' %xen_kernel > + return path This is not acceptable. "dpkg -S xen" will return any and all files that have "xen" in their name somewhere. You can't rely on it being the kernel image. > > def xen_ramdisk_path(self): > - return '/boot/initrd.img-2.6.24-19-xen' > + rcmd = run_cmd('chroot', self.destdir, 'dpkg', '-S', 'xen') > + temp = rcmd[0].split(": ") > + xen_ramdisk = temp[0].split("linux-image-") > + path = '/boot/initrd.img-%s' %xen_ramdisk > + return path When you have two functions that need to be this similar, please don't just copy/paste. Create a common function that grabs the version number and use that. > + > + def install_xen_kernel(self): > + import VMBuilder.plugins.xen > + > + if isinstance(self.vm.hypervisor, VMBuilder.plugins.xen.Xen): > + self.run_in_target('apt-get', '-y', '--force-yes', 'install', 'linux-image-xen') > > === modified file 'VMBuilder/plugins/ubuntu/intrepid.py' > --- VMBuilder/plugins/ubuntu/intrepid.py 2009-02-17 14:39:16 +0000 > +++ VMBuilder/plugins/ubuntu/intrepid.py 2009-03-24 15:28:46 +0000 > @@ -35,3 +35,8 @@ > run_cmd('sed', '-ie', 's/^# kopt=root=\([^ ]*\)\(.*\)/# kopt=root=UUID=%s\\2/g' % bootdev.fs.uuid, '%s/boot/grub/menu.lst' % self.destdir) > run_cmd('sed', '-ie', 's/^# groot.*/# groot=%s/g' % bootdev.fs.uuid, '%s/boot/grub/menu.lst' % self.destdir) > run_cmd('sed', '-ie', '/^# kopt_2_6/ d', '%s/boot/grub/menu.lst' % self.destdir) > + def install_xen_kernel(self): > + import VMBuilder.plugins.xen > + > + if isinstance(self.vm.hypervisor, VMBuilder.plugins.xen.Xen): > + logging.info('Skipping Xen kernel installation.') I don't understand this check. If we're building a Xen guest, don't install the Xen kernel? Also, the function doesn't /do/ anything different based on the check, it just logs something? It looks rather pointless. > > === modified file 'VMBuilder/plugins/ubuntu/jaunty.py' > --- VMBuilder/plugins/ubuntu/jaunty.py 2008-12-04 16:27:22 +0000 > +++ VMBuilder/plugins/ubuntu/jaunty.py 2009-03-24 15:28:46 +0000 > @@ -31,3 +31,9 @@ > run_cmd('sed', '-ie', 's/^# kopt=root=\([^ ]*\)\(.*\)/# kopt=root=UUID=%s\\2/g' % bootdev.fs.uuid, '%s/boot/grub/menu.lst' % self.destdir) > run_cmd('sed', '-ie', 's/^# groot.*/# groot=%s/g' % bootdev.fs.uuid, '%s/boot/grub/menu.lst' % self.destdir) > run_cmd('sed', '-ie', '/^# kopt_2_6/ d', '%s/boot/grub/menu.lst' % self.destdir) > + > + def install_xen_kernel(self): > + import VMBuilder.plugins.xen > + > + if isinstance(self.vm.hypervisor, VMBuilder.plugins.xen.Xen): > + logging.info('Skipping Xen kernel installation.') Same comments as above. > > === modified file 'VMBuilder/plugins/ubuntu/templates/sources.list.tmpl' > --- VMBuilder/plugins/ubuntu/templates/sources.list.tmpl 2008-11-28 09:59:59 +0000 > +++ VMBuilder/plugins/ubuntu/templates/sources.list.tmpl 2009-03-24 13:15:03 +0000 > @@ -9,7 +9,7 @@ > > #if $ppa > #for $p in $ppa > -deb http://ppa.launchpad.net/$p/ubuntu $suite #slurp > +deb http://ppa.launchpad.net/$p/ppa/ubuntu $suite #slurp This is not acceptable. The original code was written when you couldn't have more than one PPA per user on Launchpad. Now we can. Don't just hack the code to keep doing what it used to do... Rather update it to fit the new world order. > #echo ' '.join($components) > > #end for > > === modified file 'debian/control' > --- debian/control 2008-12-16 13:39:21 +0000 > +++ debian/control 2009-03-24 13:17:11 +0000 > @@ -9,7 +9,7 @@ > > Package: python-vm-builder > Architecture: all > -Depends: kvm (>= 1:69) | qemu, debootstrap (>= 1.0.9), parted, kpartx, ubuntu-keyring, ${python:Depends}, python-cheetah, devscripts > +Depends: kvm (>= 1:69) | qemu, debootstrap (>= 1.0.9), parted, kpartx, ubuntu-keyring, ${python:Depends}, python-cheetah, devscripts, rsync > Recommends: python-libvirt > XB-Python-Version: ${python:Versions} > Description: VM builder > > === modified file 'debian/python-vm-builder-ec2.install' > --- debian/python-vm-builder-ec2.install 2008-10-20 22:09:39 +0000 > +++ debian/python-vm-builder-ec2.install 2009-03-24 14:03:02 +0000 > @@ -1,1 +1,1 @@ > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/ec2 > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/ec2 > > === modified file 'debian/python-vm-builder.install' > --- debian/python-vm-builder.install 2008-10-21 08:14:00 +0000 > +++ debian/python-vm-builder.install 2009-03-24 14:15:32 +0000 > @@ -1,12 +1,12 @@ > debian/tmp/etc > debian/tmp/usr/bin/vmbuilder > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/*.py > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/*.py > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/cli > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/firstscripts > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/kvm > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/libvirt > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/postinst > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/ubuntu > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/xen > -debian/tmp/usr/lib/python2.5/site-packages/VMBuilder/plugins/vmware > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/*.py > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/*.py > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/cli > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/firstscripts > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/kvm > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/libvirt > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/postinst > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/ubuntu > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/xen > +debian/tmp/usr/lib/python*/*-packages/VMBuilder/plugins/vmware I've split out the debian/ directory, so I'll handle this differently.