Re: [Freeipa-devel] Travis CI unexpected PEP8 errors

2016-12-13 Thread Standa Laznicka

On 12/14/2016 02:53 AM, Ben Lipton wrote:

Hi all,

I'm pretty sure this is unrelated to the CI issues discussed in other 
threads recently, but they reminded me that I've been having this odd 
issue.


https://travis-ci.org/freeipa/freeipa/jobs/183756995 is the most 
recent run on my pull request, 
https://github.com/freeipa/freeipa/pull/10. For a while now, every 
time the CI runs on my PR, it fails due to several PEP8 errors that 
are not detected when I run `git diff master -U0 | pep8 --diff` on my 
local copy. In fact, the errors are all in files not touched by my PR, 
which doesn't make much sense to me based on the behavior of `git diff`.


I noticed that the top of the travis build says:

 - Commit 1f50550
 - #10: Client-side CSR autogeneration
 - Branch master

I'm not sure what the "commit" link actually means, but that commit 
seems to have nothing to do with my PR nor the current master. Could 
Travis be diffing against the wrong code? Or if not, do you have any 
idea what might be causing these failures?


Thanks,
Ben


Hi Ben,

I was going through the Travis CI log of and noticed what might have 
caused the issue:


$ cd freeipa/freeipa
$ git fetch origin +refs/pull/109/merge:

It seems that for your pull request #10 (and for some reason for your 
pull request only), Travis fetched a different (old) pull request which 
it then tried to merge with current master, hence the errors. I don't 
think it was testing your code at all.


I do not have an explanation for this other than it might be a Travis 
bug, CCing Martin for a better answer.


Standa
-- 
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] Travis CI unexpected PEP8 errors

2016-12-13 Thread Ben Lipton

Hi all,

I'm pretty sure this is unrelated to the CI issues discussed in other 
threads recently, but they reminded me that I've been having this odd issue.


https://travis-ci.org/freeipa/freeipa/jobs/183756995 is the most recent 
run on my pull request, https://github.com/freeipa/freeipa/pull/10. For 
a while now, every time the CI runs on my PR, it fails due to several 
PEP8 errors that are not detected when I run `git diff master -U0 | pep8 
--diff` on my local copy. In fact, the errors are all in files not 
touched by my PR, which doesn't make much sense to me based on the 
behavior of `git diff`.


I noticed that the top of the travis build says:

 - Commit 1f50550
 - #10: Client-side CSR autogeneration
 - Branch master

I'm not sure what the "commit" link actually means, but that commit 
seems to have nothing to do with my PR nor the current master. Could 
Travis be diffing against the wrong code? Or if not, do you have any 
idea what might be causing these failures?


Thanks,
Ben

