On 02/05/2014 12:47 PM, Petr Viktorin wrote:
> On 02/05/2014 11:23 AM, Petr Viktorin wrote:
>> On 02/05/2014 10:29 AM, Tomas Babej wrote:
>>> Hello,
>>>
>>> the attached patches fix the following tickets:
>>>
>>> https://fedorahosted.org/freeipa/ticket/4131
>>> https://fedorahosted.org/freeipa/ticket/4130
>>> https://fedorahosted.org/freeipa/ticket/4133
>>>
>>> Details in the commit messages.
>>>
>>> Tomas
>>>
>>
>> These look good, just a few nitpicks:
>>
>> Use a lowercase "A" in option and method names in 0144 to keep
>> consistent with our naming convention.
>>
>> Add an article to the add_a_record docstring & man page:
>> Adds an A record for the host to the IPA master
>>
>> and the help text for the host argument could be better:
>> Host whose record should be added
>> (or, Host for which the record should be added)
>>
>
> Another issue, in 0145 the copyfiles_command should be run with
> raiseonerr=False, so we don't fail in cease the directory doesn't exist.
>

Thank you for the review, updated patches attached.


>From 18ca2844c3ee891c06c930b01007458aa3f0115e Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 22 Jan 2014 13:33:41 +0100
Subject: [PATCH] ipatests: Add records for all hosts in master's domain

All the hosts in the domain have IPA master set as their only
nameserver. However, the IPA master does not create records for
these machines by default. This is not an big issue for clients
or replicas, since those records do get created in other ways,
but external hosts using their internal hostnames will not resolve.

Adds an A record for each host in master's domain.

https://fedorahosted.org/freeipa/ticket/4130
---
 ipatests/ipa-test-task             | 27 +++++++++++++++++++++++++++
 ipatests/man/ipa-test-task.1       |  7 +++++++
 ipatests/test_integration/tasks.py | 28 ++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/ipatests/ipa-test-task b/ipatests/ipa-test-task
index 48be36c97887b4f6aa258ff11e0075cc03defeb3..91bc8689cb63089069a8ce05396607fe4fbd032a 100755
--- a/ipatests/ipa-test-task
+++ b/ipatests/ipa-test-task
@@ -213,6 +213,24 @@ class TaskRunner(object):
                             help='Server that serves as a time source')
         subparser.set_defaults(func=self.sync_time)
 
+        subparser = subparsers.add_parser(
+            'add-a-records-in-master-domain',
+            help='Adds A records to the IPA master for all the hosts in the '
+                 'master domain.')
+        subparser.add_argument('master', type=str,
+                            help='IPA master to add records on')
+        subparser.set_defaults(
+            func=self.add_a_records_for_hosts_in_master_domain)
+
+        subparser = subparsers.add_parser(
+            'add-a-record',
+            help='Adds A record for the host to the IPA master')
+        subparser.add_argument('master', type=str,
+                            help='IPA master to add record on')
+        subparser.add_argument('host', type=str,
+                            help='Host whose record should be added')
+        subparser.set_defaults(func=self.add_a_record)
+
         return parser
 
     def main(self, argv):
@@ -397,5 +415,14 @@ class TaskRunner(object):
         server = self.get_host(args.server)
         tasks.sync_time(host, server)
 
+    def add_a_records_for_hosts_in_master_domain(self, args):
+        master = self.get_host(args.master, default=args.domain.master)
+        tasks.add_a_records_for_hosts_in_master_domain(master)
+
+    def add_a_record(self, args):
+        master = self.get_host(args.master, default=args.domain.master)
+        host = self.get_host(args.host)
+        tasks.add_a_record(master, host)
+
 if __name__ == '__main__':
     exit(TaskRunner().main(sys.argv[1:]))
diff --git a/ipatests/man/ipa-test-task.1 b/ipatests/man/ipa-test-task.1
index e73584bd3663fec72dafb68bcffbb166578d547f..3f523569951c545c9e516f2c1775871d9653d58a 100644
--- a/ipatests/man/ipa-test-task.1
+++ b/ipatests/man/ipa-test-task.1
@@ -147,6 +147,13 @@ Clears SSSD cache by removing the cache files. Restarts SSSD.
 Syncs the time with the remote server. Please note that this function leaves
 ntpd stopped.
 
