Re: [Freeipa-devel] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Tomas Babej 


On 12/14/2015 02:41 PM, Jan Cholasta wrote:
> On 14.12.2015 14:20, Tomas Babej wrote:
>>
>>
>> On 12/14/2015 12:57 PM, Jan Cholasta wrote:
>>> On 14.12.2015 12:54, Tomas Babej wrote:


 On 12/14/2015 12:52 PM, Jan Cholasta wrote:
> Hi,
>
> the attached patches fix
> .
>
> Honza
>
>
>

 Shouldn't skip_output be also marked as incompatible with
 redirect_output?
>>>
>>> Yes, it should.
>>>
>>
>> Otherwise the patch works fine, so conditional ACK here given that the
>> additional sanity check is implemented.
> 
> Updated patches attched.
> 

Thanks, I just removed the empty space in the 'print ()' call before
pushing.

ACK, Pushed to master: c856401478ce2f4fdd9cd7192afd18704f78e2e6

Tomas

-- 
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] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Jan Cholasta 

On 14.12.2015 14:20, Tomas Babej wrote:



On 12/14/2015 12:57 PM, Jan Cholasta wrote:

On 14.12.2015 12:54, Tomas Babej wrote:



On 12/14/2015 12:52 PM, Jan Cholasta wrote:

Hi,

the attached patches fix .

Honza





Shouldn't skip_output be also marked as incompatible with
redirect_output?


Yes, it should.



Otherwise the patch works fine, so conditional ACK here given that the
additional sanity check is implemented.


Updated patches attched.

--
Jan Cholasta
From fa6e8f530b7e436e975c81f23503a78d32d5fe30 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 14 Dec 2015 12:45:45 +0100
Subject: [PATCH 1/2] ipautil: allow redirecting command output to standard
 output in run()

https://fedorahosted.org/freeipa/ticket/5527
---
 ipapython/ipautil.py | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4480744..160706f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -286,7 +286,7 @@ class _RunResult(collections.namedtuple('_RunResult',
 def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 capture_output=False, skip_output=False, cwd=None,
 runas=None, timeout=None, suplementary_groups=[],
-capture_error=False, encoding=None):
+capture_error=False, encoding=None, redirect_output=False):
 """
 Execute an external command.
 
@@ -323,6 +323,7 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 :param encoding: For Python 3, the encoding to use for output,
 error_output, and (if it's not bytes) stdin.
 If None, the current encoding according to locale is used.
+:param redirect_output: Redirect (error) output to standard (error) output.
 
 :return: An object with these attributes:
 
@@ -362,6 +363,13 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 raise ValueError('skip_output is incompatible with '
  'capture_output or capture_error')
 
+if redirect_output and (capture_output or capture_error):
+raise ValueError('redirect_output is incompatible with '
+ 'capture_output or capture_error')
+
+if skip_output and redirect_output:
+raise ValueError('skip_output is incompatible with redirect_output')
+
 if env is None:
 # copy default env
 env = copy.deepcopy(os.environ)
@@ -370,6 +378,9 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 p_in = subprocess.PIPE
 if skip_output:
 p_out = p_err = open(paths.DEV_NULL, 'w')
+elif redirect_output:
+p_out = sys.stdout
+p_err = sys.stderr
 else:
 p_out = subprocess.PIPE
 p_err = subprocess.PIPE
@@ -432,7 +443,7 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 
 # The command and its output may include passwords that we don't want
 # to log. Replace those.
-if skip_output:
+if skip_output or redirect_output:
 output_log = None
 error_log = None
 else:
-- 
2.4.3

From 21b5b0391b3eafdf0baf4b80562a0571f995b171 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 14 Dec 2015 12:46:48 +0100
Subject: [PATCH 2/2] server install: redirect ipa-client-install output to
 standard output

https://fedorahosted.org/freeipa/ticket/5527
---
 ipaserver/install/server/install.py| 13 ++---
 ipaserver/install/server/replicainstall.py | 18 +-
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 540ce20..a07ca66 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -991,10 +991,10 @@ def install(installer):
 args.append("--no-sshd")
 if options.mkhomedir:
 args.append("--mkhomedir")
-run(args)
+run(args, redirect_output=True)
+print()
 except Exception as e:
-sys.exit("Configuration of client side components failed!\n"
- "ipa-client-install returned: " + str(e))
+sys.exit("Configuration of client side components failed!")
 
 # Everything installed properly, activate ipa service.
 services.knownservices.ipa.enable()
@@ -1251,13 +1251,12 @@ def uninstall(installer):
 try:
 result = run([paths.IPA_CLIENT_INSTALL, "--on-master",
   "--unattended", "--uninstall"],
- raiseonerr=False, capture_output=True)
+ raiseonerr=False, redirect_output=True)
 if result.returncode not in [0, 2]:
-raise RuntimeError(result.output)
-except Exception as e:
+raise RuntimeError("Failed to configure the client")
+except Exception:
 rv = 1
 print("Uninstall of client side components failed!")
-print("ipa-client-install returned: " + str(e))

Re: [Freeipa-devel] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Tomas Babej 


On 12/14/2015 12:57 PM, Jan Cholasta wrote:
> On 14.12.2015 12:54, Tomas Babej wrote:
>>
>>
>> On 12/14/2015 12:52 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patches fix .
>>>
>>> Honza
>>>
>>>
>>>
>>
>> Shouldn't skip_output be also marked as incompatible with
>> redirect_output?
> 
> Yes, it should.
> 

Otherwise the patch works fine, so conditional ACK here given that the
additional sanity check is implemented.

Tomas

-- 
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] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Jan Cholasta 

On 14.12.2015 12:54, Tomas Babej wrote:



On 12/14/2015 12:52 PM, Jan Cholasta wrote:

Hi,

the attached patches fix .

Honza





Shouldn't skip_output be also marked as incompatible with redirect_output?


Yes, it should.

--
Jan Cholasta

--
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] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Tomas Babej 


On 12/14/2015 12:52 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patches fix .
> 
> Honza
> 
> 
> 

Shouldn't skip_output be also marked as incompatible with redirect_output?

Tomas

-- 
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] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

2015-12-14 Thread Jan Cholasta 

Hi,

the attached patches fix .

Honza

--
Jan Cholasta
From 8ee7ad0e87b1df062fbf253170bfffcb7b3bfb85 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 14 Dec 2015 12:45:45 +0100
Subject: [PATCH 1/2] ipautil: allow redirecting command output to standard
 output in run()

https://fedorahosted.org/freeipa/ticket/5527
---
 ipapython/ipautil.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4480744..31dad34 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -286,7 +286,7 @@ class _RunResult(collections.namedtuple('_RunResult',
 def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 capture_output=False, skip_output=False, cwd=None,
 runas=None, timeout=None, suplementary_groups=[],
-capture_error=False, encoding=None):
+capture_error=False, encoding=None, redirect_output=False):
 """
 Execute an external command.
 