--
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] [freeipa PR#245][comment] Allow full customisability of IPA CA subject DN

2016-12-13 Thread frasertweedale
  URL: https://github.com/freeipa/freeipa/pull/245
Title: #245: Allow full customisability of IPA CA subject DN

frasertweedale commented:
"""
@jcholast: new tickets pertaining to subject_base / certmap.conf config:

- **do not update ipaCertificateSubjectBase and certmap.conf in CA-less mode** 
  - https://fedorahosted.org/freeipa/ticket/6556
- **do not set (or look up) subject_base in sysupgrade file**
  - https://fedorahosted.org/freeipa/ticket/6557

Other review comments will be addressed in due course.  Thanks for reviewing.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/245#issuecomment-266910282
-- 
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] Travis CI broke after merging PR 177

2016-12-13 Thread Martin Babinsky

On 12/13/2016 09:41 AM, Martin Babinsky wrote:

Hi list,

https://github.com/freeipa/freeipa/pull/177 was recently merged despite
causing nearly half of the tests in our Travis CI gating to fail. This
broke Travis CI for all other PR that were rebased after this merge,
causing false negative errors everywhere.

Fraser reverted the offending commits in
https://github.com/freeipa/freeipa/pull/329 which restored Travis to
original state (never mind PEP8 errors they were in the original code
already).

Regarding this issues I have two questions:

a)

should I merge https://github.com/freeipa/freeipa/pull/329 and thus
revert the breakage in order to unblock other contributors? Given the
current traffic I think it is sufficient to wait for us to investigate
and produce a fix. If not, please scream loudly.

b)

what can we improve to make the results of CI more visible to
contributors? I think that I should sit down with Martin 2 and
investigate the possibility to send notifications about negative CI
results (sufficient IMO) to the mailing list.

In the meanwhile I would like to ask all reviewers to carefully check
the output of failed Travis CI runs. If the job fails, you will see the
results at the very end of the log. There are two sections: PEP8 errors
and test output. You can expand both of them to see what went wrong and
report it to the PR author if necessary.

The reviewer and author can then use the very same tool used in CI [1]
to reproduce the failures locally. Using  '--no-cleanup' option during
the run [2] leaves behind a running container which you can attach to
and investigate further.

[1] https://github.com/freeipa/ipa-docker-test-runner
[2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md

If you have any additional questions/suggestions about Travis feel free
to contact me.



The PR fixing failing Travis CI wss merged to master now.

If you rebase and sync your branches the checks should be green again 
(well unless you broke something in your PR but you can not blame the 
regression for that anymore ;)).


--
Martin^3 Babinsky

--
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] [freeipa PR#332][closed] Fix regression in test suite

2016-12-13 Thread martbab
   URL: https://github.com/freeipa/freeipa/pull/332
Author: frasertweedale
 Title: #332: Fix regression in test suite
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/332/head:pr332
git checkout pr332
-- 
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] [freeipa PR#332][+pushed] Fix regression in test suite

2016-12-13 Thread martbab
  URL: https://github.com/freeipa/freeipa/pull/332
Title: #332: Fix regression in test suite

Label: +pushed
-- 
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] [freeipa PR#332][comment] Fix regression in test suite

2016-12-13 Thread martbab
  URL: https://github.com/freeipa/freeipa/pull/332
Title: #332: Fix regression in test suite

martbab commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/74b8cf2c4a8dd36577d76c35a9ef08352ef025b7
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/332#issuecomment-266785937
-- 
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] [freeipa PR#332][+ack] Fix regression in test suite

2016-12-13 Thread martbab
  URL: https://github.com/freeipa/freeipa/pull/332
Title: #332: Fix regression in test suite

Label: +ack
-- 
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] [freeipa PR#332][comment] Fix regression in test suite

2016-12-13 Thread martbab
  URL: https://github.com/freeipa/freeipa/pull/332
Title: #332: Fix regression in test suite

martbab commented:
"""
Thanks for pinpointing the issue, looks like Travis is happy with your fix.  
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/332#issuecomment-266779010
-- 
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] [freeipa PR#333][opened] Remove named-pkcs11 workarounds from DNSSEC tests.

2016-12-13 Thread pspacek
   URL: https://github.com/freeipa/freeipa/pull/333
Author: pspacek
 Title: #333: Remove named-pkcs11 workarounds from DNSSEC tests.
Action: opened

PR body:
"""
As far as I can tell the tests are passing for some time in Jenkins so
maybe a bug in some underlying component was fixed. Let's remove
workarounds to make tests actually test real setups.

https://fedorahosted.org/freeipa/ticket/5348
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/333/head:pr333
git checkout pr333
From 3c1cf308bc462641e5990c1a3a983f13fb8e9706 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 13 Dec 2016 16:43:52 +0100
Subject: [PATCH] Remove named-pkcs11 workarounds from DNSSEC tests.

As far as I can tell the tests are passing for some time in Jenkins so
maybe a bug in some underlying component was fixed. Let's remove
workarounds to make tests actually test real setups.

https://fedorahosted.org/freeipa/ticket/5348
---
 ipatests/test_integration/test_dnssec.py | 81 
 1 file changed, 81 deletions(-)

diff --git a/ipatests/test_integration/test_dnssec.py b/ipatests/test_integration/test_dnssec.py
index 56380dd..e0d5bcf 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -106,7 +106,6 @@ def test_if_zone_is_signed_master(self):
 ]
 self.master.run_command(args)
 
-tasks.restart_named(self.master, self.replicas[0])
 # test master
 assert wait_until_record_is_signed(
 self.master.ip, test_zone, self.log, timeout=100
@@ -127,7 +126,6 @@ def test_if_zone_is_signed_replica(self):
 ]
 self.replicas[0].run_command(args)
 
-tasks.restart_named(self.replicas[0])
 # test replica
 assert wait_until_record_is_signed(
 self.replicas[0].ip, test_zone_repl, self.log, timeout=300
@@ -173,7 +171,6 @@ def test_disable_reenable_signing_master(self):
 ]
 self.master.run_command(args)
 
-tasks.restart_named(self.master)
 # test master
 assert wait_until_record_is_signed(
 self.master.ip, test_zone, self.log, timeout=100
@@ -221,8 +218,6 @@ def test_disable_reenable_signing_replica(self):
 ]
 self.master.run_command(args)
 
-tasks.restart_named(self.master, self.replicas[0])
-
 # test master
 assert wait_until_record_is_signed(
 self.master.ip, test_zone_repl, self.log, timeout=100
@@ -238,77 +233,6 @@ def test_disable_reenable_signing_replica(self):
 assert dnskey_old != dnskey_new, "DNSKEY should be different"
 
 
-class TestZoneSigningWithoutNamedRestart(IntegrationTest):
-"""Test whether https://fedorahosted.org/freeipa/ticket/5348 is already
-fixed. If the issue is not fixed, the test will expectedly fail. When
-fixed, it will pass, which will cause the whole run to become "red"
-"""
-num_replicas = 1
-topology = 'star'
-
-@classmethod
-def install(cls, mh):
-tasks.install_master(cls.master, setup_dns=False)
-args = [
-"ipa-dns-install",
-"--dnssec-master",
-"--forwarder", cls.master.config.dns_forwarder,
-"-U",
-]
-cls.master.run_command(args)
-
-tasks.install_replica(cls.master, cls.replicas[0], setup_dns=True)
-
-# backup trusted key
-tasks.backup_file(cls.master, paths.DNSSEC_TRUSTED_KEY)
-tasks.backup_file(cls.replicas[0], paths.DNSSEC_TRUSTED_KEY)
-
-@classmethod
-def uninstall(cls, mh):
-# restore trusted key
-tasks.restore_files(cls.master)
-tasks.restore_files(cls.replicas[0])
-
-super(TestZoneSigningWithoutNamedRestart, cls).uninstall(mh)
-
-@pytest.mark.xfail(strict=True)
-def test_sign_root_zone_no_named_restart(self):
-args = [
-"ipa", "dnszone-add", root_zone, "--dnssec", "true",
-"--skip-overlap-check",
-]
-self.master.run_command(args)
-
-# make BIND happy: add the glue record and delegate zone
-args = [
-"ipa", "dnsrecord-add", root_zone, self.master.hostname,
-"--a-rec=" + self.master.ip
-]
-self.master.run_command(args)
-args = [
-"ipa", "dnsrecord-add", root_zone, self.replicas[0].hostname,
-"--a-rec=" + self.replicas[0].ip
-]
-self.master.run_command(args)
-
-time.sleep(10)  # sleep a bit until data are provided by bind-dyndb-ldap
-
-args = [
-"ipa", "dnsrecord-add", root_zone, self.master.domain.name,
-"--ns-rec=" + self.master.hostname
-]
-self.master.run_command(args)
-# test master
-assert wait_until_record_is_signed(
-self.master.ip, root_zone, self.log, timeout=100
-), "Zone %s is not signed (master)" % 

[Freeipa-devel] [freeipa PR#332][synchronized] Fix regression in test suite

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/332
Author: frasertweedale
 Title: #332: Fix regression in test suite
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/332/head:pr332
git checkout pr332
From caf1836023fe8128d54e781a949d752516164402 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 14 Dec 2016 00:22:56 +1000
Subject: [PATCH] Fix regression in test suite

32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d introduced a regression in
test_serverroles.py, caused by ca_find attempting to log into the
Dogtag REST API.  (ca_find is called by cert_find which is called by
server_del during cleanup).

Avoid logging into Dogtag in cert_find unless something actually
needs to be retrieved.

Fixes: https://fedorahosted.org/freeipa/ticket/6178
---
 ipaserver/plugins/ca.py | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index ef1d68c..2510a79 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -161,15 +161,21 @@ class ca(LDAPObject):
 }
 
 
-def set_certificate_attrs(entry, options, always_include_cert=True):
+def set_certificate_attrs(entry, options, want_cert=True):
 ca_id = entry['ipacaid'][0]
 full = options.get('all', False)
+want_chain = options.get('chain', False)
+
+want_data = want_cert or want_chain or full
+if not want_data:
+return
+
 with api.Backend.ra_lightweight_ca as ca_api:
-if always_include_cert or full:
+if want_cert or full:
 der = ca_api.read_ca_cert(ca_id)
 entry['certificate'] = six.text_type(base64.b64encode(der))
 
-if options.get('chain', False) or full:
+if want_chain or full:
 pkcs7_der = ca_api.read_ca_chain(ca_id)
 pems = x509.pkcs7_to_pems(pkcs7_der, x509.DER)
 ders = [x509.normalize_certificate(pem) for pem in pems]
@@ -187,7 +193,7 @@ def execute(self, *keys, **options):
 ca_enabled_check()
 result = super(ca_find, self).execute(*keys, **options)
 for entry in result['result']:
-set_certificate_attrs(entry, options, always_include_cert=False)
+set_certificate_attrs(entry, options, want_cert=False)
 return result
 
 
-- 
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] [freeipa PR#332][opened] Fix regression in test suite

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/332
Author: frasertweedale
 Title: #332: Fix regression in test suite
Action: opened

PR body:
"""
32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d introduced a regression in
test_serverroles.py, caused by ca_find attempting to log into the
Dogtag REST API.  (ca_find is called by cert_find which is caused by
server_del during cleanup).

Avoid logging into Dogtag in cert_find unless something actually
needs to be retrieved.

Fixes: https://fedorahosted.org/freeipa/ticket/6178
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/332/head:pr332
git checkout pr332
From 19a63ecd713b5133dbd5ee6ba65d4351799cebaa Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 14 Dec 2016 00:22:56 +1000
Subject: [PATCH] Fix regression in test suite

32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d introduced a regression in
test_serverroles.py, caused by ca_find attempting to log into the
Dogtag REST API.  (ca_find is called by cert_find which is caused by
server_del during cleanup).

Avoid logging into Dogtag in cert_find unless something actually
needs to be retrieved.

Fixes: https://fedorahosted.org/freeipa/ticket/6178
---
 ipaserver/plugins/ca.py | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index ef1d68c..86bec0f 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -161,15 +161,21 @@ class ca(LDAPObject):
 }
 
 
