This patch adds the realm command to anaconda. It depends on the pykickstart changes I added earlier (hmmm, how should I reflect this dependency?).
Again, the big use case is automatic AD domain joins by adding a line to a kickstart file. The line in the kickstart file looks like:
realm join domain.example.com
or:
realm join --one-time-password=mypassword domain.example.com
Attached are two patches. The first one sets the kernel hostname during the post install, so it can be used by stuff running in the chroot. realmd needs the hostname to be appropriately set.
The second patch adds the realm command implementation. It does this:
* Early in the install the realm is discovered. This tells us what kind of realm (such as AD domain vs. IPA domain etc.) and what kind of packages need to be installed to support it. The required packages are added to the install.
* After the install and configuration, we join the realm using the realmd. realmd has support for running within a chroot, and without a dbus system bus etc. So we use this to run within /mnt/sysimage.
Happy to adapt this as necessary. Looking forward to review.
Cheers,
Stef
I'll do real review later, but fow now I can answer the one easy question...
This patch adds the realm command to anaconda. It depends on the pykickstart changes I added earlier (hmmm, how should I reflect this dependency?).
We do this by changing this line in anaconda.spec.in:
%define pykickstartver 1.99.22
Of course, you don't know what version's going to be required yet and neither do I since this is bound for F19. We'll just have to make sure to remember to change this at merge time.
- Chris
On 11/21/2012 05:26 PM, Chris Lumens wrote:
I'll do real review later, but fow now I can answer the one easy question...
This patch adds the realm command to anaconda. It depends on the pykickstart changes I added earlier (hmmm, how should I reflect this dependency?).
We do this by changing this line in anaconda.spec.in:
%define pykickstartver 1.99.22
Of course, you don't know what version's going to be required yet and neither do I since this is bound for F19. We'll just have to make sure to remember to change this at merge time.
Makes sense.
Anything I can do to help get these reviewed? Or did I just post them at a bad, busy time?
Cheers,
Stef
Anything I can do to help get these reviewed? Or did I just post them at a bad, busy time?
It's just a bad time. Once we have a separate f18-final-branch and master is tracking rawhide again, we can definitely get this in. I'll leave it marked as unread in my mail so I will know to go back and take a look.
- Chris
On Wed, 2012-11-28 at 12:01 -0500, clumens@redhat.com wrote:
Anything I can do to help get these reviewed? Or did I just post them at a bad, busy time?
It's just a bad time. Once we have a separate f18-final-branch and master is tracking rawhide again, we can definitely get this in. I'll leave it marked as unread in my mail so I will know to go back and take a look.
I'm personally interested in these things, so I'll look at the patches. I expect you'll want to see it anyway, but at least as a first round.
On 11/28/2012 06:25 PM, Vratislav Podzimek wrote:
On Wed, 2012-11-28 at 12:01 -0500, clumens@redhat.com wrote:
Anything I can do to help get these reviewed? Or did I just post them at a bad, busy time?
It's just a bad time. Once we have a separate f18-final-branch and master is tracking rawhide again, we can definitely get this in. I'll leave it marked as unread in my mail so I will know to go back and take a look.
Thanks Chris.
I'm personally interested in these things, so I'll look at the patches. I expect you'll want to see it anyway, but at least as a first round.
Wonderful. Thank you.
Stef
From 5a18b92ad5e4e99c447edd29440bb0fb687b70e4 Mon Sep 17 00:00:00 2001 From: Stef Walter stefw@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:returntry: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)returnexcept 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:returnpw_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_argstry: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)returnlog.info("Joined realm %s", self.join_realm)for (command, options) in self.after:args = [command, "--install", ROOT_PATH, "--verbose"] + optionsrc = 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.
Thanks for the review. Made the changes noted to the attached patches.
On 12/03/2012 06:42 PM, Vratislav Podzimek wrote:
Are you expecting some empty lines at the begining of the output?
Nope. Your code is more readable so I've put it in the patch. Thanks.
log.info("Realm %s needs packages %s" % (self.discovered, ", ".join(self.packages)))- def execute(self, *args):
if not self.discovered:returnpw_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?
Doesn't really matter, but is more readable, so added.
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?
No. We do however need it in the installer. Once it's there, then the anaconda realm command installs it (if necessary) on the system.
Should I remove it from the line above? What does the above reflect? Because Anaconda does need it in this case. LMK.
Cheers,
Stef
On Wed, 2012-11-21 at 09:55 +0100, Stef Walter wrote:
This patch adds the realm command to anaconda. It depends on the pykickstart changes I added earlier (hmmm, how should I reflect this dependency?).
yuminstall.py is not in use on master or f18-*branch anymore. I don't think you want to keep the changes in there anyway, so you can probably just drop that part of the patch.
David
On 12/04/2012 04:39 PM, David Lehman wrote:
On Wed, 2012-11-21 at 09:55 +0100, Stef Walter wrote:
This patch adds the realm command to anaconda. It depends on the pykickstart changes I added earlier (hmmm, how should I reflect this dependency?).
yuminstall.py is not in use on master or f18-*branch anymore. I don't think you want to keep the changes in there anyway, so you can probably just drop that part of the patch.
Thanks for the heads up. Vratislav suggested the same, and I posted an updated patch without it.
Cheers,
Stef
anaconda-patches@lists.fedorahosted.org