Background ----------
OSTree is a tool to replicate pre-constructed *bootable* filesystem trees over plain HTTP, including support for things like GPG keysigning. rpm-ostree is a tool which takes RPM content on a "compose" server, and stores it into an OSTree repository.
This model is very useful for any deployment scenario where one wants to compose a system from the tested RPM parts, and then efficiently replicate it to many client systems. Examples are many cloud deployments, "Corporate Standard Build" laptops, all the way down to cash registers and embedded devices.
To make this work, OSTree is not just a tool to download content - it's also a set of *required changes* to the operating system content, such as /usr/lib/passwd: https://sourceware.org/bugzilla/show_bug.cgi?id=16142
Anaconda --------
For Fedora and derived systems like Red Hat Enterprise Linux, Anaconda is by far subsystem impacted the most. While the system is booted normally, while you are inside a chroot, for the most part the system "feels" like a traditional Unix system. You have writable /etc and /var.
However, really on disk, the operating system lives in e.g. /ostree/deploy/fedora-atomic/deploy/c79ba9b3c245fa5e256aa9217db5f4954b2639d1504d02462277cae3c30473ae.0
Anaconda however has a constant ROOT_PATH that needs to be split - some things like in the physical root, others live in what I placed in "iutil.getSysroot()".
There is no /etc/fstab on the *physical* filesystem for example - it is really in /ostree/deploy/fedora-atomic/deploy/c79ba9b3c245fa5e256aa9217db5f4954b2639d1504d02462277cae3c30473ae.0/etc/fstab.
Furthermore, OSTree wants to be in charge of setting bootloader configuration. (This is how the atomic updates work). So we have a multi-stage setup where Anaconda lays down the defaults (but no boot entries), then we tell OSTree to write the boot entries.
pykickstart -----------
This patch is pretty simple - it's defining a new "ostreesetup" verb that holds data.
blivet ------
The required changes here are similar to Anaconda - we need a distinction between ROOT_PATH and "targetSysroot".
Future ------
These patches are for early architectural feedback. The next step I'll take is to factor out some of the "quick hack" things that should be fixed elsewhere. For example, OSTree should know about /boot/extlinux in addition to /boot/syslinux. Also, https://bugzilla.gnome.org/show_bug.cgi?id=726757
On Thu, 2014-03-20 at 20:03 +0000, Colin Walters wrote:
Background
OSTree is a tool to replicate pre-constructed *bootable* filesystem trees over plain HTTP, including support for things like GPG keysigning. rpm-ostree is a tool which takes RPM content on a "compose" server, and stores it into an OSTree repository.
This model is very useful for any deployment scenario where one wants to compose a system from the tested RPM parts, and then efficiently replicate it to many client systems. Examples are many cloud deployments, "Corporate Standard Build" laptops, all the way down to cash registers and embedded devices.
To make this work, OSTree is not just a tool to download content - it's also a set of *required changes* to the operating system content, such as /usr/lib/passwd: https://sourceware.org/bugzilla/show_bug.cgi?id=16142
Anaconda
For Fedora and derived systems like Red Hat Enterprise Linux, Anaconda is by far subsystem impacted the most. While the system is booted normally, while you are inside a chroot, for the most part the system "feels" like a traditional Unix system. You have writable /etc and /var.
However, really on disk, the operating system lives in e.g. /ostree/deploy/fedora-atomic/deploy/c79ba9b3c245fa5e256aa9217db5f4954b2639d1504d02462277cae3c30473ae.0
Anaconda however has a constant ROOT_PATH that needs to be split - some things like in the physical root, others live in what I placed in "iutil.getSysroot()".
There is no /etc/fstab on the *physical* filesystem for example - it is really in /ostree/deploy/fedora-atomic/deploy/c79ba9b3c245fa5e256aa9217db5f4954b2639d1504d02462277cae3c30473ae.0/etc/fstab.
Furthermore, OSTree wants to be in charge of setting bootloader configuration. (This is how the atomic updates work). So we have a multi-stage setup where Anaconda lays down the defaults (but no boot entries), then we tell OSTree to write the boot entries.
pykickstart
This patch is pretty simple - it's defining a new "ostreesetup" verb that holds data.
blivet
The required changes here are similar to Anaconda - we need a distinction between ROOT_PATH and "targetSysroot".
Future
These patches are for early architectural feedback. The next step I'll take is to factor out some of the "quick hack" things that should be fixed elsewhere. For example, OSTree should know about /boot/extlinux in addition to /boot/syslinux. Also, https://bugzilla.gnome.org/show_bug.cgi?id=726757
The patches look less awful than I'd expect. :) They need some polishing from the coding style's perspective and some minor tweaks (blivet's targetSysroot should be set with a function as is the iutil's equivalent), but otherwise they look quite okay to me.
Once you are happy we the patches please rebase them on master branches of the projects and send them to anaconda-patches@lists.fedorahosted.org for a review, preferably with the 'git format-patch && git send-email' tool chain.
Thanks for your effort!
Just a couple things stood out to me from doing a quick review...
@@ -2361,6 +2368,26 @@ def writeSysconfigKernel(storage, version): f.write("HYPERVISOR_ARGS=logging=vga,serial,memory\n") f.close()
+def writeBootLoaderFinal(storage, payload, instClass, ksdata):
- """ Do the final write of the bootloader. """
- from pyanaconda.errors import errorHandler, ERROR_RAISE
- # set up dracut/fips boot args
- # XXX FIXME: do this from elsewhere?
- #storage.bootloader.set_boot_args(keyboard=anaconda.keyboard,
- # storage=anaconda.storage,
- # language=anaconda.instLanguage,
- # network=anaconda.network)
- storage.bootloader.set_boot_args(storage=storage,
payload=payload,
keyboard=ksdata.keyboard)
- try:
storage.bootloader.write()
- except BootLoaderError as e:
if errorHandler.cb(e) == ERROR_RAISE:
raise
def writeBootLoader(storage, payload, instClass, ksdata): """ Write bootloader configuration to disk.
@@ -2376,6 +2403,13 @@ def writeBootLoader(storage, payload, instClass, ksdata): stage2_device = storage.bootloader.stage2_device log.info("bootloader stage2 target device is %s" % stage2_device.name)
- if payload.handlesBootloaderConfiguration:
if storage.bootloader.skip_bootloader:
log.info("skipping bootloader install per user request")
return
writeBootLoaderFinal(storage, payload, instClass, ksdata)
return
- # get a list of installed kernel packages kernel_versions = payload.kernelVersionList if not kernel_versions:
@@ -2420,19 +2454,4 @@ def writeBootLoader(storage, payload, instClass, ksdata): label=label, short=short) storage.bootloader.add_image(image)
- # set up dracut/fips boot args
- # XXX FIXME: do this from elsewhere?
- #storage.bootloader.set_boot_args(keyboard=anaconda.keyboard,
- # storage=anaconda.storage,
- # language=anaconda.instLanguage,
- # network=anaconda.network)
- storage.bootloader.set_boot_args(storage=storage,
payload=payload,
keyboard=ksdata.keyboard)
- try:
storage.bootloader.write()
- except BootLoaderError as e:
if errorHandler.cb(e) == ERROR_RAISE:
raise
- writeBootLoaderFinal(storage, payload, instClass, ksdata)
Vratislav changed the set_boot_args calls a little recently to remove passing keyboard, so this won't apply cleanly.
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index df7b5e6..c788074 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -490,6 +490,13 @@ class Realm(commands.realm.F19_Realm):
log.info("Joined realm %s", self.join_realm)
+class OSTreeSetup(commands.ostreesetup.F20_OSTreeSetup):
- def __init__(self, *args):
commands.ostreesetup.F20_OSTreeSetup.__init__(self, *args)
- def execute(self, *args):
# This is implemented in packaging/ostreepayload.py
pass
class ClearPart(commands.clearpart.F17_ClearPart): def parse(self, args): @@ -1580,6 +1588,7 @@ commandMap = { "logvol": LogVol, "multipath": MultiPath, "network": Network,
"ostreesetup": OSTreeSetup, "part": Partition, "partition": Partition, "raid": Raid,
If you're not adding anaconda-specific behavior for a kickstart command, you don't need to do either of these chunks.
diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 824c8a4..7b863b5 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -1273,6 +1273,10 @@ reposdir=%s ### METHODS FOR WORKING WITH PACKAGES ### @property
- def supportsPackages(self):
return True
- @property def packages(self): from yum.Errors import RepoError
dnfpayload will also support packages.
diff --git a/pyanaconda/ui/gui/hubs/__init__.py b/pyanaconda/ui/gui/hubs/__init__.py index f8b016e..e739f1d 100644 --- a/pyanaconda/ui/gui/hubs/__init__.py +++ b/pyanaconda/ui/gui/hubs/__init__.py @@ -287,7 +287,17 @@ class Hub(GUIObject, common.Hub):
@property def continuePossible(self):
return len(self._incompleteSpokes) == 0 and len(self._notReadySpokes) == 0 and getattr(self._checker, "success", True)
if len(self._incompleteSpokes) > 0:
log.info("Incomplete spokes: %r" % (self._incompleteSpokes, ))
return False
if len(self._notReadySpokes) > 0:
log.info("NotReady spokes: %r" % (self._notReadySpokes, ))
return False
checkSuccess=getattr(self._checker, "success", True)
if not checkSuccess:
log.info("Checker returned %r" % (checkSuccess, ))
return False
return True
def _updateContinueButton(self): self.continueButton.set_sensitive(self.continuePossible)
Is this chunk just for debugging?
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py index b21866b..46e7ede 100644 --- a/pyanaconda/ui/gui/spokes/software.py +++ b/pyanaconda/ui/gui/spokes/software.py @@ -164,7 +164,9 @@ class SoftwareSelectionSpoke(NormalSpoke):
@property def showable(self):
return not flags.livecdInstall and not self.data.method.method == "liveimg"
return (self.payload.supportsPackages
and not flags.livecdInstall
and not self.data.method.method == "liveimg")
@property def status(self):
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py index 59ff82c..add7fbc 100644 --- a/pyanaconda/ui/gui/spokes/source.py +++ b/pyanaconda/ui/gui/spokes/source.py @@ -761,7 +761,9 @@ class SourceSpoke(NormalSpoke):
@property def showable(self):
return not flags.livecdInstall and not self.data.method.method == "liveimg"
return (self.payload.supportsPackages
and not flags.livecdInstall
and not self.data.method.method == "liveimg")
def _mirror_active(self): return self._protocolComboBox.get_active() == PROTOCOL_MIRROR
"not flags.livecdInstall and not self.data.method.method == 'liveimg'" is basically the same as saying self.payload.supportsPackages. It's certainly what we were going for there. So you might as well just completely replace the existing tests with your new one.
From cad071d736cb35e67245ca11cf8750931c0d98ba Mon Sep 17 00:00:00 2001
From: Colin Walters walters@verbum.org Date: Tue, 11 Mar 2014 09:33:36 -0400 Subject: [PATCH] ostreesetup: New command
This tells the installer to handle an OSTree repository.
pykickstart/commands/__init__.py | 3 +- pykickstart/commands/ostreesetup.py | 74 +++++++++++++++++++++++++++++++++++++ pykickstart/handlers/control.py | 2 + tests/commands/ostreesetup.py | 37 +++++++++++++++++++ 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 pykickstart/commands/ostreesetup.py create mode 100644 tests/commands/ostreesetup.py
diff --git a/pykickstart/commands/__init__.py b/pykickstart/commands/__init__.py index c5145ba..ed4d649 100644 --- a/pykickstart/commands/__init__.py +++ b/pykickstart/commands/__init__.py @@ -21,6 +21,7 @@ import authconfig, autopart, autostep, bootloader, btrfs, clearpart, cdrom, devi import deviceprobe, displaymode, dmraid, driverdisk, eula, fcoe, firewall, firstboot import group, harddrive, ignoredisk, interactive, iscsi, iscsiname, key, keyboard, lang import langsupport, lilocheck, liveimg, logging, logvol, mediacheck, method, monitor -import mouse, multipath, network, nfs, partition, raid, realm, reboot, repo, rescue +import mouse, multipath, network, nfs, ostreesetup +import partition, raid, realm, reboot, repo, rescue import rootpw, selinux, services, skipx, sshpw, timezone, updates, upgrade, url, user import unsupported_hardware, vnc, volgroup, xconfig, zerombr, zfcp
I changed this file to make pylint happier, so this chunk won't apply cleanly. Look at pykickstart in git and you'll see the obvious thing to change. It's not hard.
diff --git a/pykickstart/commands/ostreesetup.py b/pykickstart/commands/ostreesetup.py new file mode 100644 index 0000000..b722c2c --- /dev/null +++ b/pykickstart/commands/ostreesetup.py @@ -0,0 +1,74 @@ +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +#
+from pykickstart.base import * +from pykickstart.errors import * +from pykickstart.options import *
I've converted pykickstart away from doing global imports, so you'll want to make changes here too. The changes will look like this:
from pykickstart.base import KickstartCommand from pykickstart.options import KSOptionParser
You're not using anything out of pykickstart.errors.
+import gettext +_ = lambda x: gettext.ldgettext("pykickstart", x)
+class F20_OSTreeSetup(KickstartCommand):
- removedKeywords = KickstartCommand.removedKeywords
- removedAttrs = KickstartCommand.removedAttrs
- def __init__(self, *args, **kwargs):
KickstartCommand.__init__(self, *args, **kwargs)
self.op = self._getParser()
self.osname = kwargs.get('osname', None)
self.remote = kwargs.get("remote", self.osname)
self.url = kwargs.get('url', None)
self.ref = kwargs.get('ref', None)
self.noGpg = kwargs.get('noGpg', True)
- def __str__(self):
retval = KickstartCommand.__str__(self)
if self.osname:
retval += "# OSTree setup\n"
retval += "ostreesetup %s\n" % self._getArgsAsStr()
return retval
- def _getArgsAsStr(self):
retval = ""
if self.osname:
retval += "--osname=%s" % self.osname
if self.remote:
retval += "--remote=%s" % self.remote
if self.url:
retval += "--url=%s" % self.url
if self.ref:
retval += "--ref=%s" % self.ref
if self.noGpg:
retval += "--nogpg"
return retval
You will want to put a space in front of the -- in all of these, or using multiple options will result in a string that looks like:
ostreesetup --osname=blah--url=http://blah--nogpg
You may wish to add a test case that uses multiple arguments.
Also... can osname, remote, or ref have a space in it? If so, you'll want to surround the %s with quotes.
- Chris
On Thu, Mar 20, 2014 at 08:03:10PM +0000, Colin Walters wrote:
turnOnFilesystems(storage, mountOnly=flags.flags.dirInstall)
- if not flags.flags.livecdInstall and not flags.flags.dirInstall:
write_storage_late = flags.flags.livecdInstall or ksdata.ostreesetup.osname
if not write_storage_late and not flags.flags.dirInstall: storage.write()
# Do packaging.
@@ -169,15 +179,30 @@ def doInstall(storage, payload, ksdata, instClass): payload.preInstall(packages=packages, groups=payload.languageGroups()) payload.install()
- if flags.flags.livecdInstall:
storage.write()
- with progress_report(_("Performing post-installation setup tasks")):
payload.postInstall()
- if write_storage_late:
if iutil.getSysroot() != ROOT_PATH:
blivet.targetSysroot = iutil.getSysroot()
storage.write()
# Now that we have the FS layout in the target,
# umount things that were in the legacy sysroot,
# and put them in the target root, except for the
# physical /
storage.umountFilesystems()
rootmnt = storage.mountpoints.get('/')
rootmnt.setup()
# Explicitly mount the root on the physical sysroot
rootmnt.format.setup(rootmnt.format.options, chroot=ROOT_PATH)
# But everything else goes in the target root
storage.mountFilesystems(skipRoot=True)
else:
storage.write()
You need to skip this if flags.flags.dirInstall is True, directory install disables the storage code and probably also won't work with OSTree.
# Do bootloader. if not flags.flags.dirInstall: with progress_report(_("Installing bootloader")): writeBootLoader(storage, payload, instClass, ksdata)
with progress_report(_("Performing post-installation setup tasks")):
payload.postInstall()
Why move this here instead of leaving it above writeBootLoader?
- def _safeExecWithRedirect(self, cmd, argv, **kwargs):
"""Like iutil.execWithRedirect, but treat errors as fatal"""
rc = iutil.execWithRedirect(cmd, argv, **kwargs)
if rc != 0:
exn = PayloadInstallError("%s %s exited with code %d" % (cmd, argv, rc))
if errorHandler.cb(exn) == ERROR_RAISE:
raise exn
You may as well add this to iutil as another variation on ExecWith
diff --git a/pyanaconda/users.py b/pyanaconda/users.py index d43ad29..e00460e 100644 --- a/pyanaconda/users.py +++ b/pyanaconda/users.py @@ -192,7 +192,7 @@ class Users: """
childpid = os.fork()
root = kwargs.get("root", "/mnt/sysimage")
root = kwargs.get("root", iutil.getSysroot()) if not childpid: if not root in ["","/"]:
@@ -258,7 +258,7 @@ class Users: available one is used. """ childpid = os.fork()
root = kwargs.get("root", "/mnt/sysimage")
root = kwargs.get("root", iutil.getSysroot()) if not childpid: if not root in ["","/"]:
@@ -358,12 +358,16 @@ class Users: else: return False
- def checkUserExists(self, username, root="/mnt/sysimage"):
def checkUserExists(self, username, root=None): childpid = os.fork()
if root is not None:
rootval = root
else:
rootval = iutil.getSysroot() if not childpid:
if not root in ["","/"]:
os.chroot(root)
if not rootval in ["","/"]:
os.chroot(rootval) os.chdir("/") del(os.environ["LIBUSER_CONF"])
Looks like we have some duplicate code, it might be time to clean up users.py a bit.
On Fri, Mar 21, 2014 at 3:07 PM, Brian C. Lane bcl@redhat.com wrote:
- def _safeExecWithRedirect(self, cmd, argv, **kwargs):
"""Like iutil.execWithRedirect, but treat errors as
fatal"""
rc = iutil.execWithRedirect(cmd, argv, **kwargs)
if rc != 0:
exn = PayloadInstallError("%s %s exited with code %d"
% (cmd, argv, rc))
if errorHandler.cb(exn) == ERROR_RAISE:
raise exn
You may as well add this to iutil as another variation on ExecWith
I don't think I can throw a PayloadInstallError from iutil, which raises the question of which exception. Maybe subprocess.CalledProcessError?
Do we need to duplicate the bits to invoke errorHandler in all of these subprocess calls? Since all of these errors are fatal, it seems like a toplevel handler should catch them anyways.
On Wed, Mar 26, 2014 at 02:02:51AM +0000, Colin Walters wrote:
On Fri, Mar 21, 2014 at 3:07 PM, Brian C. Lane bcl@redhat.com wrote:
- def _safeExecWithRedirect(self, cmd, argv, **kwargs):
"""Like iutil.execWithRedirect, but treat errors as
fatal"""
rc = iutil.execWithRedirect(cmd, argv, **kwargs)
if rc != 0:
exn = PayloadInstallError("%s %s exited with code
%d" % (cmd, argv, rc))
if errorHandler.cb(exn) == ERROR_RAISE:
raise exn
You may as well add this to iutil as another variation on ExecWith
I don't think I can throw a PayloadInstallError from iutil, which raises the question of which exception. Maybe subprocess.CalledProcessError?
Do we need to duplicate the bits to invoke errorHandler in all of these subprocess calls? Since all of these errors are fatal, it seems like a toplevel handler should catch them anyways.
Nevermind, it would be more awkward to try and move it into iutil.
On Fri, Mar 21, 2014 at 3:07 PM, Brian C. Lane bcl@redhat.com wrote:
You need to skip this if flags.flags.dirInstall is True, directory install disables the storage code and probably also won't work with OSTree.
Right, OSTree is oriented towards bootable trees, it wants to write bootloader configuration. Now you *can* easily just use the raw:
"ostree --repo=/path/to/repo checkout fedora-atomic/rawhide/x86_64/buildmaster/base/core /path/to/checkout".
But I can't think of a real use case for that offhand in the Anaconda context anyways. It is useful to do inside a running system to get a tree suitable for "systemd-nspawn" though.
I'll evaluate the code here more carefully to be sure we aren't breaking dirInstall.
Why move this here instead of leaving it above writeBootLoader?
For OSTreePayload, at the moment I need to do some postprocessing of the bootloader configuration. Theoretically I could land the changes in the RPM set to make that unnecessary, but it's hard to do without breaking things.
From what I can see of the other Payload subclasses in Anaconda, they are insensitive to whether or not the bootloader is installed by the time postInstall() is invoked.
I can break out a separate patch with this rationale to just swap the order, or I can introduce a new postBootloaderInstall() method on Payload. Any preference?
On Thu, Apr 24, 2014 at 10:20:41PM +0000, Colin Walters wrote:
On Fri, Mar 21, 2014 at 3:07 PM, Brian C. Lane bcl@redhat.com wrote:
Why move this here instead of leaving it above writeBootLoader?
For OSTreePayload, at the moment I need to do some postprocessing of the bootloader configuration. Theoretically I could land the changes in the RPM set to make that unnecessary, but it's hard to do without breaking things.
From what I can see of the other Payload subclasses in Anaconda, they are insensitive to whether or not the bootloader is installed by the time postInstall() is invoked.
I can break out a separate patch with this rationale to just swap the order, or I can introduce a new postBootloaderInstall() method on Payload. Any preference?
It's probably fine as is then, thanks for the explanation.
anaconda-devel@lists.stg.fedoraproject.org