-def set_certificate_attrs(entry, options, always_include_cert=True):
+def set_certificate_attrs(entry, options, want_cert=True):
 ca_id = entry['ipacaid'][0]
 full = options.get('all', False)
+want_chain = options.get('chain', False)
+
+want_data = want_cert or want_chain or full
+if not want_data:
+return
+
 with api.Backend.ra_lightweight_ca as ca_api:
 if always_include_cert or full:
 der = ca_api.read_ca_cert(ca_id)
 entry['certificate'] = six.text_type(base64.b64encode(der))
 
-if options.get('chain', False) or full:
+if want_chain or full:
 pkcs7_der = ca_api.read_ca_chain(ca_id)
 pems = x509.pkcs7_to_pems(pkcs7_der, x509.DER)
 ders = [x509.normalize_certificate(pem) for pem in pems]
@@ -187,7 +193,7 @@ def execute(self, *keys, **options):
 ca_enabled_check()
 result = super(ca_find, self).execute(*keys, **options)
 for entry in result['result']:
-set_certificate_attrs(entry, options, always_include_cert=False)
+set_certificate_attrs(entry, options, want_cert=False)
 return result
 
 
-- 
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] [freeipa PR#329][closed] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
-- 
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] [freeipa PR#272][closed] Build: makerpms.sh generates Python 2 & 3 packages at the same time

2016-12-13 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/272
Author: pspacek
 Title: #272: Build: makerpms.sh generates Python 2 & 3 packages at the same 
time
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/272/head:pr272
git checkout pr272
-- 
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] [freeipa PR#272][comment] Build: makerpms.sh generates Python 2 & 3 packages at the same time

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/272
Title: #272: Build: makerpms.sh generates Python 2 & 3 packages at the same time

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/b7d70baee73c64898de91e2fa59b3f9f417c8e01
https://fedorahosted.org/freeipa/changeset/21a0987601dc4fa9de3fe63a18a604337a5edb7b
https://fedorahosted.org/freeipa/changeset/9c87c39e65547004031284080749bc66dab1a8f9
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/272#issuecomment-266739980
-- 
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] [freeipa PR#272][+pushed] Build: makerpms.sh generates Python 2 & 3 packages at the same time

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/272
Title: #272: Build: makerpms.sh generates Python 2 & 3 packages at the same time

Label: +pushed
-- 
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] [freeipa PR#272][+ack] Build: makerpms.sh generates Python 2 & 3 packages at the same time

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/272
Title: #272: Build: makerpms.sh generates Python 2 & 3 packages at the same time

Label: +ack
-- 
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] [freeipa PR#328][comment] fix: regression in API version comparison

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/328
Title: #328: fix: regression in API version comparison

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/0663faf2585be4af3501965934703da0ede41610
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/328#issuecomment-266731072
-- 
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] [freeipa PR#328][+pushed] fix: regression in API version comparison

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/328
Title: #328: fix: regression in API version comparison

Label: +pushed
-- 
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] [freeipa PR#328][closed] fix: regression in API version comparison

2016-12-13 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/328
Author: mbasti-rh
 Title: #328: fix: regression in API version comparison
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/328/head:pr328
git checkout pr328
-- 
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
From dcd48155a899a14cdf1de843fa729064ba06b4b7 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 20:24:30 +1000
Subject: [PATCH 1/3] ci: run tests with a single job instead of two

---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e870213..2a409f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,7 +5,6 @@ env:
 global:
 - TEST_RUNNER_IMAGE="martbab/freeipa-fedora-test-runner:master-latest"
 matrix:
-- TESTS_TO_RUN="test_xmlrpc/test_[a-k]*.py"
 - >
 TESTS_TO_RUN="test_cmdline
 test_install
@@ -13,7 +12,7 @@ env:
 test_ipapython
 test_ipaserver
 test_pkcs10
-test_xmlrpc/test_[l-z]*.py"
+test_xmlrpc/test_[a-z]*.py"
 before_install:
 - pip install pep8
 - >

From 4f29cd26fccb508538598da9dac96b12a3317aee Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 22:11:07 +1000
Subject: [PATCH 2/3] ci: make travis wait 120 mins (experimental)

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 2a409f2..b574e86 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -30,7 +30,7 @@ script:
 # output do not cause premature termination of the build
 - "docker pull ${TEST_RUNNER_IMAGE}"
 - >
-travis_wait 50
+travis_wait 120
 ipa-docker-test-runner -l ci_results_${TRAVIS_BRANCH}.log
 -c .test_runner_config.yaml
 --container-image ${TEST_RUNNER_IMAGE}

From eeac7bb902edd6aea9cb6502b9779ff82e30d4c5 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 22:46:17 +1000
Subject: [PATCH 3/3] gimme more log

---
 .travis.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index b574e86..2e83511 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,6 +38,7 @@ script:
 run-tests $test_set
 after_failure:
   - echo "Test runner output:"
-  - tail -n 5000 ci_results_${TRAVIS_BRANCH}.log
+  - cat ci_results_${TRAVIS_BRANCH}.log
+  - cat /var/log/httpd/error_log
   - echo "PEP-8 errors:"
   - cat pep8_errors.log
-- 
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] [freeipa PR#328][+ack] fix: regression in API version comparison

2016-12-13 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/328
Title: #328: fix: regression in API version comparison

Label: +ack
-- 
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] Travis CI broke after merging PR 177

2016-12-13 Thread Martin Babinsky

On 12/13/2016 01:41 PM, Fraser Tweedale wrote:

On Tue, Dec 13, 2016 at 01:11:37PM +0100, Martin Babinsky wrote:

On 12/13/2016 01:07 PM, Fraser Tweedale wrote:

On Tue, Dec 13, 2016 at 09:41:40AM +0100, Martin Babinsky wrote:

Hi list,

https://github.com/freeipa/freeipa/pull/177 was recently merged despite
causing nearly half of the tests in our Travis CI gating to fail. This broke
Travis CI for all other PR that were rebased after this merge, causing false
negative errors everywhere.

Fraser reverted the offending commits in
https://github.com/freeipa/freeipa/pull/329 which restored Travis to
original state (never mind PEP8 errors they were in the original code
already).

Regarding this issues I have two questions:

a)

should I merge https://github.com/freeipa/freeipa/pull/329 and thus revert
the breakage in order to unblock other contributors? Given the current
traffic I think it is sufficient to wait for us to investigate and produce a
fix. If not, please scream loudly.


Definitely not, I am using this PR as a playground to poke the CI in
various ways.  Also, proper fix would be best :)  See also my other
mail (I would have replied here but fetchmail was not fetching mail
and I missed it until just now >_<).


b)

what can we improve to make the results of CI more visible to contributors?
I think that I should sit down with Martin 2 and investigate the possibility
to send notifications about negative CI results (sufficient IMO) to the
mailing list.


Or the commit author.

It would be *great* if the test job, in event of failure, would
collect all the obvious logs and dump them somewhere as artifacts.



That would be great but AFAIK Travis CI does support only archiving
artifacts into a running AWS instance[1] which we do not have and would have
to buy, run and maintain ourselves. For now we have to inspect the log dumps
generated in the job output.


