Re: [Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Oleg Fayans]

Any chance this patch can be merged this week?

On 12/14/2015 02:08 PM, Oleg Fayans wrote:
> Hi Martin,
> 
> On 12/11/2015 05:58 PM, Martin Basti wrote:
>>
>>
>> On 11.12.2015 17:28, Oleg Fayans wrote:
>>> +myre = re.compile(".*Backed up to (?P.*?)\n.*")
>>
>> IMO this regexp is not good.
>>
>> 1)
>> please name it better than "myre"
> 
> Done
> 
>>
>> 2)
>> initial '.*' is not needed because regexp does not start with '^' and
>> you use search() later
>>
>> 3)
>>
>> trailing '.*' is not needed as well, because it does not end with '$'
>>
>> 4)
>> You can use re.MULTILINE that will parse string per lines
>>
>> path_re = re.compile("^Backed up to (?P.*)$", re.MULTILINE)
> 
> Used it, thanks!
> 
>>
>> 5)
>> +matched = myre.search(result.stdout_text + result.stderr_text)
>> Why do you need search in both stderr and stdout?
> 
> Because of this bug: https://fedorahosted.org/freeipa/ticket/5484
> 
>>
>> Martin^2
>>
>>
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Martin Basti]

I cannot apply your patch on master branch, missing blobs, can you 
rebase please?


On 06.01.2016 11:47, Oleg Fayans wrote:

Any chance this patch can be merged this week?

On 12/14/2015 02:08 PM, Oleg Fayans wrote:

Hi Martin,

On 12/11/2015 05:58 PM, Martin Basti wrote:


On 11.12.2015 17:28, Oleg Fayans wrote:

+myre = re.compile(".*Backed up to (?P.*?)\n.*")

IMO this regexp is not good.

1)
please name it better than "myre"

Done


2)
initial '.*' is not needed because regexp does not start with '^' and
you use search() later

3)

trailing '.*' is not needed as well, because it does not end with '$'

4)
You can use re.MULTILINE that will parse string per lines

path_re = re.compile("^Backed up to (?P.*)$", re.MULTILINE)

Used it, thanks!


5)
+matched = myre.search(result.stdout_text + result.stderr_text)
Why do you need search in both stderr and stdout?

Because of this bug: https://fedorahosted.org/freeipa/ticket/5484


Martin^2







--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Oleg Fayans]

Hi Martin,

On 12/11/2015 05:58 PM, Martin Basti wrote:
> 
> 
> On 11.12.2015 17:28, Oleg Fayans wrote:
>> +myre = re.compile(".*Backed up to (?P.*?)\n.*")
> 
> IMO this regexp is not good.
> 
> 1)
> please name it better than "myre"

Done

> 
> 2)
> initial '.*' is not needed because regexp does not start with '^' and
> you use search() later
> 
> 3)
> 
> trailing '.*' is not needed as well, because it does not end with '$'
> 
> 4)
> You can use re.MULTILINE that will parse string per lines
> 
> path_re = re.compile("^Backed up to (?P.*)$", re.MULTILINE)

Used it, thanks!

> 
> 5)
> +matched = myre.search(result.stdout_text + result.stderr_text)
> Why do you need search in both stderr and stdout?

Because of this bug: https://fedorahosted.org/freeipa/ticket/5484

> 
> Martin^2
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 1805ac791e061117405560dbca371f1666c1b9e2 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Mon, 14 Dec 2015 14:04:07 +0100
Subject: [PATCH] Fixed install_ca and install_kra under domain level 0

Also added ipa_backup, ipa_restore and replica_uninstall functions
---
 ipatests/test_integration/tasks.py | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c3681fca952807ac6ebcca56ce961df2d3f33f0c..70354919e6ed8b1f21b19e838a3083c89a1bdc6d 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -932,9 +932,22 @@ def resolve_record(nameserver, query, rtype="SOA", retry=True, timeout=100):
 time.sleep(1)
 
 
+def ipa_backup(master):
+result = master.run_command(["ipa-backup"])
+path_re = re.compile("^Backed up to (?P.*)$", re.MULTILINE)
+matched = path_re.search(result.stdout_text + result.stderr_text)
+return matched.group("backup")
+
+
+def ipa_restore(master, backup_path):
+master.run_command(["ipa-restore", "-U",
+"-p", master.config.dirman_password,
+backup_path])
+
+
 def install_kra(host, domain_level=None, first_instance=False, raiseonerr=True):
