Re: [Freeipa-devel] First batch of ipatests fixes

2014-02-05 Thread Petr Viktorin

On 02/05/2014 01:06 PM, Tomas Babej wrote:


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.


Thank you! ACK, pushed to
master: 1601860023193ec295458a71f1f097edbb57d787
ipa-3-3: 57e6b5bdc540551312fd674c56ded7dbb7322677


--
Petr³

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


Re: [Freeipa-devel] First batch of ipatests fixes

2014-02-05 Thread Tomas Babej

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 
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

Re: [Freeipa-devel] First batch of ipatests fixes

2014-02-05 Thread Petr Viktorin

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.


--
Petr³

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


Re: [Freeipa-devel] First batch of ipatests fixes

2014-02-05 Thread Petr Viktorin

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)

--
Petr³

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


[Freeipa-devel] First batch of ipatests fixes

2014-02-05 Thread Tomas Babej
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
>From 64542c3c9e8f56d28e39775d8351262514322873 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 22 Jan 2014 11:44:34 +0100
Subject: [PATCH] ipatests: test_legacy_clients: Change "test group" to
 "testgroup"

The integration test for legacy clients used incorrectly "test group"
instead of "testgroup" as group used on AD for test purposes. This
is inconsistent with the usage of "testuser".

https://fedorahosted.org/freeipa/ticket/4131
---
 ipatests/test_integration/test_legacy_clients.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/test_legacy_clients.py b/ipatests/test_integration/test_legacy_clients.py
index 6bbe54b32778b9185d523b00c9c87fd76bd15d53..3edceb2dcb110557fe7566534fb10a466d8f78a8 100644
--- a/ipatests/test_integration/test_legacy_clients.py
+++ b/ipatests/test_integration/test_legacy_clients.py
@@ -118,7 +118,7 @@ class BaseTestLegacyClient(trust_tests.TestEnforcedPosixADTrust):
 
 def test_getent_ad_group(self):
 self.clear_sssd_caches()
-testgroup = 'test group@%s' % self.ad.domain.name
+testgroup = 'testgroup@%s' % self.ad.domain.name
 result = self.legacy_client.run_command(['getent', 'group', testgroup])
 
 testgroup_stdout = "%s:\*:10047:" % testgroup
@@ -127,7 +127,7 @@ class BaseTestLegacyClient(trust_tests.TestEnforcedPosixADTrust):
 def test_id_ad_user(self):
 self.clear_sssd_caches()
 testuser = 'testuser@%s' % self.ad.domain.name
-testgroup = 'test group@%s' % self.ad.domain.name
+testgroup = 'testgroup@%s' % self.ad.domain.name
 
 result = self.legacy_client.run_command(['id', testuser])
 
-- 
1.8.4.2

>From 8ae83923b07560d4f2b5019cc0961be28ee18acc Mon Sep 17 00:00:00 2001
From: Tomas Babej 
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..e5bc395349136aca8b98aedc7b8f34dec4569468 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 which 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..0858a517826517ccca0e5c324127649f12d12972 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