Do the containers not have internet access?  That is all you really
need, surely.

The log dumps are not enough... `tail -n 5000' is not enough :)

Even if there is no other way besides S3, it might still be worth
it.



Travis imposes hard 10k line limit in the job logs that's why I had to 
truncate them, we have discussed this issue ad nauseum. And AFAIK the 
workers are not accessible from outside.



In the meanwhile I would like to ask all reviewers to carefully check the
output of failed Travis CI runs. If the job fails, you will see the results
at the very end of the log. There are two sections: PEP8 errors and test
output. You can expand both of them to see what went wrong and report it to
the PR author if necessary.

The reviewer and author can then use the very same tool used in CI [1] to
reproduce the failures locally. Using  '--no-cleanup' option during the run
[2] leaves behind a running container which you can attach to and
investigate further.

[1] https://github.com/freeipa/ipa-docker-test-runner
[2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md

If you have any additional questions/suggestions about Travis feel free to
contact me.

--
Martin^3 Babinsky

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


[1] https://docs.travis-ci.com/user/uploading-artifacts/

--
Martin^3 Babinsky



--
Martin^3 Babinsky

--
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] Travis CI broke after merging PR 177

2016-12-13 Thread Fraser Tweedale
On Tue, Dec 13, 2016 at 01:11:37PM +0100, Martin Babinsky wrote:
> On 12/13/2016 01:07 PM, Fraser Tweedale wrote:
> > On Tue, Dec 13, 2016 at 09:41:40AM +0100, Martin Babinsky wrote:
> > > Hi list,
> > > 
> > > https://github.com/freeipa/freeipa/pull/177 was recently merged despite
> > > causing nearly half of the tests in our Travis CI gating to fail. This 
> > > broke
> > > Travis CI for all other PR that were rebased after this merge, causing 
> > > false
> > > negative errors everywhere.
> > > 
> > > Fraser reverted the offending commits in
> > > https://github.com/freeipa/freeipa/pull/329 which restored Travis to
> > > original state (never mind PEP8 errors they were in the original code
> > > already).
> > > 
> > > Regarding this issues I have two questions:
> > > 
> > > a)
> > > 
> > > should I merge https://github.com/freeipa/freeipa/pull/329 and thus revert
> > > the breakage in order to unblock other contributors? Given the current
> > > traffic I think it is sufficient to wait for us to investigate and 
> > > produce a
> > > fix. If not, please scream loudly.
> > > 
> > Definitely not, I am using this PR as a playground to poke the CI in
> > various ways.  Also, proper fix would be best :)  See also my other
> > mail (I would have replied here but fetchmail was not fetching mail
> > and I missed it until just now >_<).
> > 
> > > b)
> > > 
> > > what can we improve to make the results of CI more visible to 
> > > contributors?
> > > I think that I should sit down with Martin 2 and investigate the 
> > > possibility
> > > to send notifications about negative CI results (sufficient IMO) to the
> > > mailing list.
> > > 
> > Or the commit author.
> > 
> > It would be *great* if the test job, in event of failure, would
> > collect all the obvious logs and dump them somewhere as artifacts.
> > 
> 
> That would be great but AFAIK Travis CI does support only archiving
> artifacts into a running AWS instance[1] which we do not have and would have
> to buy, run and maintain ourselves. For now we have to inspect the log dumps
> generated in the job output.
> 
Do the containers not have internet access?  That is all you really
need, surely.

The log dumps are not enough... `tail -n 5000' is not enough :)

Even if there is no other way besides S3, it might still be worth
it.

> > > In the meanwhile I would like to ask all reviewers to carefully check the
> > > output of failed Travis CI runs. If the job fails, you will see the 
> > > results
> > > at the very end of the log. There are two sections: PEP8 errors and test
> > > output. You can expand both of them to see what went wrong and report it 
> > > to
> > > the PR author if necessary.
> > > 
> > > The reviewer and author can then use the very same tool used in CI [1] to
> > > reproduce the failures locally. Using  '--no-cleanup' option during the 
> > > run
> > > [2] leaves behind a running container which you can attach to and
> > > investigate further.
> > > 
> > > [1] https://github.com/freeipa/ipa-docker-test-runner
> > > [2] 
> > > https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md
> > > 
> > > If you have any additional questions/suggestions about Travis feel free to
> > > contact me.
> > > 
> > > --
> > > Martin^3 Babinsky
> > > 
> > > --
> > > 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
> 
> [1] https://docs.travis-ci.com/user/uploading-artifacts/
> 
> -- 
> Martin^3 Babinsky

