[PATCH 1/2 livemedia-creator atomic] Add --make-pxe-live target and --ostree flag for Atomic Host.
Radek Vykydal
rvykydal at redhat.com
Mon Jan 5 14:01:52 UTC 2015
On 12/18/2014 09:05 PM, Brian C. Lane wrote:
> On Mon, Dec 15, 2014 at 04:49:06PM +0100, Radek Vykydal wrote:
>> Generate live squashfs and initrd for pxe live install.
>> Also generate pxe config template.
>>
>> To be used with virt installation with /boot on separate partition (the only
>> way supported by Anaconda currently). Content of boot partition is added to
>> live root fs so that ostree can find deployment by boot configuration.
>>
>> --ostree flag ensures using deployment root instead of physical root of
>> installed disk image where needed.
>> ---
>> docs/rhel-atomic-pxe-live.ks | 39 +++++++
>> src/sbin/livemedia-creator | 238 ++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 274 insertions(+), 3 deletions(-)
>> create mode 100644 docs/rhel-atomic-pxe-live.ks
> Thanks for working on this, I have a few comments inline.
Thanks for the review.
> We should add a section on using this to the README.livemedia-creator
> file, and add the new options to the livemedia-creator.1 manpage
>
>
Okay, I'll come with patches.
>> diff --git a/docs/rhel-atomic-pxe-live.ks b/docs/rhel-atomic-pxe-live.ks
>> new file mode 100644
>> index 0000000..324dad2
>> --- /dev/null
>> +++ b/docs/rhel-atomic-pxe-live.ks
>> @@ -0,0 +1,39 @@
>> +# Kickstart to be used to genrate pxe to live atomic image using --pxe-to-live and --ostree options
>> +lang en_US.UTF-8
>> +keyboard us
>> +timezone America/New_York
>> +clearpart --all --initlabel
>> +network --bootproto=dhcp --device=link --activate
>> +rootpw --plaintext atomic
>> +
>> +# We are only able to install atomic with separate /boot partition currently
>> +part / --fstype="ext4" --size=6000
>> +part /boot --size=500 --fstype="ext4"
>> +
>> +shutdown
>> +
>> +services --disabled=cloud-init,cloud-init-local,cloud-final,cloud-config,docker-storage-setup
>> +
>> +# Regenerating initramfs requires dracut-network package in repo, relabeling rootfs requires policycoreutils
>> +#ostreesetup --osname=rhel-atomic-host --remote=rhel-atomic-host --url=http://10.34.102.90:8000/repo --ref=rhel-atomic-host/7.0-buildmaster/x86_64/standard --nogpg
>> +ostreesetup --osname=rhel-atomic-host --remote=rhel-atomic-host --url=http://10.34.102.90:8000/repo --ref=rhel-atomic-host/7.0-buildmaster/x86_64/live-selinux --nogpg
> The IP should point to a working repo if available, be fedora specific
> for master and rhel specific for the rhel branch. Or use some obvious
> placeholder for the user to set themselves.
I think I'll go with a placeholder for now.
> [SNIP]
>
>> +def find_ostree_root(phys_root):
>> + """
>> + Find root of ostree deployment
>> +
>> + :param str phys_root: Path to physical root
>> + :returns: Relative path of ostree deployment root
>> + :rtype: str
>> + """
>> + ostree_root = ""
>> + ostree_sysroots = glob.glob(joinpaths(phys_root, "ostree/boot.0/*/*/0"))
>> + if ostree_sysroots:
>> + if len(ostree_sysroots) > 1:
>> + log.error("Too many deployment roots found: %s" % ostree_sysroots)
>> + ostree_root = os.path.relpath(ostree_sysroots[0], phys_root)
>> + return ostree_root
> If it is an error we should raise and exit, if not then call it a
> warning and log which root it is using. Also, use the log.warning(string,
> args) form to make pylint happy.
>
It should be an error I'll raise and exit.
>> +def make_live_rootfs(mount_dir, image_path, size=2, sys_root=""):
>> + """
>> + Make rootfs image from a directory
>> +
>> + :param str mount_dir: Root directory
>> + :param str image_path: Path of output image file
>> + :param int size: Size of the image, if None computed automatically
>> + :param str sys_root: path to system (deployment) root relative to physical root
>> + """
>> + if size:
>> + fssize = size * (1024*1024*1024) # 2GB sparse file compresses down to nothin'
>> + else:
>> + fssize = None # Let mkext4img figure out the needed size
>> +
>> + mkext4img(mount_dir, image_path, label="LiveOS", size=fssize)
>> + # Reset selinux context on new rootfs
>> + with LoopDev(image_path) as loopdev:
>> + with Mount(loopdev) as mnt:
>> + cmd = [ "setfiles", "-e", "/proc", "-e", "/sys", "-e", "/dev", "-e", "/install",
>> + "/etc/selinux/targeted/contexts/files/file_contexts", "/"]
>> + root = joinpaths(mnt, sys_root.lstrip("/"))
>> + runcmd(cmd, root=root)
> This is really similar to RuntimeBuilder.create_runtime, instead of
> duplicating code I'd rather see a new utility method made that can be
> shared between create_runtime and where you're calling this in
> make_live_images. I'd leave the squashfs bit in create_runtime and move
> everything above that to src/pylorax/imgutils.py
>
Sure (the patch was a bit raw yet, I wanted to know if the approach
is acceptable at all).
>> +def rebuild_initrds_for_live(opts, sys_root_dir, results_dir):
>> + """
>> + Rebuild intrds for pxe live image (root=live:http://)
>> +
>> + :param opts: options passed to livemedia-creator
>> + :type opts: argparse options
>> + :param str sys_root_dir: Path to root of the system
>> + :param str results_dir: Path of directory for storing results
>> + """
> I wish we could keep this kind of thing out of lmc, but it is different
> enough from the normal initrd recreation in pylorax that I guess it
> can't be avoided.
Yeah, consolidating with content of rebuild_initrds seemed not to be
doable in
any sane way to me.
>
> [SNIP]
>
>> + for kernel in kernels:
>> +
> No need for the blank line ^^
Sure.
>> +def create_pxe_config(images_dir, live_image_name, add_args = None):
>> + """
>> + Create template for pxe to live configuration
>> +
>> + :param str images_dir: Path of directory with images to be used
>> + :param str live_image_name: Name of live rootfs image file
>> + :param list add_args: Arguments to be added to initrd= pxe config
>> + """
>> +
>> + add_args = add_args or []
>> +
>> + kernels = [kernel for kernel in findkernels(images_dir, kdir="")
>> + if hasattr(kernel, "initrd")]
>> + if not kernels:
>> + return
>> +
>> + kernel = kernels[0]
>> +
>> + add_args_str = " ".join(add_args)
>> +
>> + buf = """# PXE configuration template generated by livemedia-creator
>> +kernel <PXE_DIR>%s
>> +append initrd=<PXE_DIR>%s root=live:<URL>/%s %s
>> +""" % (kernel.path, kernel.initrd.path, live_image_name, add_args_str)
>> + with open (joinpaths(images_dir, "PXE_CONFIG"), "w") as f:
>> + f.write(buf)
> This should be handled using Mako templates, like I did with the
> appliance template in make_appliance with libvirt.tmpl. That way users
> can override the content without having to change the source. I'd put
> it into share/pxe-live/pxe.tmpl
I'll look at it.
>>
>> def make_livecd(opts, mount_dir, work_dir):
>> """
>> @@ -588,6 +715,34 @@ def make_livecd(opts, mount_dir, work_dir):
>>
>> return work_dir
>>
>> +def mount_boot_part_over_root(img_mount):
>> + """
>> + Mount boot partition to /boot of root fs mounted in img_mount
>> +
>> + Used for OSTree so it finds deployment configurations on live rootfs
>> +
>> + param img_mount: object with mounted disk image root partition
>> + type img_mount: imgutils.PartitionMount
>> + """
>> + root_dir = img_mount.mount_dir
>> + is_boot_part = lambda dir: os.path.exists(dir+"/loader.0")
>> + tmp_mount_dir = tempfile.mkdtemp()
>> + sys_root = find_ostree_root(root_dir)
>> + sys_root_bootdir = None
> Should be sysroot_boot_dir
>
Yup.
>> + for dev, size in img_mount.loop_devices:
> Should use _size to make pylint happy about the unused variable.
Okay.
>> + try:
>> + mount("/dev/mapper/"+dev, mnt=tmp_mount_dir)
>> + if is_boot_part(tmp_mount_dir):
>> + umount(tmp_mount_dir)
>> + sysroot_boot_dir = joinpaths(joinpaths(root_dir, sys_root), "boot")
>> + mount("/dev/mapper/"+dev, mnt=sysroot_boot_dir)
>> + break
>> + else:
>> + umount(tmp_mount_dir)
>> + except subprocess.CalledProcessError as e:
>> + log.debug("Looking for boot partition error: %s", e)
> Failure should probably raise an error, shouldn't it?
I followed non-fatality of the error from PartitionMount._enter
assuming there is a reason for it but not really thinking about
it much.
>
>> + remove(tmp_mount_dir)
>> + return sysroot_boot_dir
> It is possible that /boot may already exist on / and not be a separate
> partition. This should be checked for either here, or before calling it
> (maybe by looking at the kickstart for a /boot mountpoint)
One thing here is that in ostree/atomic installs there is /boot on root
partition but it contains data (initrd, kernel) irrelevant for
ostree/atomic.
But if we exclude root partition as proposed just below, finding (ostree
specific) boot partition should be enough to use it and checking kickstart
is not necessary (also I imagine Atomic having separate boot as autopart
default where we wouldn't find /boot in ks (well this imaginary case may
hit some ks checks in lmc))
> I think it should skip the already mounted / partition to prevent any
> potential issue with double mounting/unmounting it.
Good, catch, I'll add the check.
> [SNIP]
>
>> def setup_logging(opts):
>> """
>> @@ -892,6 +1093,8 @@ def main():
>> help="Build an ami image")
>> action.add_argument("--make-tar", action="store_true",
>> help="Build a tar of the root filesystem")
>> + action.add_argument("--make-pxe-live", action="store_true",
>> + help="Build live in squashfs and initrd for pxe boot")
> "Build a live pxe boot squashfs image"
Will apply.
>
>>
>> parser.add_argument("--iso", type=os.path.abspath,
>> help="Anaconda installation .iso path to use for virt-install")
>> @@ -914,6 +1117,9 @@ def main():
>> help="location of iso directory tree with initrd.img "
>> "and vmlinuz. Used to run virt-install with a "
>> "newer initrd than the iso.")
>> + parser.add_argument("--ostree", action="store_true",
>> + help="Ostree (Atomic) installation")
>> +
> Currently ostree is a variation on pxe-live, does it make sense to have
> it alter other --make-* targets? Or would a --make-ostree-live switch
> make more sense (one that just turns on make-pxe-live and ostree)?
I think --make-ostree-live is really better. I don't expect --ostree to
be combined
with other --make-* targets.
>
>>
>> parser.add_argument("--logfile", default="./livemedia.log",
>> type=os.path.abspath,
>> @@ -1165,6 +1371,32 @@ def main():
>> make_appliance(opts.disk_image or disk_img, opts.app_name,
>> opts.app_template, opts.app_file, networks, opts.ram,
>> opts.vcpus, opts.arch, opts.title, opts.project, opts.releasever)
>> + elif opts.make_pxe_live:
>> +
> No need for extra blank line ^^
Okay.
>> + work_dir = tempfile.mkdtemp()
>> + log.info("working dir is {0}".format(work_dir))
>> +
>> + if (opts.fs_image or opts.no_virt) and not opts.disk_image:
>> + # Create pxe live images from a filesystem image
>> + disk_img = opts.fs_image or disk_img
>> + with Mount(disk_img, opts="loop") as mnt_dir:
>> + result_dir = make_live_images(opts, work_dir, mnt_dir, rootfs_image=disk_img)
>> + else:
>> + # Create pxe live images from a partitioned disk image
>> + disk_img = opts.disk_image or disk_img
>> + is_root_part = None
>> + if opts.ostree:
>> + is_root_part = lambda dir: os.path.exists(dir+"/ostree/deploy")
>> + with PartitionMount(disk_img, mount_ok=is_root_part) as img_mount:
>> + if img_mount and img_mount.mount_dir:
>> +
> No need for extra blank line ^^
>
>> + mounted_sysroot_boot_dir = None
>> + if opts.ostree:
>> + mounted_sysroot_boot_dir = mount_boot_part_over_root(img_mount)
>> +
>> + result_dir = make_live_images(opts, work_dir, img_mount.mount_dir)
>> + if mounted_sysroot_boot_dir:
>> + umount(mounted_sysroot_boot_dir)
> Wrap this bit in a try: finally with the umount in the finally
> part so that if things fail it doesn't leave things partially mounted.
Good idea.
> Also note that 'make check' will run pylint on the codebase, it
> shouldn't return any errors or warnings.
>
I'll use it.
More information about the anaconda-patches
mailing list