Merge lp:~gandelman-a/ubuntu/oneiric/cobbler/lp850880-850866 into lp:ubuntu/oneiric/cobbler

[Adam Gandelman]

This extends and adds some needed functionality:

* -u/--update can be used to download a newer ISO from the archive, import it into cobbler as a new distro. After its imported with a temp. name, sub-profiles of the outdated profile are reassigned to the updated version. The outdated distro is removed and the newer version renamed to match. This whole song and dance is necessary to preserve profiles and systems that are dependent on the root distro within the cobbler resource tree.

* both new imports and update checks are done first against a releases -update pocket and, failing that (404), falls back to the release pocket. This should allow new imports to import the most recent mini.isos, and later runs to check against whats been published since last run. A trivial change to orchestra-import-isos can make use of this and --update to keep cached ISOs up to date between cron'd runs.

* -v existed previously, but the corresponding VERBOSITY variable was never used. Added debug() which is useful for debugging info during some of the more involved stuff

[Scott Moser (smoser)]

Some comments:
 * I don't like the use of globals in update_available and get_isos. I dont have a simple solution for you, and i wouldnt nack it completely based on that, but globals (or lexical scoped variables) are just confusing. If you have to use them, please use upper case names to indicate that they're global.
 * use more quotes:
   Things like '[ -n $md5sum_local ]' should really be '[ -n "$md5sum_local" ]', as the variable md5sum_local could have either shell wildcards or spaces in it which change behavior unexpectedly.
 * there are some white space indentation rather than tab (search for '[ ][ ][ ]' and you'll see).
   Also, you can set vi to read the modelines (the last line of the file) with 'set modeline modelines=3'

Also, I'd like a general "just do the right thing" flag.
So that I could do something like:
  cobbler-ubuntu-import --do-what-i-mean natty-i386 natty-amd64 oneiric-amd64 oneiric-i386

And it would import or update if necessary, and exit success unless an operation failed. It would even make sense to me that that should be the default behavior.

The last comment is that it would be nice if 'cobbler import' had a '--replace' flag. We might want to do that and submit it upstream that did your rename dance inside. It would be able to do more appropriate locking and such to avoid two things attempting the call at the same time.

Thanks for your work, I really hope not to sound negative.

review: Needs Fixing

[lp:~gandelman-a/ubuntu/oneiric/cobbler/lp850880-850866]

53. By Adam Gandelman on 2011-10-11

Better quoting of shell variables

54. By Adam Gandelman on 2011-10-11

Also remove new, temp. profile after distro has been renamed

55. By Adam Gandelman on 2011-10-11

Capitalize global variables, lower case locals

56. By Adam Gandelman on 2011-10-11

Fix up some whitespace

[Adam Gandelman (gandelman-a)]

Hi Scott-

Just pushed some changes which address your 3 points. I agree global variables suck, but to do anything to properly get rid of them at this point would be a major rewrite of the script itself. Perhaps this is something we should address next cycle, maybe thinking about a better way to go about managing a local ISO cache.

I'd prefer to get these changes worked out and tested before changing the default behavior of the script, but I agree that it should probably do all this by default. And +1 to trying to get the same support in cobbler upstream. In the meantime, the --check and --update stuff should be enough.

[Andres Rodriguez (andreserl)]

From conversation with Scott, this branch needs fixing:

smoser> * debug() should go to stderr, not stdout

I'll look into fixing it to getting it uploaded.

review: Needs Fixing

[lp:~gandelman-a/ubuntu/oneiric/cobbler/lp850880-850866]

57. By Adam Gandelman on 2011-10-12

debug(): Redirect to stderr instead of stdout

[James Page (james-page)]

Merged in precise - please can this branch be marked as merged unless we are going to use this branch for the SRU.

---------------

cobbler (2.1.0+git20110602-0ubuntu27) precise; urgency=low

  [ Adam Gandelman ]
  * debian/cobbler-ubuntu-import:
      - Check update pockets for releases on download and update check.
        (LP: #850880 )
      - Allow '-u' to upgrade existing profiles to a newer version of an ISO.
        (LP: #850886)
      - '-v' to allow debug msgs.

  [ Andres Rodriguez ]
  * debian/patches:
    - 42_fix_repomirror_create_sync.patch: Updated to correctly create the
      mirror. (LP: #872926)
  * debian/cobbler-web.postrm: Remove symlinks that were created on
    postinst. (LP: #872892)
 -- Andres Rodriguez <email address hidden> Mon, 24 Oct 2011 19:29:26 -0400

=== modified file 'debian/changelog'
--- debian/changelog	2011-09-29 23:58:08 +0000
+++ debian/changelog	2011-10-12 20:47:41 +0000
@@ -1,3 +1,14 @@
+cobbler (2.1.0+git20110602-0ubuntu27) UNRELEASED; urgency=low
+
+  * debian/cobbler-ubuntu-import:
+      - Check update pockets for releases on download and update check.
+        (LP: #850880 )
+      - Allow '-u' to upgrade existing profiles to a newer version of an ISO.
+        (LP: #850886)
+      - '-v' to allow debug msgs.
+
+ -- Adam Gandelman <[e-mail]>  Mon, 10 Oct 2011 18:44:00 -0700
+
 cobbler (2.1.0+git20110602-0ubuntu26) oneiric; urgency=low
 
   * debian/cobbler-ubuntu-import: Add --check-update and --remove
 
=== modified file 'debian/cobbler-ubuntu-import'
--- debian/cobbler-ubuntu-import	2011-09-16 05:10:52 +0000
+++ debian/cobbler-ubuntu-import	2011-10-12 20:47:41 +0000
@@ -10,6 +10,7 @@
 errorp() { printf "$@" 1>&2; }
 fail() { [ $# -eq 0 ] || error "$@"; exit 1; }
 failp() { [ $# -eq 0 ] || errorp "$@"; exit 1; }
+debug() { [ $VERBOSITY -gt 0 ] && echo "[DEBUG] $@" 1>&2; }
 
 Usage() {
 	cat <<EOF
@@ -26,6 +27,8 @@
                           mirror.  exits 0 if an update is needed or iso does
                           not exist in $ISO_DIR
       -r | --remove       remove cobbler profile/distro and cached ISO
+      -u | --update	  update iso, import, replace outdated distro and profile
+      -v | --verbose	  increase verbosity
    Note, arch can be i386, x86_64, or amd64.  However,
    the registered distro and profile will use 'x86_64' rather than 'amd64'.
    This is to keep consistent with cobbler naming.
@@ -37,7 +40,7 @@
 
 bad_Usage() { Usage 1>&2; [ $# -eq 0 ] || error "$@"; exit 1; }
 cleanup() {
-   [ -z "${TEMP_D}" -o ! -d "${TEMP_D}" ] || rm -Rf "${TEMP_D}"
+	[ -z "${TEMP_D}" -o ! -d "${TEMP_D}" ] || rm -Rf "${TEMP_D}"
 }
 
 loop_mount() {
@@ -49,10 +52,91 @@
 		chown root:disk /dev/loop$loops
 	fi
 	# Do the loop mount
+	debug "Creating loopback mount from $1 at $2"
 	mount -o loop "$1" "$2"
 }
 
-short_opts="Dhm:o:vcr"
+get_iso() {
+	# first check release's update pocket for an iso.
+	# grab from release pocket if that does not exist.
+	if wget -O "$TEMP_D/$ISO" "$U_UPDATE"; then
+		debug "Downloaded ISO from updates pocket: $U_UPDATE"
+	elif wget -O "$TEMP_D/$ISO" "$U"; then
+		debug "Updating from release pocket: $U"
+	else
+		fail "failed download for $REL-$ARCH"
+	fi
+	{ $delete || mv "$TEMP_D/$ISO" "$ISO_DIR/$ISO"; } ||
+		fail "failed to move iso $ISO_DIR/$ISO"
+}
+
+import_iso() {
+	# import iso into cobbler as $DISTRO
+	local mounted
+	local distro="$1"
+	if cobbler distro list | grep -qs " $distro"; then
+		fail "distro '$distro' already exists. cannot import."
+	fi
+	{ [ -f "$TEMP_D/$ISO" ] || ln -sf "$ISO_DIR/$ISO" "$TEMP_D/$ISO"; } ||
+		fail "failed to create symlink to existing $ISO_DIR/$ISO"
+	mounted=0 && loop_mount "$TEMP_D/$ISO" "$TEMP_D/mnt" && mounted=1 &&
+		cobbler import --name="$distro" --path="$TEMP_D/mnt" \
+			--breed=ubuntu --os-version="$REL" --arch="$ARCH" &&
+		umount "${TEMP_D}/mnt" && mounted=0 || {
+				[ $mounted -eq 0 ] || umount "$TEMP_D/mnt";
+				fail "failed to import $REL-$ARCH";
+			}
+	echo "imported $distro"
+}
+
+reassign_distro() {
+	# swap out backing distro of profile with a newer, imported
+	# distro. after, remove the original and rename the new distro
+	# to match the previous distro name associated with profile
+	local new_distro="$1"
+	local profile="$2"
+	debug "Reassigning $profile to $new_distro"
+	if ! cobbler profile list | grep -qs " $profile"; then
+		fail "cannot reassign non-existent profile '$profile'"
+	fi
+	if ! cobbler distro list | grep -qs " $profile"; then
+		fail "distro matching profile '$profile' does not exist"
+	fi
+	debug "Renaming old profile '$profile' to 'last-$profile'"
+	cobbler distro rename --name="$profile" --newname="last-$profile" ||
+		fail "could not rename distro '$profile' to 'last-$profile'"
+	debug "Assigning profile $profile to distro $new_distro"
+	cobbler profile edit --name="$profile" --distro="$new_distro" ||
+		fail "could not assign profile '$profile' to distro '$new_distro'"
+	debug "Removing distro last-$profile"
+	cobbler distro remove --name="last-$profile" ||
+		fail "could not remove stale distro 'last-$profile'"
+	debug "Renaming distro $new_distro to $profile"
+	cobbler distro rename --name="$new_distro" --newname="$profile" ||
+		fail "could not rename distro '$new_distro' to '$profile'"
+	debug "Cleaning up temporary profile '$new_distro'"
+	cobbler profile remove --name="$new_distro" ||
+		fail "could not remove temp profile '$new_distro'"
+}
+
+function update_available() {
+	# check MD5SUM first from updates pocket if it exists, then release pocket.
+	# compare netboot mini iso with what is local.
+	local md5sum_local
+	local md5sum_remote
+	[ ! -f "$ISO_DIR/$ISO" ] && return 0
+	md5sum_local=$(md5sum "$ISO_DIR/$ISO") && md5sum_local=${md5sum_local%% *} &&
+		[ -n "$md5sum_local" ] || fail "failed to checksum $ISO_DIR/$ISO: $md5sum_local"
+	debug "local md5sum: $md5sum_local ($ISO_DIR/$ISO)"
+	wget -qO "$TEMP_D/MD5SUMS" "$MD5_UPDATE" || wget -qO "$TEMP_D/MD5SUMS" "$MD5" ||
+		fail "failed to download MD5SUMS for $REL-$ARCH."
+	md5sum_remote=$(awk '$2 == "./netboot/mini.iso" { print $1 }' $TEMP_D/MD5SUMS) &&
+		[ -n "$md5sum_remote" ] || fail "failed to parse checksum from $TEMP_D/MD5SUMS: $md5sum_remote"
+	debug "remote md5sum: $md5sum_remote"
+	[ "$md5sum_local" != "$md5sum_remote" ]
+}
+
+short_opts="Dhm:o:vcru"
 long_opts="delete-iso,help,mirror:,verbose,update-check,remove"
 getopt_out=$(getopt --name "${0##*/}" \
 	--options "${short_opts}" --long "${long_opts}" -- "$@") &&
@@ -63,6 +147,7 @@
 check_update=false
 remove=false
 mirror=${DEFAULT_MIRROR}
+do_update=false
 
 while [ $# -ne 0 ]; do
 	cur=${1}; next=${2};
@@ -73,6 +158,7 @@
 		-v|--verbose) VERBOSITY=$((${VERBOSITY}+1));;
 		-c|--update-check) check_update=true;;
 		-r|--remove) remove=true;;
+		-u|--update) do_update=true;;
 		--) shift; break;;
 	esac
 	shift;
@@ -82,62 +168,51 @@
 [ $# -ne 0 ] || bad_Usage "must provide arguments"
 
 TEMP_D=$(mktemp -d "${TMPDIR:-/tmp}/${0##*/}.XXXXXX") ||
-   fail "failed to make tempdir"
+	fail "failed to make tempdir"
 trap cleanup EXIT
 
 # program starts here
 if $delete; then
-	{ [ -d "$ISO_DIR" ] || mkdir -p "$ISO_DIR"; }  || fail "failed to create $ISO_DIR"
+	{ [ -d "$ISO_DIR" ] || mkdir -p "$ISO_DIR"; } || fail "failed to create $ISO_DIR"
 fi
 mkdir "$TEMP_D/mnt" || fail "failed to make tempdir/mnt"
 
 for tok in "$@"; do
-	rel=${tok%-*}; arch=${tok#*-}
-	[ "$arch" = "amd64" ] && 
+	REL=${tok%-*}; ARCH=${tok#*-}
+	[ "$ARCH" = "amd64" ] && 
 		{ error "Warning: using x86_64 for arch rather than amd64"; arch="x86_64"; }
-	ubuntu_arch=$arch; [ "$arch" = "x86_64" ] && ubuntu_arch=amd64
-	iso=$rel-$arch-mini.iso
-	u=$mirror/dists/$rel/main/installer-$ubuntu_arch/current/images/netboot/mini.iso
-	md5=$mirror/dists/$rel/main/installer-$ubuntu_arch/current/images/MD5SUMS
+	ubuntu_arch=$ARCH; [ "$ARCH" = "x86_64" ] && ubuntu_arch=amd64
+	ISO=$REL-$ARCH-mini.iso
+	U=$mirror/dists/$REL/main/installer-$ubuntu_arch/current/images/netboot/mini.iso
+	MD5=$mirror/dists/$REL/main/installer-$ubuntu_arch/current/images/MD5SUMS
+	U_UPDATE=$mirror/dists/$REL-updates/main/installer-$ubuntu_arch/current/images/netboot/mini.iso
+	MD5_UPDATE=$mirror/dists/$REL-updates/main/installer-$ubuntu_arch/current/images/MD5SUMS
 	if $remove; then
-		if [ -f "$ISO_DIR/$iso" ]; then
-			rm -rf "$ISO_DIR/$iso" ||
-				fail "Could not delete $ISO_DIR/$iso"
+		if [ -f "$ISO_DIR/$ISO" ]; then
+			debug "Removing $ISO_DIR/$ISO"
+			rm -rf "$ISO_DIR/$ISO" ||
+				fail "Could not delete $ISO_DIR/$ISO"
 		fi
-		if cobbler distro list | grep -qs " $rel-$arch"; then
-			cobbler distro remove --name=$rel-$arch ||
-				fail "Could not remove cobbler profile for $rel-$arch"
+		if cobbler distro list | grep -qs " $REL-$ARCH"; then
+			debug "Removing cobbler distro '$REL-$ARCH'"
+			cobbler distro remove --name="$REL-$ARCH" ||
+				fail "Could not remove cobbler profile for $REL-$ARCH"
 		fi
 		exit 0
 	fi
 	if $check_update; then
-		# compare md5 of the local iso to whats on the mirror.
-		[ ! -f "$ISO_DIR/$iso" ] && exit 0
-		md5=$mirror/dists/$rel/main/installer-$ubuntu_arch/current/images/MD5SUMS
-		md5sum_local=$(md5sum "$ISO_DIR/$iso") && md5sum_local=${md5sum_local% *} \
-			&& [ -n $md5sum_local ] || fail "failed to checksum $ISO_DIR/$iso"
-		wget -qO $TEMP_D/MD5SUMS $md5 || fail "failed to download MD5SUMS for $rel-$arch: $u"
-		md5sum_remote=$(awk '$2 == "./netboot/mini.iso" { print $1 }' $TEMP_D/MD5SUMS) &&
-			[ -n $md5sum_remote ] || fail "failed to parse checksum from $TEMP_D/MD5SUM"
-		[ $md5sum_local != $md5sum_remote ] && exit 0
-		error "$ISO_DIR/$iso up to date." && exit 3
-	fi
-	if [ ! -f "$ISO_DIR/$iso" ]; then
-		wget -O "$TEMP_D/$iso" "$u" && [ -s "$TEMP_D/$iso" ] ||
-			fail "failed download for $rel-$arch: $u"
-		{ $delete || mv "$TEMP_D/$iso" "$ISO_DIR/$iso"; } ||
-			fail "failed to write to move iso to $ISO_DIR/$iso"
-	fi
-	{ [ -f $TEMP_D/$iso ] || ln -sf "$ISO_DIR/$iso" "$TEMP_D/$iso"; } ||
-		fail "failed to create symlink to existing $ISO_DIR/$iso"
-	mounted=0 && loop_mount "$TEMP_D/$iso" "$TEMP_D/mnt" && mounted=1 &&
-		cobbler import --name=$rel-$arch --path="$TEMP_D/mnt" \
-			--breed=ubuntu --os-version=$rel --arch=$arch &&
-		umount "${TEMP_D}/mnt" && mounted=0 || {
-				[ $mounted -eq 0 ] || umount "$TEMP_D/mnt";
-				fail "failed to import $rel-$arch";
-			}
-	echo "imported $rel-$arch"
+		update_available && echo "Update for $REL-$ARCH available" && exit 0
+		echo "$ISO_DIR/$ISO up to date." && exit 3
+	fi
+	if $do_update; then
+		update_available || fail "No update available"
+		# get iso from either pocket and import it with a tmp name.
+		get_iso && import_iso "tmp-$REL-$ARCH"
+		reassign_distro "tmp-$REL-$ARCH" "$REL-$ARCH"
+		exit 0
+	fi
+	[[ ! -f "$ISO_DIR/$ISO" ]] && get_iso
+	import_iso "$REL-$ARCH" && echo "imported $REL-$ARCH"
 done
 
 # vi: ts=4 noexpandtab