-- 
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] [freeipa PR#331][opened] WebUI: don't change casing of Auth Indicators values

2016-12-13 Thread pvomacka
   URL: https://github.com/freeipa/freeipa/pull/331
Author: pvomacka
 Title: #331: WebUI: don't change casing of Auth Indicators values
Action: opened

PR body:
"""
All values were previously converted to lowercase which was not
coresponding with CLI behaviour. Now they stay as they are
inserted. I also have to change the strings to lowercase because
the otp and radius should be inserted as lowercase words.

https://fedorahosted.org/freeipa/ticket/6308

"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/331/head:pr331
git checkout pr331
From a0dd61fdd6f04a2e1079e9e9b1996c0547bb1742 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 13 Dec 2016 13:21:29 +0100
Subject: [PATCH 1/2] WebUI: Allow disabling lowering text in
 custom_checkbox_widget

Add new attribute which keeps information whether each text added
using custom_checkbox_widget shoud be transformed to lowercase.

Part of: https://fedorahosted.org/freeipa/ticket/6308
---
 install/ui/src/freeipa/widget.js | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 041eaa2..7965d9f 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -2509,6 +2509,9 @@ IPA.custom_checkboxes_widget = function(spec) {
 
 var that = IPA.checkboxes_widget(spec);
 
+that.set_value_to_lowercase = spec.set_value_to_lowercase === undefined
+? true : spec.set_value_to_lowercase;
+
 that.add_dialog_title = spec.add_dialog_title ||
 "@i18n:dialogs.add_custom_value";
 that.add_field_label = spec.add_field_label ||
@@ -2626,7 +2629,7 @@ IPA.custom_checkboxes_widget = function(spec) {
 
 if (!value || value === '') continue;
 
-value = value.toLowerCase();
+if (that.set_value_to_lowercase) value = value.toLowerCase();
 that.values.push(value);
 }
 

From e40d717ebfc8dac544d646951b22f3747ff2aad4 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Tue, 13 Dec 2016 13:25:48 +0100
Subject: [PATCH 2/2] WebUI: don't change casing of Auth Indicators values

All values were previously converted to lowercase which was not
coresponding with CLI behaviour. Now they stay as they are
inserted. I also have to change the strings to lowercase because
the otp and radius should be inserted as lowercase words.

https://fedorahosted.org/freeipa/ticket/6308
---
 install/ui/src/freeipa/host.js| 1 +
 install/ui/src/freeipa/service.js | 5 +++--
 ipaserver/plugins/internal.py | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/install/ui/src/freeipa/host.js b/install/ui/src/freeipa/host.js
index 87cf264..a54cb8f 100644
--- a/install/ui/src/freeipa/host.js
+++ b/install/ui/src/freeipa/host.js
@@ -119,6 +119,7 @@ return {
 $type: 'custom_checkboxes',
 label: '@i18n:authtype.auth_indicators',
 name: 'krbprincipalauthind',
+set_value_to_lowercase: false,
 add_dialog_title: '@i18n:authtype.custom_auth_ind_title',
 add_field_label: '@i18n:authtype.auth_indicator',
 options: [
diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js
index a6607d2..a86205a 100644
--- a/install/ui/src/freeipa/service.js
+++ b/install/ui/src/freeipa/service.js
@@ -129,16 +129,17 @@ return {
 $type: 'custom_checkboxes',
 label: '@i18n:authtype.auth_indicators',
 name: 'krbprincipalauthind',
+set_value_to_lowercase: false,
 add_dialog_title: '@i18n:authtype.custom_auth_ind_title',
 add_field_label: '@i18n:authtype.auth_indicator',
 options: [
 {
 label: '@i18n:authtype.otp',
-value: 'otp'
+value: 'OTP'
 },
 {
 label: '@i18n:authtype.type_radius',
-value: 'radius'
+value: 'RADIUS'
 }
 ],
 tooltip: {
diff --git a/ipaserver/plugins/internal.py b/ipaserver/plugins/internal.py
index 6107a14..74c264e 100644
--- a/ipaserver/plugins/internal.py
+++ b/ipaserver/plugins/internal.py
@@ -201,10 +201,10 @@ class i18n_messages(Command):
 "auth_indicator": _("Authentication indicator"),
 "config_tooltip": _("Implicit method 

Re: [Freeipa-devel] CI failures - I need your help

2016-12-13 Thread Martin Babinsky

On 12/13/2016 12:20 PM, Fraser Tweedale wrote:

Hi all,

The CI failures caused by one of my recent commits have me baffled.
It is exactly this commit[1] at which the problems begin.  I cannot
see anything in the commit to point a finger at.  In-tree tests run
fine.

[1] 
https://github.com/freeipa/freeipa/commit/32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d

In terms of symptoms: a Travis run has two jobs, one dealing with
`test_xmlrpc/test_[a-k]*.py' and the other dealing with
`test_xmlrpc/test_[l-z]*.py' as well as `test_install/',
`test_ipalib/', `test_ipapython/', `test_ipaserver/' and
`test_pkcs10'.  See [2] for an example of a failing build: the first
job (which includes tests for the ca plugin, which is what the patch
relates to) does succeed, but the second fails catastrophically with
myriad occurences of:

E   NetworkError: cannot connect to
'https://master1.ipa.test/ipa/session/json':
(SSL_ERROR_NO_CIPHERS_SUPPORTED) No cipher suites are present
and enabled in this program.

Leading to the unfortunate outcome:

=== 254 failed, 576 passed, 98 skipped, 1756 error in 254.59 seconds 
===

[2] https://travis-ci.org/freeipa/freeipa/builds/183544973


I have been unsuccessful in running `ipa-docker-test-runner' on my
own machine due to Java thread creation problems, so I need the help
of others to analyse and fix this issue.  If someone was able to
analyse a running container that had the test failures, send logs,
or even make an image of the container and upload it somewhere for
me, it might lead to some answers.

Thanks for your help.
Fraser

(P.S. I hope it is not a trivial thing I have overlook ^_^)



Hi Fraser,

I am also playing around with this issue and have observed that when I 
run XMLRPC tests alone all is green, so the error must first occur in 
some other suite and then propagate to tests ran later. This also agrees 
with the observation that the half which does not run non-XMLRPC tests 
always passes.


I have observed that the first failure comes in 
test_ipaserver/test_serverroles suite at teardown with 
SEC_ERROR_LEGACY_DATABASE thrown from nss:

https://paste.fedoraproject.org/505681/14816316/

I will investigate further and keep you posted.\

--
Martin^3 Babinsky

--
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] Travis CI broke after merging PR 177

2016-12-13 Thread Martin Babinsky

On 12/13/2016 01:07 PM, Fraser Tweedale wrote:

On Tue, Dec 13, 2016 at 09:41:40AM +0100, Martin Babinsky wrote:

Hi list,

https://github.com/freeipa/freeipa/pull/177 was recently merged despite
causing nearly half of the tests in our Travis CI gating to fail. This broke
Travis CI for all other PR that were rebased after this merge, causing false
negative errors everywhere.

Fraser reverted the offending commits in
https://github.com/freeipa/freeipa/pull/329 which restored Travis to
original state (never mind PEP8 errors they were in the original code
already).

Regarding this issues I have two questions:

a)

should I merge https://github.com/freeipa/freeipa/pull/329 and thus revert
the breakage in order to unblock other contributors? Given the current
traffic I think it is sufficient to wait for us to investigate and produce a
fix. If not, please scream loudly.


Definitely not, I am using this PR as a playground to poke the CI in
various ways.  Also, proper fix would be best :)  See also my other
mail (I would have replied here but fetchmail was not fetching mail
and I missed it until just now >_<).


b)

what can we improve to make the results of CI more visible to contributors?
I think that I should sit down with Martin 2 and investigate the possibility
to send notifications about negative CI results (sufficient IMO) to the
mailing list.


Or the commit author.

It would be *great* if the test job, in event of failure, would
collect all the obvious logs and dump them somewhere as artifacts.



That would be great but AFAIK Travis CI does support only archiving 
artifacts into a running AWS instance[1] which we do not have and would 
have to buy, run and maintain ourselves. For now we have to inspect the 
log dumps generated in the job output.



In the meanwhile I would like to ask all reviewers to carefully check the
output of failed Travis CI runs. If the job fails, you will see the results
at the very end of the log. There are two sections: PEP8 errors and test
output. You can expand both of them to see what went wrong and report it to
the PR author if necessary.

The reviewer and author can then use the very same tool used in CI [1] to
reproduce the failures locally. Using  '--no-cleanup' option during the run
[2] leaves behind a running container which you can attach to and
investigate further.

[1] https://github.com/freeipa/ipa-docker-test-runner
[2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md

If you have any additional questions/suggestions about Travis feel free to
contact me.

--
Martin^3 Babinsky

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


[1] https://docs.travis-ci.com/user/uploading-artifacts/

--
Martin^3 Babinsky

--
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
From dcd48155a899a14cdf1de843fa729064ba06b4b7 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 20:24:30 +1000
Subject: [PATCH 1/2] ci: run tests with a single job instead of two

---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e870213..2a409f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,7 +5,6 @@ env:
 global:
 - TEST_RUNNER_IMAGE="martbab/freeipa-fedora-test-runner:master-latest"
 matrix:
-- TESTS_TO_RUN="test_xmlrpc/test_[a-k]*.py"
 - >
 TESTS_TO_RUN="test_cmdline
 test_install
@@ -13,7 +12,7 @@ env:
 test_ipapython
 test_ipaserver
 test_pkcs10
-test_xmlrpc/test_[l-z]*.py"
+test_xmlrpc/test_[a-z]*.py"
 before_install:
 - pip install pep8
 - >

From 4f29cd26fccb508538598da9dac96b12a3317aee Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 22:11:07 +1000
Subject: [PATCH 2/2] ci: make travis wait 120 mins (experimental)

---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 2a409f2..b574e86 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -30,7 +30,7 @@ script:
 # output do not cause premature termination of the build
 - "docker pull ${TEST_RUNNER_IMAGE}"
 - >
-travis_wait 50
+travis_wait 120
 ipa-docker-test-runner -l ci_results_${TRAVIS_BRANCH}.log
 -c .test_runner_config.yaml
 --container-image ${TEST_RUNNER_IMAGE}
-- 
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] Travis CI broke after merging PR 177

2016-12-13 Thread Fraser Tweedale
On Tue, Dec 13, 2016 at 09:41:40AM +0100, Martin Babinsky wrote:
> Hi list,
> 
> https://github.com/freeipa/freeipa/pull/177 was recently merged despite
> causing nearly half of the tests in our Travis CI gating to fail. This broke
> Travis CI for all other PR that were rebased after this merge, causing false
> negative errors everywhere.
> 
> Fraser reverted the offending commits in
> https://github.com/freeipa/freeipa/pull/329 which restored Travis to
> original state (never mind PEP8 errors they were in the original code
> already).
> 
> Regarding this issues I have two questions:
> 
> a)
> 
> should I merge https://github.com/freeipa/freeipa/pull/329 and thus revert
> the breakage in order to unblock other contributors? Given the current
> traffic I think it is sufficient to wait for us to investigate and produce a
> fix. If not, please scream loudly.
> 
Definitely not, I am using this PR as a playground to poke the CI in
various ways.  Also, proper fix would be best :)  See also my other
mail (I would have replied here but fetchmail was not fetching mail
and I missed it until just now >_<).

> b)
> 
> what can we improve to make the results of CI more visible to contributors?
> I think that I should sit down with Martin 2 and investigate the possibility
> to send notifications about negative CI results (sufficient IMO) to the
> mailing list.
> 
Or the commit author.

It would be *great* if the test job, in event of failure, would
collect all the obvious logs and dump them somewhere as artifacts.

> In the meanwhile I would like to ask all reviewers to carefully check the
> output of failed Travis CI runs. If the job fails, you will see the results
> at the very end of the log. There are two sections: PEP8 errors and test
> output. You can expand both of them to see what went wrong and report it to
> the PR author if necessary.
> 
> The reviewer and author can then use the very same tool used in CI [1] to
> reproduce the failures locally. Using  '--no-cleanup' option during the run
> [2] leaves behind a running container which you can attach to and
> investigate further.
> 
> [1] https://github.com/freeipa/ipa-docker-test-runner
> [2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md
> 
> If you have any additional questions/suggestions about Travis feel free to
> contact me.
> 
> -- 
> Martin^3 Babinsky
> 
> -- 
> 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

-- 
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
From dcd48155a899a14cdf1de843fa729064ba06b4b7 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 20:24:30 +1000
Subject: [PATCH] ci: run tests with a single job instead of two

---
 .travis.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e870213..2a409f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,7 +5,6 @@ env:
 global:
 - TEST_RUNNER_IMAGE="martbab/freeipa-fedora-test-runner:master-latest"
 matrix:
-- TESTS_TO_RUN="test_xmlrpc/test_[a-k]*.py"
 - >
 TESTS_TO_RUN="test_cmdline
 test_install
@@ -13,7 +12,7 @@ env:
 test_ipapython
 test_ipaserver
 test_pkcs10
-test_xmlrpc/test_[l-z]*.py"
+test_xmlrpc/test_[a-z]*.py"
 before_install:
 - pip install pep8
 - >
-- 
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] [freeipa PR#324][comment] Check for conflict entries before raising domain level

2016-12-13 Thread martbab
  URL: https://github.com/freeipa/freeipa/pull/324
Title: #324: Check for conflict entries before raising domain level

martbab commented:
"""
There were some PEP8 errors but I fixed them and pushed to:

ipa-4-4:
https://fedorahosted.org/freeipa/changeset/d028d23c5f0c3e1b18c15fad67a0893870f5d27c
master:
https://fedorahosted.org/freeipa/changeset/26bd7ebfa27d15221e5d3fa1e3871a0085c31e0f
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/324#issuecomment-266714549
-- 
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] [freeipa PR#324][closed] Check for conflict entries before raising domain level

2016-12-13 Thread martbab
   URL: https://github.com/freeipa/freeipa/pull/324
Author: tbordaz
 Title: #324: Check for conflict entries before raising domain level
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/324/head:pr324
git checkout pr324
-- 
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] CI failures - I need your help

