[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