@@ -323,6 +323,7 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 :param encoding: For Python 3, the encoding to use for output,
 error_output, and (if it's not bytes) stdin.
 If None, the current encoding according to locale is used.
+:param redirect_output: Redirect (error) output to standard (error) output.
 
 :return: An object with these attributes:
 
@@ -362,6 +363,10 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 raise ValueError('skip_output is incompatible with '
  'capture_output or capture_error')
 
+if redirect_output and (capture_output or capture_error):
+raise ValueError('redirect_output is incompatible with '
+ 'capture_output or capture_error')
+
 if env is None:
 # copy default env
 env = copy.deepcopy(os.environ)
@@ -370,6 +375,9 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 p_in = subprocess.PIPE
 if skip_output:
 p_out = p_err = open(paths.DEV_NULL, 'w')
+elif redirect_output:
+p_out = sys.stdout
+p_err = sys.stderr
 else:
 p_out = subprocess.PIPE
 p_err = subprocess.PIPE
@@ -432,7 +440,7 @@ def run(args, stdin=None, raiseonerr=True, nolog=(), env=None,
 
 # The command and its output may include passwords that we don't want
 # to log. Replace those.
-if skip_output:
+if skip_output or redirect_output:
 output_log = None
 error_log = None
 else:
-- 
2.4.3

From d75712df78099d71d3cf245f96bac35c008c3beb Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 14 Dec 2015 12:46:48 +0100
Subject: [PATCH 2/2] server install: redirect ipa-client-install output to
 standard output

https://fedorahosted.org/freeipa/ticket/5527
---
 ipaserver/install/server/install.py| 12 +---
 ipaserver/install/server/replicainstall.py | 14 ++
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 540ce20..1791832 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -991,10 +991,9 @@ def install(installer):
 args.append("--no-sshd")
 if options.mkhomedir:
 args.append("--mkhomedir")
-run(args)
+run(args, redirect_output=True)
 except Exception as e:
-sys.exit("Configuration of client side components failed!\n"
- "ipa-client-install returned: " + str(e))
+sys.exit("Configuration of client side components failed!")
 
 # Everything installed properly, activate ipa service.
 services.knownservices.ipa.enable()
@@ -1251,13 +1250,12 @@ def uninstall(installer):
 try:
 result = run([paths.IPA_CLIENT_INSTALL, "--on-master",
   "--unattended", "--uninstall"],
- raiseonerr=False, capture_output=True)
+ raiseonerr=False, redirect_output=True)
 if result.returncode not in [0, 2]:
-raise RuntimeError(result.output)
-except Exception as e:
+raise RuntimeError("Failed to configure the client")
+except Exception:
 rv = 1
 print("Uninstall of client side components failed!")
-print("ipa-client-install returned: " + str(e))
 
 sys.exit(rv)
 
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 311f0e5..6d2589f 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -409,7 +409,7 @@ def uninstall_client():
 
 print("Removing client side components")
 ipautil.run([paths.IPA_CLIENT_INSTALL, "--unattended", "--uninstall"],
-raiseonerr=False)
+raiseonerr=False, redirect_output=True)
 
 
 def promote_sssd(host_name):
@@ -794,10 +794,9 @@