2016-12-13 Thread Fraser Tweedale
Hi all,

The CI failures caused by one of my recent commits have me baffled.
It is exactly this commit[1] at which the problems begin.  I cannot
see anything in the commit to point a finger at.  In-tree tests run
fine.

[1] 
https://github.com/freeipa/freeipa/commit/32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d

In terms of symptoms: a Travis run has two jobs, one dealing with
`test_xmlrpc/test_[a-k]*.py' and the other dealing with
`test_xmlrpc/test_[l-z]*.py' as well as `test_install/',
`test_ipalib/', `test_ipapython/', `test_ipaserver/' and
`test_pkcs10'.  See [2] for an example of a failing build: the first
job (which includes tests for the ca plugin, which is what the patch
relates to) does succeed, but the second fails catastrophically with
myriad occurences of:

E   NetworkError: cannot connect to
'https://master1.ipa.test/ipa/session/json':
(SSL_ERROR_NO_CIPHERS_SUPPORTED) No cipher suites are present
and enabled in this program.

Leading to the unfortunate outcome:

=== 254 failed, 576 passed, 98 skipped, 1756 error in 254.59 seconds 
===

[2] https://travis-ci.org/freeipa/freeipa/builds/183544973


I have been unsuccessful in running `ipa-docker-test-runner' on my
own machine due to Java thread creation problems, so I need the help
of others to analyse and fix this issue.  If someone was able to
analyse a running container that had the test failures, send logs,
or even make an image of the container and upload it somewhere for
me, it might lead to some answers.

Thanks for your help.
Fraser

(P.S. I hope it is not a trivial thing I have overlook ^_^)

