[PATCH] anaconda realm command

Vratislav Podzimek vpodzime at redhat.com
Mon Dec 3 17:42:56 UTC 2012


> From 5a18b92ad5e4e99c447edd29440bb0fb687b70e4 Mon Sep 17 00:00:00 2001
> From: Stef Walter <stefw at gnome.org>
> Date: Fri, 16 Nov 2012 11:28:43 +0100
> Subject: [PATCH 2/2] Add support for the realm kickstart command
> 
> This command uses realmd to discover information about the domain
> to join, which adds in the right packages, and then joins the domain
> post install.
> ---
>  anaconda.spec.in         |  1 +
>  pyanaconda/install.py    | 11 +++++++-
>  pyanaconda/kickstart.py  | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
>  pyanaconda/yuminstall.py |  2 +-
>  4 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/anaconda.spec.in b/anaconda.spec.in
> index 0b5ef39..c2d8333 100644
> --- a/anaconda.spec.in
> +++ b/anaconda.spec.in
> @@ -122,6 +122,7 @@ Requires: python-pyblock >= %{pythonpyblockver}
>  Requires: libuser-python
>  Requires: newt-python
>  Requires: authconfig
> +Requires: realmd
>  Requires: firewalld >= %{firewalldver}
>  Requires: cryptsetup-luks
>  Requires: python-cryptsetup >= %{pythoncryptsetupver}
> diff --git a/pyanaconda/install.py b/pyanaconda/install.py
> index b6fc3e8..bdd7090 100644
> --- a/pyanaconda/install.py
> +++ b/pyanaconda/install.py
> @@ -78,6 +78,10 @@ def doConfiguration(storage, payload, ksdata, instClass):
>          ksdata.group.execute(storage, ksdata, instClass, u)
>          ksdata.user.execute(storage, ksdata, instClass, u)
>  
> +    if ksdata.realm.discovered:
> +        with progress_report(_("Joining realm: %s" % ksdata.realm.discovered)):
> +            ksdata.realm.execute(storage, ksdata, instClass)
> +
You need to increment the number in progress.send_init() at the
begining of doConfiguration if there is one more step. Otherwise the
progress bar will be one step ahead.

>      with progress_report(_("Running post install scripts")):
>          runPostScripts(ksdata.scripts)
>  
> @@ -116,10 +120,15 @@ def doInstall(storage, payload, ksdata, instClass):
>  
>      # Do packaging.
>  
> +    # Discover information about realms to join, to determine additional packages
> +    if ksdata.realm.join_realm:
> +        with progress_report(_("Discovering realm to join")):
> +            ksdata.realm.discover()
> +
The same goes also for progress.send_init() in doInstall.

>      # anaconda requires storage packages in order to make sure the target
>      # system is bootable and configurable, and some other packages in order
>      # to finish setting up the system.
> -    packages = storage.packages + ["authconfig", "firewalld"]
> +    packages = storage.packages + ["authconfig", "firewalld"] + ksdata.realm.packages
>      payload.preInstall(packages=packages, groups=payload.languageGroups(ksdata.lang.lang))
>      payload.install()
>  
> diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> index 2600c65..e501947 100644
> --- a/pyanaconda/kickstart.py
> +++ b/pyanaconda/kickstart.py
> @@ -1262,6 +1262,75 @@ class RaidData(commands.raid.F18_RaidData):
>                                       parents=request)
>              storage.createDevice(luksdev)
>  
> +class Realm(commands.realm.F19_Realm):
> +    def __init__(self, *args):
> +        commands.realm.F19_Realm.__init__(self, *args)
> +        self.packages = []
> +        self.discovered = ""
> +
> +    def discover(self):
> +        if not self.join_realm:
> +            return
> +
> +        try:
> +            args = ["discover", "--verbose", "--install", "/"] + \
> +                   self.discover_options + [self.join_realm]
> +            output = iutil.execWithCapture("/usr/sbin/realm", args, stderr="/dev/tty5", fatal=True)
Could you please split this line to two? The same goes for some others
longer than let's say 90 characters (80 ideally).