-if not domain_level:
-   domain_level = domainlevel(host)
+if domain_level is None:
+domain_level = domainlevel(host)
 command = ["ipa-kra-install", "-U", "-p", host.config.dirman_password]
 if domain_level == DOMAIN_LEVEL_0 and not first_instance:
 replica_file = get_replica_filename(host)
@@ -943,8 +956,8 @@ def install_kra(host, domain_level=None, first_instance=False, raiseonerr=True):
 
 
 def install_ca(host, domain_level=None, first_instance=False, raiseonerr=True):
-if not domain_level:
-   domain_level = domainlevel(host)
+if domain_level is None:
+domain_level = domainlevel(host)
 command = ["ipa-ca-install", "-U", "-p", host.config.dirman_password,
"-P", 'admin', "-w", host.config.admin_password]
 if domain_level == DOMAIN_LEVEL_0 and not first_instance:
@@ -961,3 +974,10 @@ def install_dns(host, raiseonerr=True):
 "-U",
 ]
 return host.run_command(args, raiseonerr=raiseonerr)
+
+
+def uninstall_replica(master, replica):
+master.run_command(["ipa-replica-manage", "del", "--force",
+"-p", master.config.dirman_password,
+replica.hostname], raiseonerr=False)
+uninstall_master(replica)
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Martin Basti]

On 11.12.2015 17:28, Oleg Fayans wrote:

+myre = re.compile(".*Backed up to (?P.*?)\n.*")


IMO this regexp is not good.

1)
please name it better than "myre"

2)
initial '.*' is not needed because regexp does not start with '^' and 
you use search() later


3)

trailing '.*' is not needed as well, because it does not end with '$'

4)
You can use re.MULTILINE that will parse string per lines

path_re = re.compile("^Backed up to (?P.*)$", re.MULTILINE)

5)
+matched = myre.search(result.stdout_text + result.stderr_text)
Why do you need search in both stderr and stdout?

Martin^2


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Petr Vobornik]

On 12/11/2015 05:28 PM, Oleg Fayans wrote:

HI Oleg,

could you prefix the commit message and mail subject with "tests: " or 
something similar to make clear that this is a fix in tests and not 
actual CA or KRA installation.

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0018] Fixed install_ca and install_kra failures at domain level 0

[Oleg Fayans]

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 93b8e9fcbbba0db1f21924b46097c557c9cca358 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Fri, 11 Dec 2015 15:58:39 +0100
Subject: [PATCH] Fixed install_ca and install_kra under domain level 0

Also added ipa_backup, ipa_restore and replica_uninstall functions
---
 ipatests/test_integration/tasks.py | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c3681fca952807ac6ebcca56ce961df2d3f33f0c..70071aaad8457e6be1cfaecec7da36bb9f31eaf1 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -932,9 +932,22 @@ def resolve_record(nameserver, query, rtype="SOA", retry=True, timeout=100):
 time.sleep(1)
 
 
+def ipa_backup(master):
+result = master.run_command(["ipa-backup"])
+myre = re.compile(".*Backed up to (?P.*?)\n.*")
+matched = myre.search(result.stdout_text + result.stderr_text)
+return matched.group("backup")
+
+
+def ipa_restore(master, backup_path):
+master.run_command(["ipa-restore", "-U",
+"-p", master.config.dirman_password,
+backup_path])
+
+
 def install_kra(host, domain_level=None, first_instance=False, raiseonerr=True):
-if not domain_level:
-   domain_level = domainlevel(host)
+if domain_level is None:
+domain_level = domainlevel(host)
 command = ["ipa-kra-install", "-U", "-p", host.config.dirman_password]
 if domain_level == DOMAIN_LEVEL_0 and not first_instance:
 replica_file = get_replica_filename(host)
@@ -943,8 +956,8 @@ def install_kra(host, domain_level=None, first_instance=False, raiseonerr=True):
 
 
 def install_ca(host, domain_level=None, first_instance=False, raiseonerr=True):
-if not domain_level:
-   domain_level = domainlevel(host)
+if domain_level is None:
+domain_level = domainlevel(host)
 command = ["ipa-ca-install", "-U", "-p", host.config.dirman_password,
"-P", 'admin', "-w", host.config.admin_password]
 if domain_level == DOMAIN_LEVEL_0 and not first_instance:
@@ -961,3 +974,10 @@ def install_dns(host, raiseonerr=True):
 "-U",
 ]
 return host.run_command(args, raiseonerr=raiseonerr)
+
+
+def uninstall_replica(master, replica):
+master.run_command(["ipa-replica-manage", "del", "--force",
+"-p", master.config.dirman_password,
+replica.hostname], raiseonerr=False)
+uninstall_master(replica)
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code