-- 
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] [freeipa PR#324][synchronized] Check for conflict entries before raising domain level

2016-12-13 Thread tbordaz
   URL: https://github.com/freeipa/freeipa/pull/324
Author: tbordaz
 Title: #324: Check for conflict entries before raising domain level
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/324/head:pr324
git checkout pr324
From 94d592d557795cdf05f3fd3679ea7fcc9ed7f153 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz 
Date: Fri, 9 Dec 2016 15:04:21 +0100
Subject: [PATCH] Check for conflict entries before raising domain level

Checking of conflicts is not only done in topology container as
tests showed it can occurs elsewhere

https://fedorahosted.org/freeipa/ticket/6534
---
 ipaserver/plugins/domainlevel.py | 28 
 1 file changed, 28 insertions(+)

diff --git a/ipaserver/plugins/domainlevel.py b/ipaserver/plugins/domainlevel.py
index 42603d7..e1f0251 100644
--- a/ipaserver/plugins/domainlevel.py
+++ b/ipaserver/plugins/domainlevel.py
@@ -48,6 +48,30 @@ def get_domainlevel_range(master_entry):
 return DomainLevelRange(0, 0)
 
 
+def check_conflict_entries(ldap, api, desired_value):
+"""
+Check if conflict entries exist in topology subtree
+"""
+
+container_dn = DN(
+('cn', 'ipa'),
+('cn', 'etc'),
+api.env.basedn
+)
+conflict="(nsds5replconflict=*)"
+subentry="(|(objectclass=ldapsubentry)(objectclass=*))"
+try:
+ldap.get_entries(
+filter="(& %s %s)" % (conflict, subentry),
+base_dn=container_dn,
+scope=ldap.SCOPE_SUBTREE)
+message = _("Domain Level cannot be raised to {0}, "
+"existing replication conflicts have to be resolved."
+.format(desired_value))
+raise errors.InvalidDomainLevelError(reason=message)
+except errors.NotFound:
+pass
+
 def get_master_entries(ldap, api):
 """
 Returns list of LDAPEntries representing IPA masters.
@@ -131,6 +155,10 @@ def execute(self, *args, **options):
 .format(desired_value, master['cn'][0]))
 raise errors.InvalidDomainLevelError(reason=message)
 
+# Check if conflict entries exist in topology subtree
+# should be resolved first
+check_conflict_entries(ldap, self.api, desired_value)
+
 current_entry.single_value['ipaDomainLevel'] = desired_value
 ldap.update_entry(current_entry)
 
-- 
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] [freeipa PR#329][reopened] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: reopened

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
-- 
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
-- 
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
-- 
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] [bind-dyndb-ldap PR#5][opened] Add GDB pretty-printers for plugin data structures to contrib.

2016-12-13 Thread pspacek
   URL: https://github.com/freeipa/bind-dyndb-ldap/pull/5
Author: pspacek
 Title: #5: Add GDB pretty-printers for plugin data structures to contrib.
Action: opened

PR body:
"""
These are convenience scripts I created over time to ease digging in 
bind-dyndb-ldap data structures. It might make sense to include these in the 
main repo...
"""

To pull the PR as Git branch:
git remote add ghbind-dyndb-ldap https://github.com/freeipa/bind-dyndb-ldap
git fetch ghbind-dyndb-ldap pull/5/head:pr5
git checkout pr5
From e9aae86c5ff874af3a7f85667767a9912b47ce86 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 13 Dec 2016 11:20:04 +0100
Subject: [PATCH] Add GDB pretty-printers for plugin data structures to
 contrib.

---
 contrib/gdb_extensions/README| 16 +
 contrib/gdb_extensions/ldap_attribute.py | 59 
 contrib/gdb_extensions/ldap_entry.py | 45 
 contrib/gdb_extensions/ldap_valuelist.py | 50 +++
 4 files changed, 170 insertions(+)
 create mode 100644 contrib/gdb_extensions/README
 create mode 100644 contrib/gdb_extensions/ldap_attribute.py
 create mode 100644 contrib/gdb_extensions/ldap_entry.py
 create mode 100644 contrib/gdb_extensions/ldap_valuelist.py

diff --git a/contrib/gdb_extensions/README b/contrib/gdb_extensions/README
new file mode 100644
index 000..23c8406
--- /dev/null
+++ b/contrib/gdb_extensions/README
@@ -0,0 +1,16 @@
+This directory contains pretty-printers which make easier to read
+bind-dyndb-ldap data structures in GDB with Python support.
+
+To use them, simply source the files into GDB:
+(gdb) source /path/to/pretty/printer.py
+
+BEWARE:
+The pretty printers work well if memory is not corrupted.
+Disable pretty printers if you suspect memory corruption:
+(gdb) disable pretty-printer
+
+Pretty-printers for BIND 9 data structures might be handy, too:
+https://github.com/pspacek/bind-gdb-pretty-printers
+
+For further information see
+https://sourceware.org/gdb/onlinedocs/gdb/Python-API.html
diff --git a/contrib/gdb_extensions/ldap_attribute.py b/contrib/gdb_extensions/ldap_attribute.py
new file mode 100644
index 000..757e8e1
--- /dev/null
+++ b/contrib/gdb_extensions/ldap_attribute.py
@@ -0,0 +1,59 @@
+import os
+import sys
+import json
+
+def gdb_printer_decorator(fn):
+gdb.pretty_printers.append(fn)
+return fn
+
+class ldap_attribute_Printer(object):
+def __init__(self, val):
+self.val = val
+self.values_pp = gdb.default_visualizer(val['values'])
+self.l = self._enumerate_children()
+
+def singleton(self):
+return self.values_pp and len(self.l) <= 1
+
+def display_hint(self):
+if self.singleton():
+return "string"
+else:
+return "array"
+
+def _enumerate_children(self):
+values = self.val['values']
+# pretty printer for ldap_valuelist returns array so [('0', values)]
+# results in list inside list which is not pretty
+# -> remove inner list
+if not self.values_pp:
+# prettyprinter for ldap_valuelist is not available
+return False
+
+l = []
+for v in self.values_pp.children():
+l.append(v)
+return l
+
+def to_string(self):
+out = "attribute: %s" % self.val['name'].string()
+if self.singleton():
+out += ' = { %s }' % self.values_pp.to_string()
+
+return out
+
+def children(self):
+if self.singleton():
+return []
+l = self._enumerate_children()
+if not l: # pretty printer is not available - cannot enumerate children
+return [('0', self.val['values'])]
+else:
+return l
+
+# register pretty printers
+@gdb_printer_decorator
+def dns_rbt_printer(val):
+if str(val.type) == 'ldap_attribute_t':
+return ldap_attribute_Printer(val)
+return None
diff --git a/contrib/gdb_extensions/ldap_entry.py b/contrib/gdb_extensions/ldap_entry.py
new file mode 100644
index 000..4b91c49
--- /dev/null
+++ b/contrib/gdb_extensions/ldap_entry.py
@@ -0,0 +1,45 @@
+import os
+import sys
+import json
+
+def gdb_printer_decorator(fn):
+gdb.pretty_printers.append(fn)
+return fn
+
+class ldap_entry_Printer(object):
+def __init__(self, val):
+self.val = val
+
+def display_hint(self):
+return 'array'
+
+def to_string(self):
+out = "entry DN: %s" % self.val['dn'].string()
+return out
+
+def children(self):
+i = 0
+l = []
+head = self.val['attrs']['head'].dereference()
+while head:
+l.append((str(i), head))
+if head['link']['next']:
+head = head['link']['next'].dereference()
+else:
+head = None
+i += 1
+return l
+
+class TestPrinter(object):
+def __init__(self, val):
+self.val = val
+
+  

[Freeipa-devel] [freeipa PR#330][opened] Build: forbid builds in working directories containing white spaces

2016-12-13 Thread pspacek
   URL: https://github.com/freeipa/freeipa/pull/330
Author: pspacek
 Title: #330: Build: forbid builds in working directories containing white 
spaces
Action: opened

PR body:
"""
Spaces are causing problems in libtool, makefiles, autoconf itself, gettextize
framework etc. so this issue cannot be easily fixed.

Return on investment is too small to invest into this. Let's detect the
whitespace early and error out with descriptive error message.

https://fedorahosted.org/freeipa/ticket/6537
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/330/head:pr330
git checkout pr330
From bdd0bdb27b21ce0c1c4ce58ea59e8dbedb77f403 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 13 Dec 2016 10:52:46 +0100
Subject: [PATCH] Build: forbid builds in working directories containing white
 spaces

Spaces are causing problems in libtool, makefiles, autoconf itself, gettextize
framework etc. so this issue cannot be easily fixed.

Return on investment is too small to invest into this. Let's detect the
whitespace early and error out with descriptive error message.

https://fedorahosted.org/freeipa/ticket/6537
---
 configure.ac | 8 
 1 file changed, 8 insertions(+)

diff --git a/configure.ac b/configure.ac
index c02a672..9bf72a2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -5,6 +5,14 @@ AC_INIT([freeipa],
 IPA_VERSION,
 [https://hosted.fedoraproject.org/projects/freeipa/newticket])
 
+dnl Make sure the build directory name does not contain spaces!
+dnl Spaces are causing problems in libtool, makefiles, autoconf itself,
+dnl gettextize framework etc.
+case "$PWD" in
+  *\ * | *\	*)
+AC_MSG_ERROR([whitespace in working directory path is not supported]) ;;
+esac
+
 AC_CONFIG_HEADERS([config.h])
 
 AM_INIT_AUTOMAKE([foreign 1.9 tar-ustar])
-- 
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] [freeipa PR#329][synchronized] experiment: did pull/177 break ci?

2016-12-13 Thread frasertweedale
   URL: https://github.com/freeipa/freeipa/pull/329
Author: frasertweedale
 Title: #329: experiment: did pull/177 break ci?
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/329/head:pr329
git checkout pr329
From 8e13b7c01311e44eb3ec1dc16dac26b8d3287139 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 13 Dec 2016 10:50:50 +1000
Subject: [PATCH] Revert "Add options to write lightweight CA cert or chain to
 file"

This reverts commit 32b1743e5fb318b226a602ec8d9a4b6ef2a25c9d.
---
 API.txt   |  6 +--
 VERSION.m4|  4 +-
 ipaclient/plugins/ca.py   | 53 -
 ipaserver/plugins/ca.py   | 65 +++
 ipaserver/plugins/dogtag.py   | 12 --
 ipatests/test_xmlrpc/tracker/ca_plugin.py | 31 ---
 ipatests/test_xmlrpc/xmlrpc_test.py   | 17 
 7 files changed, 16 insertions(+), 172 deletions(-)
 delete mode 100644 ipaclient/plugins/ca.py

diff --git a/API.txt b/API.txt
index 543cec5..bad3b92 100644
--- a/API.txt
+++ b/API.txt
@@ -445,11 +445,10 @@ option: Str('version?')
 output: Output('count', type=[])
 output: Output('results', type=[, ])
 command: ca_add/1
-args: 1,8,3
+args: 1,7,3
 arg: Str('cn', cli_name='name')
 option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Flag('chain', autofill=True, default=False)
 option: Str('description?', cli_name='desc')
 option: DNParam('ipacasubjectdn', cli_name='subject')
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
@@ -520,10 +519,9 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: ca_show/1
-args: 1,5,3
+args: 1,4,3
 arg: Str('cn', cli_name='name')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Flag('chain', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Flag('rights', autofill=True, default=False)
 option: Str('version?')
diff --git a/VERSION.m4 b/VERSION.m4
index 36929ee..7d9e107 100644
--- a/VERSION.m4
+++ b/VERSION.m4
@@ -73,8 +73,8 @@ define(IPA_DATA_VERSION, 2010061412)
 #  #
 
 define(IPA_API_VERSION_MAJOR, 2)
-define(IPA_API_VERSION_MINOR, 217)
-# Last change: Add options to write lightweight CA cert or chain to file
+define(IPA_API_VERSION_MINOR, 216)
+# Last change: DNS: Support URI resource record type
 
 
 
diff --git a/ipaclient/plugins/ca.py b/ipaclient/plugins/ca.py
deleted file mode 100644
index fcdf484..000
--- a/ipaclient/plugins/ca.py
+++ /dev/null
@@ -1,53 +0,0 @@
-#
-# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
-#
-
-import base64
-from ipaclient.frontend import MethodOverride
-from ipalib import util, x509, Str
-from ipalib.plugable import Registry
-from ipalib.text import _
-
-register = Registry()
-
-
-class WithCertOutArgs(MethodOverride):
-
-takes_options = (
-Str(
-'certificate_out?',
-doc=_('Write certificate (chain if --chain used) to file'),
-include='cli',
-cli_metavar='FILE',
-),
-)
-
-def forward(self, *keys, **options):
-filename = None
-if 'certificate_out' in options:
-filename = options.pop('certificate_out')
-util.check_writable_file(filename)
-
-result = super(WithCertOutArgs, self).forward(*keys, **options)
-if filename:
-def to_pem(x):
-return x509.make_pem(x)
-if options.get('chain', False):
-ders = result['result']['certificate_chain']
-data = '\n'.join(to_pem(base64.b64encode(der)) for der in ders)
-else:
-data = to_pem(result['result']['certificate'])
-with open(filename, 'wb') as f:
-f.write(data)
-
-return result
-
-
-@register(override=True, no_fail=True)
-class ca_add(WithCertOutArgs):
-pass
-
-
-@register(override=True, no_fail=True)
-class ca_show(WithCertOutArgs):
-pass
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index ef1d68c..d9ae8c8 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -2,18 +2,14 @@
 # Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
 
-import base64
-
-import six
-
-from ipalib import api, errors, output, Bytes, DNParam, Flag, Str
+from ipalib import api, errors, output, DNParam, Str
 from ipalib.constants import IPA_CA_CN
 from ipalib.plugable import Registry
 from ipaserver.plugins.baseldap import (
 LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete,
 LDAPUpdate, 

[Freeipa-devel] [freeipa PR#301][comment] scripts, tests: explicitly set confdir in the rest of server code

2016-12-13 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/301
Title: #301: scripts, tests: explicitly set confdir in the rest of server code

stlaz commented:
"""
@tiran I find all the changes actually required. I think ACK is in order unless 
you spell out those which you think are not necessary.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/301#issuecomment-266683420
-- 
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] Travis CI broke after merging PR 177

2016-12-13 Thread Martin Babinsky

Hi list,

https://github.com/freeipa/freeipa/pull/177 was recently merged despite 
causing nearly half of the tests in our Travis CI gating to fail. This 
broke Travis CI for all other PR that were rebased after this merge, 
causing false negative errors everywhere.


Fraser reverted the offending commits in 
https://github.com/freeipa/freeipa/pull/329 which restored Travis to 
original state (never mind PEP8 errors they were in the original code 
already).


Regarding this issues I have two questions:

a)

should I merge https://github.com/freeipa/freeipa/pull/329 and thus 
revert the breakage in order to unblock other contributors? Given the 
current traffic I think it is sufficient to wait for us to investigate 
and produce a fix. If not, please scream loudly.


b)

what can we improve to make the results of CI more visible to 
contributors? I think that I should sit down with Martin 2 and 
investigate the possibility to send notifications about negative CI 
results (sufficient IMO) to the mailing list.


In the meanwhile I would like to ask all reviewers to carefully check 
the output of failed Travis CI runs. If the job fails, you will see the 
results at the very end of the log. There are two sections: PEP8 errors 
and test output. You can expand both of them to see what went wrong and 
report it to the PR author if necessary.


The reviewer and author can then use the very same tool used in CI [1] 
to reproduce the failures locally. Using  '--no-cleanup' option during 
the run [2] leaves behind a running container which you can attach to 
and investigate further.


[1] https://github.com/freeipa/ipa-docker-test-runner
[2] https://github.com/freeipa/ipa-docker-test-runner/blob/master/README.md

If you have any additional questions/suggestions about Travis feel free 
to contact me.


--
Martin^3 Babinsky

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