> +        except RuntimeError as msg:
> +            log.error("Error running /usr/sbin/realm %s: %s", args, msg)
> +            return
> +        except OSError as msg:
> +            # TODO: This is a lousy way of propagating what will usually be 'no such realm'
> +            log.error("Error running /usr/sbin/realm %s: %s", args, msg)
> +            return
> +
> +        # Now parse the output for the required software. First line is the
> +        # realm name, and following lines are information as "name: value"
> +        self.packages = ["realmd"]
> +        self.discovered = ""
> +
> +        for line in output.split("\n"):
> +            if not self.discovered:
> +                self.discovered = line.strip()
> +            else:
> +                parts = line.split (":", 1)
> +                if len(parts) == 2 and parts[0].strip() == "required-package":
> +                    self.packages.append(parts[1].strip())
Are you expecting some empty lines at the begining of the output?
Because otherwise I believe something like:

        lines = output.split("\n")
        if not lines:
            return
        	self.discovered = lines.pop(0).strip()
        
        for line in lines:
            parts = line.split(":", 1)
            if len(parts) == 2 and parts[0].strip() == "required-package":
                self.packagse.append(parts[1].strip())
        
would do the same and could be better readable. This is probably just a
matter of opinion. If you are going to use your version, please remove
the additional space in 'parts = line.split (":", 1)'
> 
> +
> +        log.info("Realm %s needs packages %s" % (self.discovered, ", ".join(self.packages)))
> +
> +    def execute(self, *args):
> +        if not self.discovered:
> +            return
> +
> +        pw_args = ["--no-password"]
> +        for arg in self.join_args:
> +            if arg.startswith("--no-password") or arg.startswith("--one-time-password"):
> +                pw_args = []
'break' should go here? Or, if you want, you could use:

        if any(arg.startswith("--no-password")
               or arg.startswith("--one-time-password")
               for arg in self.join_args):
             pw_args = []
        else:
             pw_args = ["--no-password"]

> +
> +        args = ["join", "--install", ROOT_PATH, "--verbose"] + pw_args + self.join_args
> +        try:
> +            rc = iutil.execWithRedirect("/usr/sbin/realm", args,
> +                                        stdout="/dev/tty5", stderr="/dev/tty5")
> +        except RuntimeError as msg:
> +            log.error("Error running /usr/sbin/realm %s: %s", args, msg)
> +
> +        if rc != 0:
> +            log.error("Command failure: /usr/sbin/realm %s: %d", args, rc)
> +            return
> +
> +        log.info("Joined realm %s", self.join_realm)
> +
> +        for (command, options) in self.after:
> +            args = [command, "--install", ROOT_PATH, "--verbose"] + options
> +            rc = iutil.execWithRedirect("/usr/sbin/realm", args,
> +                                        stdout="/dev/tty5", stderr="/dev/tty5")
> +            if rc != 0:
> +                log.error("Command failure: /usr/sbin/realm %s: %d", args, rc)
> +
> +            log.info("Ran /usr/sbin/realm %s", args)
> +
> +
>  class RootPw(commands.rootpw.F18_RootPw):
>      def execute(self, storage, ksdata, instClass, users):
>          algo = getPassAlgo(ksdata.authconfig.authconfig)
> @@ -1443,6 +1512,7 @@ commandMap = {
>          "part": Partition,
>          "partition": Partition,
>          "raid": Raid,
> +        "realm": Realm,
>          "rootpw": RootPw,
>          "selinux": SELinux,
>          "services": Services,
> diff --git a/pyanaconda/yuminstall.py b/pyanaconda/yuminstall.py
> index aef7e8f..f8eb609 100644
> --- a/pyanaconda/yuminstall.py
> +++ b/pyanaconda/yuminstall.py
> @@ -1488,7 +1488,7 @@ reposdir=/etc/anaconda.repos.d,/tmp/updates/anaconda.repos.d,/tmp/product/anacon
>      # installed (they could have been removed in kickstart).  So we'll force
>      # it.
>      def selectAnacondaNeeds(self):
> -        for pkg in ['authconfig', 'chkconfig', 'system-config-firewall-base']:
> +        for pkg in ['authconfig', 'chkconfig', 'realmd', 'system-config-firewall-base']:
Do we really want 'realmd' installed on every system?

Otherwise it looks good to me but as I've already mentioned in the
reply for the pykickstart patches I believe this should be a separate
addon and not part of the Anaconda itself.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list