launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/vdenv-known-hosts into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/vdenv-known-hosts/+merge/97163

Several small changes to vdenv:

1. Don't write ssh fingerprint for virtual zimmer host to known_hosts on first login.

Every time you set up a new virtual zimmer host, it gets registered in your .ssh/known_hosts, and when you later log in to (or create) a new one, ssh gives you a loud warning or error about the host's key having changed.

By not writing the key to your known_host on initial login, we can avoid at least some of the cases where you'd otherwise get this.  You may still be prompted for whether to trust the host, if you log in with host checking enabled; you may want to edit your ssh config to disable that for this host.  You can crib off the “-o” options in the script's ssh command line.

2. Don't check zimmer host's IP on initial ssh login.

This too may help prevent “HOST IDENTIFICATION HAS CHANGED!” errors from ssh, after which you'd find that your ssh key was not set up for passwordless login to the zimmer host.

3. Unconditionally install missing required packages.

Just “apt-get install” the required packages instead of first checking them all with dpkg-query.  Yes, this will require you to type your sudo password a bit more often.  We can probably fix that later.  But it skips some very costly logic whose value I doubt: the packages were installed only if “dpkg-query --show $pkg” returned a non-zero code for at least one of the required packages.

As far as I can make out, “dpkg-query --show $pkg” returns zero even if $pkg is not installed.  It does return non-zero if $pkg is unknown — but what's the point of that?

4. Don't refresh the apt indexes every time.

Keeping your apt info up-to-date with “apt-get update” is your own problem.  If your downloads are slow and fragile, you don't want to keep updating these indexes on every run.  You might get an error just downloading fresh indexes that have been generated *because* the script is slow!

5. Fixed a typo in an error message, that used to produce nonsense output and now triggers a meta-error.

Clearly, a variable “$dest” accidentally got merged into its next word: “first.”  So you'd get “$destfirst” in the error string this happened in, which was normally not defined so in odev you'd just get a sentence with too much whitespace and not enough text.  In the vdenv you'd get an error while trying to print the error.  Also, clearly, the bogus variable name had been skipped in a renaming of “$dest.”  May even have been my fault.

Jeroen
-- 
https://code.launchpad.net/~jtv/maas/vdenv-known-hosts/+merge/97163
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/vdenv-known-hosts into lp:maas.

=== modified file 'vdenv/bin/authorize-ssh'
--- vdenv/bin/authorize-ssh	2012-03-09 20:44:46 +0000
+++ vdenv/bin/authorize-ssh	2012-03-13 09:31:17 +0000
@@ -40,7 +40,8 @@
     remotecmd="ssh-import-id $keyowner"
 fi
 
-while ! cat $inputfiles | ssh $cobblerlogin -o StrictHostKeyChecking=no $remotecmd
+while ! cat $inputfiles |
+    ssh $cobblerlogin -o CheckHostIP=no -o StrictHostKeyChecking=no $remotecmd
 do
     sleep 5
 done

=== modified file 'vdenv/bin/system-setup'
--- vdenv/bin/system-setup	2012-03-09 20:44:46 +0000
+++ vdenv/bin/system-setup	2012-03-13 09:31:17 +0000
@@ -18,16 +18,7 @@
 pkgs="$pkgs python-cheetah" # for setup.py
 pkgs="$pkgs qemu-utils qemu-kvm" # needed generally
 
-new_pkgs=""
-for pkg in ${pkgs}; do
-    dpkg-query --show "$pkg" >/dev/null ||
-        new_pkgs="${new_pkgs:+${new_pkgs} }${pkg}"
-done
-
-if [ -n "$new_pkgs" ]; then
-    sudo apt-get update -qq || /bin/true
-    sudo apt-get install -y $pkgs </dev/null
-fi
+sudo apt-get install -y $pkgs </dev/null
 
 new_groups=""
 for group in libvirtd kvm; do

=== modified file 'vdenv/zimmer-build/build'
--- vdenv/zimmer-build/build	2012-03-09 20:44:46 +0000
+++ vdenv/zimmer-build/build	2012-03-13 09:31:17 +0000
@@ -166,7 +166,7 @@
             o_zimg="${zimg}"
             zimg=${save_d}/$(basename "$o_zimg" ".img").zimg
             [ -f "$zimg" ] &&
-                fail "please delete $destfirst or use --zimg|--img"
+                fail "please delete $zimg first or use --zimg|--img"
             download "$o_zimg" "$zimg"
             ;;
         file://*)