+.TP
+\fBipa\-test\-task add\-a\-records\-in\-master\-domain MASTER\fR
+Adds A records to the IPA master for all the hosts in the master domain.
+
+.TP
+\fBipa\-test\-task add\-a\-record MASTER HOST\fR
+Adds an A record for the host to the IPA master.
 
 .SH "EXIT STATUS"
 0 if the command was successful
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 72196914f6d27cd46dd84eef15b3d1dd60aacdf3..ab027e0286aa8fddd743b1ed1e472e2592b75db0 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -572,6 +572,9 @@ def install_topo(topo, master, replicas, clients,
     installed = {master}
     if not skip_master:
         install_master(master)
+
+    add_a_records_for_hosts_in_master_domain(master)
+
     for parent, child in get_topo(topo)(master, replicas):
         if child in installed:
             log.info('Connecting replica %s to %s' % (parent, child))
@@ -632,3 +635,28 @@ def wait_for_replication(ldap, timeout=30):
             break
     else:
         log.error('Giving up wait for replication to finish')
+
+
+def add_a_records_for_hosts_in_master_domain(master):
+    for host in master.domain.hosts:
+        # We don't need to take care of the zone creation since it is master
+        # domain
+        add_a_record(master, host)
+
+
+def add_a_record(master, host):
+    # Find out if the record is already there
+    cmd = master.run_command(['ipa',
+                              'dnsrecord-find',
+                              master.domain.name,
+                              host.hostname,
+                              '--a-rec', host.ip],
+                             raiseonerr=False)
+
+    # If not, add it
+    if cmd.returncode != 0:
+        master.run_command(['ipa',
+                            'dnsrecord-add',
+                            master.domain.name,
+                            host.hostname,
+                            '--a-rec', host.ip])
-- 
1.8.4.2

>From ef7ecd6603fe18b5237f151256312d68bd542619 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 22 Jan 2014 15:11:04 +0100
Subject: [PATCH] ipatests: Run restoring backup files and restoring their
 context in one session

Restoring backup files and restoring their context were two separate commands,
what means that in case we use SSHTrasport, which creates a separate SSH
session for each command, we try to restore the SELinux context of the
changed files in a new session.

This causes problems, if the access to files themselves are necessary
for the creation of the new SSH session.

https://fedorahosted.org/freeipa/ticket/4133
---
 ipatests/test_integration/tasks.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index ab027e0286aa8fddd743b1ed1e472e2592b75db0..e13070a061bd1d38728ba1bfd202ce91831ec882 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -119,17 +119,21 @@ def restore_files(host):
     backupname = os.path.join(host.config.test_dir, 'file_backup')
     rmname = os.path.join(host.config.test_dir, 'file_remove')
 
-    # Restore the backed up files
-    host.run_command('cp -arvf %s/* /' % ipautil.shell_quote(backupname),
-                     raiseonerr=False)
-
-    # Restore context of the backed-up files
+    # Prepare command for restoring context of the backed-up files
     sed_remove_backupdir = 's/%s//g' % backupname.replace('/', '\/')
-    host.run_command("find %s | "
-                     "sed '%s' | "
-                     "sed '/^$/d' | "
-                     "xargs -d '\n' "
-                     "/sbin/restorecon -v" % (backupname, sed_remove_backupdir))
+    restorecon_command = (
+        "find %s | "
+        "sed '%s' | "
+        "sed '/^$/d' | "
+        "xargs -d '\n' "
+        "/sbin/restorecon -v" % (backupname, sed_remove_backupdir))
+
+    # Prepare command for actual restoring of the backed up files
+    copyfiles_command = 'cp -arvf %s/* /' % ipautil.shell_quote(backupname)
+
+    # Run both commands in one session. For more information, see:
+    # https://fedorahosted.org/freeipa/ticket/4133
+    host.run_command('%s ; (%s ||:)' % (restorecon_command, copyfiles_command))
 
     # Remove all the files that did not exist and were 'backed up'
     host.run_command(['xargs', '-d', r'\n', '-a', rmname, 'rm', '-vf'],
-- 
1.8.4.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to