Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
On 10/18/2012 05:51 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 05:22 PM, Rob Crittenden wrote: Martin Kosek wrote: Report errors just like with ipa-ldap-updater. These messages should warn user that some parts of the upgrades may have not been successful and he should follow up on them. Otherwise, user may not notice them at all. ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater logging - only error level or higher severity log messages are printed to stderr, with the same console format. A new debug log that an upgrade script has started was added to make log investigation easier. https://fedorahosted.org/freeipa/ticket/3157 --- Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update will report errors to user: # rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-* Preparing...### [100%] 1:freeipa-python ### [ 14%] 2:freeipa-client ### [ 29%] 3:freeipa-admintools ### [ 43%] 4:freeipa-server ### [ 57%] ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission denied: '/etc/named.conf' 5:freeipa-server-selinux ### [ 71%] 6:freeipa-server-trust-ad### [ 86%] 7:freeipa-debuginfo ### [100%] Martin Can you add an option to run in info mode (the equivalent of verbose=True)? Otherwise running from the cli returns nothing unless there is an error, I like some amount of output myself. rob ipa-upgradeconfig has a --debug option. I doubt that the output can be more verbose :-) Martin Yeah, but there is no middle ground. You either get everything or nothing. rob Well, I can add it if you want it really hard. But from my perspective, having both --debug and --verbose is strange and it will also make it inconsistent with our other CLI tools where we have just --debug option (ipa-ldap-updater, ipa-replica-prepare, ipa-*-install). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges
On 10/18/2012 10:00 PM, Sumit Bose wrote:
On Thu, Oct 18, 2012 at 08:31:50AM +0200, Tomas Babej wrote:
On 10/17/2012 08:12 PM, Sumit Bose wrote:
On Wed, Oct 17, 2012 at 03:29:11PM +0200, Tomas Babej wrote:
On 10/17/2012 02:34 PM, Sumit Bose wrote:
On Wed, Oct 17, 2012 at 12:59:52PM +0200, Tomas Babej wrote:
On 10/17/2012 11:14 AM, Sumit Bose wrote:
On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:
Hi,
commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.
Unit tests added to test_range_plugin.py
https://fedorahosted.org/freeipa/ticket/3086
Tomas
Thank you for the patch, comments are in-line.
bye,
Sumit
Thank you for your suggestions. New version of the patch attached.
Tomas
Thank you for addressing my comments. I just realized that the check is
too strict.
The ranges of the Posix IDs [base_id - base_id+id_range_size) may not
overlap for any existing range because those IDs belong to the single
Posix ID namespace of the IPA domain. I.e each user, local or from a
trusted domain, must have a unique Posix ID.
The RID ranges [base_rid, base_rid+id_range_size) and
[secondary_base_rid, secondary_base_rid+id_range_size) may not overlap
with RID ranges from the same domain. So the RID ranges for the local
domain may not overlap and the RID ranges for any specific trusted
domain may not overlap. It is allowed that there is a range form the
local domain may have base_rid=1000 and a range from a trusted domain as
well. This is ok because the RID is only part of the identifier, each
domain has a unique domain SID which is used together with the RID to
identify e.g. a user.
I would suggest to look for the ipaNTTrustedDomainSID attribute in
slapi_entry_to_range_info() too and add it to struct range_info. In
ranges_overlap() you can then check the Posix ID range for all ranges
but do the RID checks only when the domain identifiers are either both
NULL (local IPA domain) or are the same strings.
Sorry for not seeing this earlier.
bye,
Sumit
Thanks for catching this issue. It is solved in the newest revision
of the patch.
Tomas
sorry, found another one ...
...
+static int ranges_overlap(struct range_info *r1, struct range_info *r2)
+{
+if (r1-name != NULL r2-name != NULL
+strcasecmp(r1-name, r2-name) == 0) {
+return 0;
+}
+
+/* check if base range overlaps with existing base range */
+if (intervals_overlap(r1-base_id, r2-base_id,
+r1-id_range_size, r2-id_range_size)){
+return 1;
+}
+
+/* if both base_rid and secondary_base_rid = 0, the rid range is not
set */
+bool rid_ranges_set = (r1-base_rid != 0 || r1-secondary_base_rid !=
0)
+ (r2-base_rid != 0 || r2-secondary_base_rid !=
0);
+
+bool ranges_from_same_domain =
+ (r1-domain_id == NULL r2-domain_id == NULL) ||
+ (strcasecmp(r1-domain_id, r2-domain_id) == 0);
+
you have to check that both domain_id are not NULL before calling
strcasecmp.
bye,
Sumit
Null pointer check added.
Thank you.
ACK
bye,
Sumit
Tomas
Thanks guys. Pushed to master, ipa-3-0.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 75] log dogtag errors
On 10/18/2012 07:20 PM, John Dennis wrote: On 10/18/2012 05:06 AM, Petr Viktorin wrote: This looks much better. I found one more issue, though. +if detail is not None: +err_msg += ' (%s)' % detail Here I get TypeError: unsupported operand type(s) for +=: 'Gettext' and 'unicode'. Until our Gettext class supports addition (part of #3188), please use `err_msg = u'%s (%s)' % (err_msg, detail)` instead. Good catch, fixed. New patch attached. It works fine now, thanks. ACK -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage
On 10/18/2012 08:01 PM, Rob Crittenden wrote: Tomas Babej wrote: On 10/02/2012 03:55 PM, Rob Crittenden wrote: Tomas Babej wrote: Hi, When executing ipa-replica-manage connect to an unknown or irrelevant master, we now print a sensible error message informing the user about this possiblity as well. https://fedorahosted.org/freeipa/ticket/3105 Tomas I put a whole bunch of code into a try/except and this may be catching errors in unexpected ways. I'm not entirely sure right now what we should do, but looking at the code in the try: repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE) repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE) We take in replica1 and replica1 as arguments (the default for replica1 is the current host). If either of these raise a NotFound it means there there is no master of that name. Does that mean that the master was deleted? Well, clearly not. A lot has changed since I did this, I may have been relying on a side-effect, or just hadn't tested well-enough. I wonder if we need that message at all. Is foo is not an IPA server good enough? It still might be confusing if someone didn't know that foo was deleted and it was still running. We could probably verify that it is at least an IPA server by doing similar checking in the client, it all depends on how far we want to take it. rob I modified the patch. Now if the NotFound error is encountered, we try to investigate whether we're trying to connect to an IPA server at all. Please see if you have any suggestions. Tomas Getting there, the errors are a lot better. Can you modify the 'Connection unsuccessful' message to include the server we failed to connect to? The logger isn't so nice either, I think I'd prefer it if we could exclude the severity: ipa: ERROR: LDAP Error: Connect error: TLS error -8172:Peer's certificate issuer has been marked as not trusted by the user. Connection unsuccessful. So drop the 'ipa: ERROR: ' part for consistency. TI don't believe we configure the root logger at all in this tool so it is using the defaults. It's not very easy to find the right call to configure the logger to drop the ipa: ERROR: part: standard_logging_setup(console_format='%(message)s') The function is in ipapython.ipa_log_manager. Hopefully that helps. I'd also replace the test for -4 with the constant ipadiscovery.NOT_IPA_SERVER rob -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
On 10/19/2012 08:15 AM, Martin Kosek wrote: On 10/18/2012 05:51 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 05:22 PM, Rob Crittenden wrote: Martin Kosek wrote: Report errors just like with ipa-ldap-updater. These messages should warn user that some parts of the upgrades may have not been successful and he should follow up on them. Otherwise, user may not notice them at all. ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater logging - only error level or higher severity log messages are printed to stderr, with the same console format. A new debug log that an upgrade script has started was added to make log investigation easier. https://fedorahosted.org/freeipa/ticket/3157 --- Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update will report errors to user: # rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-* Preparing...### [100%] 1:freeipa-python ### [ 14%] 2:freeipa-client ### [ 29%] 3:freeipa-admintools ### [ 43%] 4:freeipa-server ### [ 57%] ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission denied: '/etc/named.conf' 5:freeipa-server-selinux ### [ 71%] 6:freeipa-server-trust-ad### [ 86%] 7:freeipa-debuginfo ### [100%] Martin Can you add an option to run in info mode (the equivalent of verbose=True)? Otherwise running from the cli returns nothing unless there is an error, I like some amount of output myself. rob ipa-upgradeconfig has a --debug option. I doubt that the output can be more verbose :-) Martin Yeah, but there is no middle ground. You either get everything or nothing. rob Well, I can add it if you want it really hard. But from my perspective, having both --debug and --verbose is strange and it will also make it inconsistent with our other CLI tools where we have just --debug option (ipa-ldap-updater, ipa-replica-prepare, ipa-*-install). I like ipa-replica-conncheck's approach: --debug (output everything) and --quiet (only output errors). -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request Do you want to fix it in current patch or should I create a ticket? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 75] log dogtag errors
On 10/19/2012 09:45 AM, Petr Viktorin wrote: On 10/18/2012 07:20 PM, John Dennis wrote: On 10/18/2012 05:06 AM, Petr Viktorin wrote: This looks much better. I found one more issue, though. +if detail is not None: +err_msg += ' (%s)' % detail Here I get TypeError: unsupported operand type(s) for +=: 'Gettext' and 'unicode'. Until our Gettext class supports addition (part of #3188), please use `err_msg = u'%s (%s)' % (err_msg, detail)` instead. Good catch, fixed. New patch attached. It works fine now, thanks. ACK Pushed to master, ipa-3-0. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
On 10/19/2012 10:10 AM, Martin Kosek wrote: On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request IMHO there should be a --force option. SAN can contain a lot of different things. Also, we can't assume that we manage the whole world ... (now :-)) Do you want to fix it in current patch or should I create a ticket? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent
On 10/18/2012 11:27 AM, Martin Kosek wrote: On 10/11/2012 05:11 PM, Tomas Babej wrote: On 10/11/2012 12:32 PM, Martin Kosek wrote: On 10/11/2012 12:26 PM, Tomas Babej wrote: Hi, This patch forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. Sample output produced by this patch attached. https://fedorahosted.org/freeipa/ticket/3059 Tomas Just based on reading the patch, this does not look right: -self.start_creation(Configuring certificate server, 210) +self.start_creation(Configuring directory server for the CA, +end_message=Done configuring directory server for the CA, +show_service_name=True, runtime=210) Martin Thanks, glitch fixed. Tomas Ok, I managed to get the patch a proper review. I like the result, in the console, but I still not entirely satisfied with the patch, looks over-engineered to me + there is a lot of duplication with Configuring %(service)s and Done configuring %(service)s messages. These messages could be easily generated only based on name of a service (self.service_name, we got that) and a new friendly service name or description. If we add description this way: class KrbInstance(service.Service): def __init__(self, fstore=None): service.Service.__init__(self, krb5kdc, description=Kerberos KDC) ... the start_creation could be very simple: ... self.start_creation(runtime=30) ... All messages could be simply generated by the Service class just based on self.service_name and self.description with having the same output as you do now. Martin I simplified the approach as you suggested. However, I kept optional start_message and end_message parameters in case we would want to specify the messages instead of using the pre-generated ones. This is used in baseupdate.py and upgradeinstance.py More info in the documentation of start_creation() function. Tomas From 243d582d085f9ecf468d99ddf1fc6cb11db9533f Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Thu, 11 Oct 2012 03:32:17 -0400 Subject: [PATCH] Make service naming in ipa-server-install consistent Forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. See documentation for more info. https://fedorahosted.org/freeipa/ticket/3059 --- ipaserver/install/adtrustinstance.py| 4 +-- ipaserver/install/bindinstance.py | 9 +-- ipaserver/install/cainstance.py | 18 +++--- ipaserver/install/dsinstance.py | 11 ++--- ipaserver/install/httpinstance.py | 4 +-- ipaserver/install/krbinstance.py| 6 ++--- ipaserver/install/ntpinstance.py| 4 +-- ipaserver/install/plugins/baseupdate.py | 2 +- ipaserver/install/service.py| 44 +++-- ipaserver/install/upgradeinstance.py| 2 +- 10 files changed, 77 insertions(+), 27 deletions(-) diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index b3602ddce7fa1c184bdf1283e67b50463e49c5fd..41edeac47e403491b0a55ffeeebb1072549262d5 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -132,7 +132,7 @@ class ADTRUSTInstance(service.Service): self.rid_base = None self.secondary_rid_base = None -service.Service.__init__(self, smb, dm_password=None, ldapi=True) +service.Service.__init__(self, smb, service_desc=CIFS, dm_password=None, ldapi=True) if fstore: self.fstore = fstore @@ -757,7 +757,7 @@ class ADTRUSTInstance(service.Service): self.step(adding SIDs to existing users and groups, self.__add_sids) -self.start_creation(Configuring CIFS:) +self.start_creation() def uninstall(self): if self.is_configured(): diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index f43a9ff0f9114388ae072daa82c39aaff716f7b2..56bc8686a8fd6dbbcde5af1e67ef5095f96be50e 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -409,7 +409,12 @@ class DnsBackup(object): class BindInstance(service.Service): def __init__(self, fstore=None, dm_password=None): -service.Service.__init__(self, named, dm_password=dm_password, ldapi=False, autobind=service.DISABLED) +service.Service.__init__(self, named, +service_desc=DNS, +dm_password=dm_password, +ldapi=False, +autobind=service.DISABLED +) self.dns_backup =
Re: [Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent
On 10/19/2012 01:26 PM, Tomas Babej wrote: On 10/18/2012 11:27 AM, Martin Kosek wrote: On 10/11/2012 05:11 PM, Tomas Babej wrote: On 10/11/2012 12:32 PM, Martin Kosek wrote: On 10/11/2012 12:26 PM, Tomas Babej wrote: Hi, This patch forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. Sample output produced by this patch attached. https://fedorahosted.org/freeipa/ticket/3059 Tomas Just based on reading the patch, this does not look right: -self.start_creation(Configuring certificate server, 210) +self.start_creation(Configuring directory server for the CA, +end_message=Done configuring directory server for the CA, +show_service_name=True, runtime=210) Martin Thanks, glitch fixed. Tomas Ok, I managed to get the patch a proper review. I like the result, in the console, but I still not entirely satisfied with the patch, looks over-engineered to me + there is a lot of duplication with Configuring %(service)s and Done configuring %(service)s messages. These messages could be easily generated only based on name of a service (self.service_name, we got that) and a new friendly service name or description. If we add description this way: class KrbInstance(service.Service): def __init__(self, fstore=None): service.Service.__init__(self, krb5kdc, description=Kerberos KDC) ... the start_creation could be very simple: ... self.start_creation(runtime=30) ... All messages could be simply generated by the Service class just based on self.service_name and self.description with having the same output as you do now. Martin I simplified the approach as you suggested. However, I kept optional start_message and end_message parameters in case we would want to specify the messages instead of using the pre-generated ones. This is used in baseupdate.py and upgradeinstance.py More info in the documentation of start_creation() function. Tomas NACK. Tried running ./make-lint with your patch, got thousands of errors... But the approach looks OK, I am just thinking that default value for show_service_name of start_creation() should be True as it is the most widely used value - you would not have to specify it everytime you call start_creation() in *instance.py files. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 223 Simpler instructions to generate certificate
On 10/18/2012 05:24 PM, Rob Crittenden wrote: Petr Vobornik wrote: Instructions to generate certificate were simplified. New instructions: 1) Create a certificate database or use an existing one. To create a new database: # certutil -N -d database path 2) Create a CSR with subject CN=hostname,O=realm, for example: # certutil -R -d database path -a -g key size -s 'CN=dev.example.com,O=DEV.EXAMPLE.COM' 3) Copy and paste the CSR (from -BEGIN NEW CERTIFICATE REQUEST- to -END NEW CERTIFICATE REQUEST-) into the text area below: https://fedorahosted.org/freeipa/ticket/3056 ACK, looks good to me. rob Pushed to ipa-3-0, master. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent
On 10/19/2012 01:44 PM, Martin Kosek wrote: On 10/19/2012 01:26 PM, Tomas Babej wrote: On 10/18/2012 11:27 AM, Martin Kosek wrote: On 10/11/2012 05:11 PM, Tomas Babej wrote: On 10/11/2012 12:32 PM, Martin Kosek wrote: On 10/11/2012 12:26 PM, Tomas Babej wrote: Hi, This patch forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. Sample output produced by this patch attached. https://fedorahosted.org/freeipa/ticket/3059 Tomas Just based on reading the patch, this does not look right: -self.start_creation(Configuring certificate server, 210) +self.start_creation(Configuring directory server for the CA, +end_message=Done configuring directory server for the CA, +show_service_name=True, runtime=210) Martin Thanks, glitch fixed. Tomas Ok, I managed to get the patch a proper review. I like the result, in the console, but I still not entirely satisfied with the patch, looks over-engineered to me + there is a lot of duplication with Configuring %(service)s and Done configuring %(service)s messages. These messages could be easily generated only based on name of a service (self.service_name, we got that) and a new friendly service name or description. If we add description this way: class KrbInstance(service.Service): def __init__(self, fstore=None): service.Service.__init__(self, krb5kdc, description=Kerberos KDC) ... the start_creation could be very simple: ... self.start_creation(runtime=30) ... All messages could be simply generated by the Service class just based on self.service_name and self.description with having the same output as you do now. Martin I simplified the approach as you suggested. However, I kept optional start_message and end_message parameters in case we would want to specify the messages instead of using the pre-generated ones. This is used in baseupdate.py and upgradeinstance.py More info in the documentation of start_creation() function. Tomas NACK. Tried running ./make-lint with your patch, got thousands of errors... More like one error thousand times :) I somehow botched up rebasing the patch, I swear that I corrected that indentation issue beforehand. Nevermind, thanks for checking. ./make-lint runs clean now :) But the approach looks OK, I am just thinking that default value for show_service_name of start_creation() should be True as it is the most widely used value - you would not have to specify it everytime you call start_creation() in *instance.py files. Martin Default values switched. Tomas From 6b510c128aec1d78e6326a96be959cd33b06df8a Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Thu, 11 Oct 2012 03:32:17 -0400 Subject: [PATCH] Make service naming in ipa-server-install consistent Forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. See documentation for more info. https://fedorahosted.org/freeipa/ticket/3059 --- ipaserver/install/adtrustinstance.py| 4 +-- ipaserver/install/bindinstance.py | 9 +-- ipaserver/install/cainstance.py | 18 ++--- ipaserver/install/dsinstance.py | 11 +--- ipaserver/install/httpinstance.py | 4 +-- ipaserver/install/krbinstance.py| 6 ++--- ipaserver/install/ntpinstance.py| 4 +-- ipaserver/install/plugins/baseupdate.py | 3 ++- ipaserver/install/service.py| 45 - ipaserver/install/upgradeinstance.py| 3 ++- 10 files changed, 80 insertions(+), 27 deletions(-) diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index b3602ddce7fa1c184bdf1283e67b50463e49c5fd..c27fac99cf624ca6460ce84e76be52db38f11a5b 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -132,7 +132,7 @@ class ADTRUSTInstance(service.Service): self.rid_base = None self.secondary_rid_base = None -service.Service.__init__(self, smb, dm_password=None, ldapi=True) +service.Service.__init__(self, smb, service_desc=CIFS, dm_password=None, ldapi=True) if fstore: self.fstore = fstore @@ -757,7 +757,7 @@ class ADTRUSTInstance(service.Service): self.step(adding SIDs to existing users and groups, self.__add_sids) -self.start_creation(Configuring CIFS:) +self.start_creation(show_service_name=False) def uninstall(self): if self.is_configured(): diff --git a/ipaserver/install/bindinstance.py
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
Petr Spacek wrote: On 10/19/2012 10:10 AM, Martin Kosek wrote: On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request IMHO there should be a --force option. SAN can contain a lot of different things. Also, we can't assume that we manage the whole world ... (now :-)) The intention was just to provide support for DNS alt names. I don't think requiring a host entry exist for any alt hosts is asking too much. I think a new ticket should be opened to support non-DNS alt names. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent
On 10/19/2012 02:49 PM, Tomas Babej wrote: On 10/19/2012 01:44 PM, Martin Kosek wrote: On 10/19/2012 01:26 PM, Tomas Babej wrote: On 10/18/2012 11:27 AM, Martin Kosek wrote: On 10/11/2012 05:11 PM, Tomas Babej wrote: On 10/11/2012 12:32 PM, Martin Kosek wrote: On 10/11/2012 12:26 PM, Tomas Babej wrote: Hi, This patch forces more consistency into ipa-server-install output. All descriptions of services that are not instances of SimpleServiceInstance are now in the following format: Description (Service Name) Furthermore, start_creation method has been modified to support custom start and end messages. Sample output produced by this patch attached. https://fedorahosted.org/freeipa/ticket/3059 Tomas Just based on reading the patch, this does not look right: -self.start_creation(Configuring certificate server, 210) +self.start_creation(Configuring directory server for the CA, +end_message=Done configuring directory server for the CA, +show_service_name=True, runtime=210) Martin Thanks, glitch fixed. Tomas Ok, I managed to get the patch a proper review. I like the result, in the console, but I still not entirely satisfied with the patch, looks over-engineered to me + there is a lot of duplication with Configuring %(service)s and Done configuring %(service)s messages. These messages could be easily generated only based on name of a service (self.service_name, we got that) and a new friendly service name or description. If we add description this way: class KrbInstance(service.Service): def __init__(self, fstore=None): service.Service.__init__(self, krb5kdc, description=Kerberos KDC) ... the start_creation could be very simple: ... self.start_creation(runtime=30) ... All messages could be simply generated by the Service class just based on self.service_name and self.description with having the same output as you do now. Martin I simplified the approach as you suggested. However, I kept optional start_message and end_message parameters in case we would want to specify the messages instead of using the pre-generated ones. This is used in baseupdate.py and upgradeinstance.py More info in the documentation of start_creation() function. Tomas NACK. Tried running ./make-lint with your patch, got thousands of errors... More like one error thousand times :) I somehow botched up rebasing the patch, I swear that I corrected that indentation issue beforehand. Nevermind, thanks for checking. ./make-lint runs clean now :) But the approach looks OK, I am just thinking that default value for show_service_name of start_creation() should be True as it is the most widely used value - you would not have to specify it everytime you call start_creation() in *instance.py files. Martin Default values switched. Tomas NACK. Server installation crashed with the first step: # ipa-server-install Configuring NTP daemon (ntpd) [1/4]: stopping ntpd [2/4]: writing configuration [3/4]: configuring ntpd to start on boot [4/4]: starting ntpd Unexpected error - see /var/log/ipaserver-install.log for details: TypeError: expected a character buffer object ipaserver-install.log: 2012-10-19T12:59:37Z INFO File /usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, line 614, in run_script return_value = main_function() File /sbin/ipa-server-install, line 904, in main ntp.create_instance() File /usr/lib/python2.7/site-packages/ipaserver/install/ntpinstance.py, line 158, in create_instance self.start_creation() File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line 358, in start_creation self.print_msg(end_message) File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line 295, in print_msg print_msg(message, self.output_fd) File /usr/lib/python2.7/site-packages/ipaserver/install/service.py, line 60, in print_msg output_fd.write(message) 2012-10-19T12:59:37Z INFO The ipa-server-install command failed, exception: TypeError: expected a character buffer object Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
Petr Viktorin wrote: On 10/19/2012 08:15 AM, Martin Kosek wrote: On 10/18/2012 05:51 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 05:22 PM, Rob Crittenden wrote: Martin Kosek wrote: Report errors just like with ipa-ldap-updater. These messages should warn user that some parts of the upgrades may have not been successful and he should follow up on them. Otherwise, user may not notice them at all. ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater logging - only error level or higher severity log messages are printed to stderr, with the same console format. A new debug log that an upgrade script has started was added to make log investigation easier. https://fedorahosted.org/freeipa/ticket/3157 --- Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update will report errors to user: # rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 14%] 2:freeipa-client ### [ 29%] 3:freeipa-admintools ### [ 43%] 4:freeipa-server ### [ 57%] ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission denied: '/etc/named.conf' 5:freeipa-server-selinux ### [ 71%] 6:freeipa-server-trust-ad### [ 86%] 7:freeipa-debuginfo ### [100%] Martin Can you add an option to run in info mode (the equivalent of verbose=True)? Otherwise running from the cli returns nothing unless there is an error, I like some amount of output myself. rob ipa-upgradeconfig has a --debug option. I doubt that the output can be more verbose :-) Martin Yeah, but there is no middle ground. You either get everything or nothing. rob Well, I can add it if you want it really hard. But from my perspective, having both --debug and --verbose is strange and it will also make it inconsistent with our other CLI tools where we have just --debug option (ipa-ldap-updater, ipa-replica-prepare, ipa-*-install). I like ipa-replica-conncheck's approach: --debug (output everything) and --quiet (only output errors). I guess I'm sad because we spent a fair bit of time adding pretty output to the script in non-debug mode which we are not suppressing. ACK on quiet mode. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 325 Create reverse zone in unattended mode
Previous fix for ticket #3161 caused ipa-{server,dns}-install to
skip creation of reverse zone when running in unattended mode. Make
sure that reverse zone is created also in unattended mode (unless
--no-reverse is specified).
https://fedorahosted.org/freeipa/ticket/3161
From d9c06a5bb374962b004bd7f89e2b621eb9fefac7 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Fri, 19 Oct 2012 15:34:49 +0200
Subject: [PATCH] Create reverse zone in unattended mode
Previous fix for ticket #3161 caused ipa-{server,dns}-install to
skip creation of reverse zone when running in unattended mode. Make
sure that reverse zone is created also in unattended mode (unless
--no-reverse is specified).
https://fedorahosted.org/freeipa/ticket/3161
---
install/tools/ipa-dns-install| 4 +++-
install/tools/ipa-server-install | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index b0c20c533d559477d4906195a787fda95a61902e..71592d4899d702606b33e0ac89592d91f99c5e29 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -213,7 +213,9 @@ def main():
else:
reverse_zone = bindinstance.find_reverse_zone(ip)
if reverse_zone is None and not options.no_reverse:
-if not options.unattended and bindinstance.create_reverse():
+if options.unattended:
+reverse_zone = util.get_reverse_zone_default(ip)
+elif bindinstance.create_reverse():
reverse_zone = util.get_reverse_zone_default(ip)
reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip)
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index cc25fb85500b6a97e71c822a882cf6f2aca4f0a0..6d1e6998ca4accbcf3900d5f97f889290e330c6b 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -816,7 +816,9 @@ def main():
if options.reverse_zone:
reverse_zone = bindinstance.normalize_zone(options.reverse_zone)
elif not options.no_reverse:
-if not options.unattended and bindinstance.create_reverse():
+if options.unattended:
+reverse_zone = util.get_reverse_zone_default(ip)
+elif bindinstance.create_reverse():
reverse_zone = util.get_reverse_zone_default(ip)
reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip)
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 325 Create reverse zone in unattended mode
Martin Kosek wrote:
Previous fix for ticket #3161 caused ipa-{server,dns}-install to
skip creation of reverse zone when running in unattended mode. Make
sure that reverse zone is created also in unattended mode (unless
--no-reverse is specified).
https://fedorahosted.org/freeipa/ticket/3161
ACK
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
On 10/19/2012 03:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 10/19/2012 10:10 AM, Martin Kosek wrote: On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request IMHO there should be a --force option. SAN can contain a lot of different things. Also, we can't assume that we manage the whole world ... (now :-)) The intention was just to provide support for DNS alt names. I don't think requiring a host entry exist for any alt hosts is asking too much. I think a new ticket should be opened to support non-DNS alt names. IMHO SAN names usually contain a lot of virtual names like www.shop1.com, ftp.shop1.com, etc. These names are usually CNAMEs to real name like srv1.shop1.com. In that case host object doesn't make sense. (But SAN is required for proper certificate validation.) Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
Petr Spacek wrote: On 10/19/2012 03:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 10/19/2012 10:10 AM, Martin Kosek wrote: On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request IMHO there should be a --force option. SAN can contain a lot of different things. Also, we can't assume that we manage the whole world ... (now :-)) The intention was just to provide support for DNS alt names. I don't think requiring a host entry exist for any alt hosts is asking too much. I think a new ticket should be opened to support non-DNS alt names. IMHO SAN names usually contain a lot of virtual names like www.shop1.com, ftp.shop1.com, etc. These names are usually CNAMEs to real name like srv1.shop1.com. In that case host object doesn't make sense. (But SAN is required for proper certificate validation.) The purpose is so we more tightly control was certificates are issued by our CA because we automatically issue them. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
On 10/19/2012 03:23 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 10/19/2012 08:15 AM, Martin Kosek wrote:
On 10/18/2012 05:51 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 10/18/2012 05:22 PM, Rob Crittenden wrote:
Martin Kosek wrote:
Report errors just like with ipa-ldap-updater. These messages
should warn
user that some parts of the upgrades may have not been successful and
he should follow up on them. Otherwise, user may not notice them
at all.
ipa-upgradeconfig logging has been made consistent with
ipa-ldap-updater
logging - only error level or higher severity log messages are
printed
to stderr, with the same console format. A new debug log that an
upgrade
script has started was added to make log investigation easier.
https://fedorahosted.org/freeipa/ticket/3157
---
Reproducer is in Bugzilla attached to the ticket. With this patch,
RPM update
will report errors to user:
# rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-*
Preparing...
### [100%]
1:freeipa-python
### [
14%]
2:freeipa-client
### [
29%]
3:freeipa-admintools
### [
43%]
4:freeipa-server
### [
57%]
ERROR: Cannot update connections in /etc/named.conf: [Errno 13]
Permission
denied: '/etc/named.conf'
5:freeipa-server-selinux
### [
71%]
6:freeipa-server-trust-ad###
[
86%]
7:freeipa-debuginfo
###
[100%]
Martin
Can you add an option to run in info mode (the equivalent of
verbose=True)?
Otherwise running from the cli returns nothing unless there is an
error, I like
some amount of output myself.
rob
ipa-upgradeconfig has a --debug option. I doubt that the output can
be more
verbose :-)
Martin
Yeah, but there is no middle ground. You either get everything or
nothing.
rob
Well, I can add it if you want it really hard. But from my
perspective, having
both --debug and --verbose is strange and it will also make it
inconsistent
with our other CLI tools where we have just --debug option
(ipa-ldap-updater,
ipa-replica-prepare, ipa-*-install).
I like ipa-replica-conncheck's approach: --debug (output everything) and
--quiet (only output errors).
I guess I'm sad because we spent a fair bit of time adding pretty output to
the
script in non-debug mode which we are not suppressing.
ACK on quiet mode.
rob
Ok, you won, this actually makes sense - added a --quiet mode. I also played a
bit with console level formatting and print the level only in --debug mode.
Martin
From 95366eb5f5685415be348c64f15308ed19ee7c71 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 17 Oct 2012 13:05:24 +0200
Subject: [PATCH] Report ipa-upgradeconfig errors during RPM upgrade
Report errors just like with ipa-ldap-updater. These messages should warn
user that some parts of the upgrades may have not been successful and
he should follow up on them. Otherwise, user may not notice them at all.
ipa-upgradeconfig now has a new --quiet option to make it output only error
level log messages or higher. ipa-upgradeconfig run without options still
pring INFO log messages as it can provide a clean overview about its
actions (unlike ipa-ldap-updater).
https://fedorahosted.org/freeipa/ticket/3157
---
freeipa.spec.in | 5 -
install/tools/ipa-upgradeconfig | 15 ---
install/tools/man/ipa-upgradeconfig.8 | 4
3 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 060f09b12e6891defc8cb00d4f52a0d019198a70..916630029f6dfac8ef32dabb00f338052cbbf08e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -493,7 +493,7 @@ if [ $1 = 1 ]; then
fi
%endif
if [ $1 -gt 1 ] ; then
-/usr/sbin/ipa-upgradeconfig /dev/null 21 || :
+/usr/sbin/ipa-upgradeconfig --quiet /dev/null || :
fi
%posttrans server
@@ -815,6 +815,9 @@ fi
%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
%changelog
+* Wed Oct 17 2012 Martin Kosek mko...@redhat.com - 2.99.0-51
+- Print ipa-upgradeconfig errors during RPM update
+
* Wed Oct 10 2012 Alexander Bokovoy aboko...@redhat.com - 2.99.0-50
- Make sure server-trust-ad subpackage alternates winbind_krb5_locator.so
plugin to /dev/null since they cannot be used when trusts are configured
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 38426149864017789614095445f947a997d63b3c..14d4e0829162ab78665f794c582e704b5901ea41 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -61,6 +61,9 @@ def parse_options():
parser = IPAOptionParser(version=version.VERSION)
parser.add_option(-d, --debug,
Re: [Freeipa-devel] [PATCH] 1066 requesting certs with subject alt name
On 10/19/2012 03:46 PM, Rob Crittenden wrote: Petr Spacek wrote: On 10/19/2012 03:10 PM, Rob Crittenden wrote: Petr Spacek wrote: On 10/19/2012 10:10 AM, Martin Kosek wrote: On 10/18/2012 09:42 PM, Rob Crittenden wrote: We were seeing a unicode failure when trying to request a certificate with subject alt names. This one-liner should fix it. rob Yup, this fixes it, works fine on --selfsign IPA CA too. Just when testing your patch, I found out we don't treat some non-DNS subject alternative name well, e.g. email extension, an we try to match it with our hosts: Certificate Request: ... Attributes: Requested Extensions: X509v3 Subject Alternative Name: email:f...@testcert.example.com, DNS:web.example.com ... cert-request result: ipa: ERROR: no host record for subject alt name f...@testcert.example.com in certificate request IMHO there should be a --force option. SAN can contain a lot of different things. Also, we can't assume that we manage the whole world ... (now :-)) The intention was just to provide support for DNS alt names. I don't think requiring a host entry exist for any alt hosts is asking too much. I think a new ticket should be opened to support non-DNS alt names. IMHO SAN names usually contain a lot of virtual names like www.shop1.com, ftp.shop1.com, etc. These names are usually CNAMEs to real name like srv1.shop1.com. In that case host object doesn't make sense. (But SAN is required for proper certificate validation.) The purpose is so we more tightly control was certificates are issued by our CA because we automatically issue them. rob I opened a ticket for better Subject Alternative Name check: https://fedorahosted.org/freeipa/ticket/3196 ACK for patch 1066. Pushed to master, ipa-3-0. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 325 Create reverse zone in unattended mode
On 10/19/2012 03:40 PM, Rob Crittenden wrote:
Martin Kosek wrote:
Previous fix for ticket #3161 caused ipa-{server,dns}-install to
skip creation of reverse zone when running in unattended mode. Make
sure that reverse zone is created also in unattended mode (unless
--no-reverse is specified).
https://fedorahosted.org/freeipa/ticket/3161
ACK
Pushed to master, ipa-3-0.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
On 10/19/2012 04:06 PM, Martin Kosek wrote: On 10/19/2012 03:23 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 10/19/2012 08:15 AM, Martin Kosek wrote: On 10/18/2012 05:51 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 05:22 PM, Rob Crittenden wrote: Martin Kosek wrote: Report errors just like with ipa-ldap-updater. These messages should warn user that some parts of the upgrades may have not been successful and he should follow up on them. Otherwise, user may not notice them at all. ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater logging - only error level or higher severity log messages are printed to stderr, with the same console format. A new debug log that an upgrade script has started was added to make log investigation easier. https://fedorahosted.org/freeipa/ticket/3157 --- Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update will report errors to user: # rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 14%] 2:freeipa-client ### [ 29%] 3:freeipa-admintools ### [ 43%] 4:freeipa-server ### [ 57%] ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission denied: '/etc/named.conf' 5:freeipa-server-selinux ### [ 71%] 6:freeipa-server-trust-ad### [ 86%] 7:freeipa-debuginfo ### [100%] Martin Can you add an option to run in info mode (the equivalent of verbose=True)? Otherwise running from the cli returns nothing unless there is an error, I like some amount of output myself. rob ipa-upgradeconfig has a --debug option. I doubt that the output can be more verbose :-) Martin Yeah, but there is no middle ground. You either get everything or nothing. rob Well, I can add it if you want it really hard. But from my perspective, having both --debug and --verbose is strange and it will also make it inconsistent with our other CLI tools where we have just --debug option (ipa-ldap-updater, ipa-replica-prepare, ipa-*-install). I like ipa-replica-conncheck's approach: --debug (output everything) and --quiet (only output errors). I guess I'm sad because we spent a fair bit of time adding pretty output to the script in non-debug mode which we are not suppressing. ACK on quiet mode. rob Ok, you won, this actually makes sense - added a --quiet mode. I also played a bit with console level formatting and print the level only in --debug mode. Nice idea. I think it would be useful if standard_logging_setup did this by default (though this patch is probably not the place for standard_logging_setup changes). -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0092 Make sure the CA is running when starting services
Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/3084 See ticket commit message. Please tell me of a better way to extend the Services. What's interesting is that usually the CA is running right after the ports are opened, but if not, it takes *exactly* one minute between the ports being open and the time I stop getting 503 Service Temporarily Unavailable from ca/admin/ca/getStatus. Is there a sleep somewhere in pki? or httpd? or IPΑ? No sleep that I know of, and I'm not seeing that behavior. In my testing I got 503 exactly once. Most of the time once the port(s) were open and the request went through the status was that dogtag was up and ready. Just a few minor requests. Can you add a block comment to ca_status? I think particularly explaining why port 443 and not a CA port directly (I assume so we test the proxy). I'm a little confused by the wait variable. It is a boolean in some cases and a string in others (no-proxy)? Why not just pass in False? The patch itself looks good. I'm having a replica install problem which I'm guessing is unrelated. The configure proxy step is failing to restart httpd. It is failing because the default mod_nss port is 8443 which is also being used by dogtag, so httpd fails to restart and the installation blows up. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0093 ipa-replica-install: Use configured IPA DNS servers in forward/reverse resolution check
Fix a regression in one of my earlier patches.
https://fedorahosted.org/freeipa/ticket/3194
--
Petr³
From 3223c4e3bb7bec0c3daf8cbd36a6d72b339da0e3 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 19 Oct 2012 12:22:33 -0400
Subject: [PATCH] ipa-replica-install: Use configured IPA DNS servers in
forward/reverse resolution check
Previously, ipa-replica-install tried to check DNS resolution on the master
being cloned. If that master was not a DNS server, the check failed.
Change the check to query the first available configured DNS server.
Log about the check before actually running it.
Log in the case the check is skipped (no IPA DNS servers installed).
https://fedorahosted.org/freeipa/ticket/3194
---
install/tools/ipa-replica-install | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index b56fa2ea5826b6fe25e6db02c7e640e50bca0790..e39698914d066509279b9729212a58621bf95753 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -313,12 +313,23 @@ def check_bind():
sys.exit(1)
-def check_dns_resolution(host_name, dns_server):
-Check forward and reverse resolution of host_name using dns_server
+def check_dns_resolution(host_name, dns_servers):
+Check forward and reverse resolution of host_name using dns_servers
# Point the resolver at specified DNS server
-server_ips = list(
-a[4][0] for a in socket.getaddrinfo(dns_server, None))
+server_ips = []
+for dns_server in dns_servers:
+try:
+server_ips = list(
+a[4][0] for a in socket.getaddrinfo(dns_server, None))
+except socket.error:
+pass
+else:
+break
+if not server_ips:
+root_logger.error(
+'Could not resolve any DNS server hostname: %s', dns_servers)
+return False
resolver = dns.resolver.Resolver()
resolver.nameservers = server_ips
@@ -547,15 +558,18 @@ def main():
config.master_host_name, config.dirman_password):
dns_masters = api.Object['dnsrecord'].get_dns_masters()
if dns_masters:
-master = config.master_host_name
if not options.no_host_dns:
-resolution_ok = (
-check_dns_resolution(master, master) and
-check_dns_resolution(config.host_name, master))
+master = config.master_host_name
root_logger.debug('Check forward/reverse DNS resolution')
+resolution_ok = (
+check_dns_resolution(master, dns_masters) and
+check_dns_resolution(config.host_name, dns_masters))
if not resolution_ok and not options.unattended:
if not ipautil.user_input(Continue?, False):
sys.exit(0)
+else:
+root_logger.debug('No IPA DNS servers, '
+'skipping forward/reverse resolution check')
# Check that we don't already have a replication agreement
try:
--
1.7.11.7
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. Yeah I have been thinking about that. I am thinking of changing the patch so that it's the service class itself that saves this data. That way saving is always performed no matter 'what' starts the service. Do you agree ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 324 Add fallback for httpd restarts
Martin Kosek wrote: On 10/18/2012 04:36 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 02:47 PM, Rob Crittenden wrote: Martin Kosek wrote: Attaching a script I used to reproduce the issue on machine with sysV (RHEL 6.4 in my case). With the patch applied, httpd restarts correctly fallback-ed. If you think that the wait is not enough, I can add a more complicated procedure, like this one: wait_time = 5 retries = 3 for x in xrange(retries): try: sleep(wait_time) http.stop() sleep(wait_time) http.start() except CalledProcessError: wait_time = wait_time * 2 continue break httpd init script on sysV based platforms cannot guarantee that two consecutive httpd service restarts succeed when run in a small time distance. Add fallback procedure that adds additional waiting time after such failed restart attempt, and then try to stop and start the service again. https://fedorahosted.org/freeipa/ticket/2965 Should we attempt to retrieve a file to ensure that the service is up? The ipa.crt would be a candidate for this. rob Hm, this looks over-engineered from my POV. We already check that ports are open, right? Well, this is assuming that sysV is going to return an error when httpd doesn't start. It should at least call service status to make sure the service is operational. rob Actually, we generally already do that as a part of start() process, you will see it when running my test script: DEBUG: stderr= DEBUG: args=/sbin/service httpd restart DEBUG: stdout=Stopping httpd: [FAILED] Starting httpd:[FAILED] DEBUG: stderr=(98)Address already in use: make_sock: could not bind to address [::]:80 (98)Address already in use: make_sock: could not bind to address 0.0.0.0:80 no listening sockets available, shutting down Unable to open logs DEBUG: httpd restart failed, try to stopstart again DEBUG: args=/sbin/service httpd stop DEBUG: stdout=Stopping httpd: [ OK ] DEBUG: stderr= DEBUG: args=/sbin/service httpd start DEBUG: stdout=Starting httpd: [ OK ] DEBUG: stderr= DEBUG: args=/sbin/service httpd status DEBUG: stdout=httpd dead but subsys locked But I just realized, that my change in httpinstance.py is redundant, it uses platform service to do the restart which has the fallback already, i.e. modification there is enough. Updated patch attached. Martin ack, pushed to master and ipa-3-0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. Yeah I have been thinking about that. I am thinking of changing the patch so that it's the service class itself that saves this data. That way saving is always performed no matter 'what' starts the service. Do you agree ? Yes, that sounds like it'll be very robust. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Report ipa-upgradeconfig errors during RPM upgrade
Martin Kosek wrote: On 10/19/2012 03:23 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 10/19/2012 08:15 AM, Martin Kosek wrote: On 10/18/2012 05:51 PM, Rob Crittenden wrote: Martin Kosek wrote: On 10/18/2012 05:22 PM, Rob Crittenden wrote: Martin Kosek wrote: Report errors just like with ipa-ldap-updater. These messages should warn user that some parts of the upgrades may have not been successful and he should follow up on them. Otherwise, user may not notice them at all. ipa-upgradeconfig logging has been made consistent with ipa-ldap-updater logging - only error level or higher severity log messages are printed to stderr, with the same console format. A new debug log that an upgrade script has started was added to make log investigation easier. https://fedorahosted.org/freeipa/ticket/3157 --- Reproducer is in Bugzilla attached to the ticket. With this patch, RPM update will report errors to user: # rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-* Preparing... ### [100%] 1:freeipa-python ### [ 14%] 2:freeipa-client ### [ 29%] 3:freeipa-admintools ### [ 43%] 4:freeipa-server ### [ 57%] ERROR: Cannot update connections in /etc/named.conf: [Errno 13] Permission denied: '/etc/named.conf' 5:freeipa-server-selinux ### [ 71%] 6:freeipa-server-trust-ad### [ 86%] 7:freeipa-debuginfo ### [100%] Martin Can you add an option to run in info mode (the equivalent of verbose=True)? Otherwise running from the cli returns nothing unless there is an error, I like some amount of output myself. rob ipa-upgradeconfig has a --debug option. I doubt that the output can be more verbose :-) Martin Yeah, but there is no middle ground. You either get everything or nothing. rob Well, I can add it if you want it really hard. But from my perspective, having both --debug and --verbose is strange and it will also make it inconsistent with our other CLI tools where we have just --debug option (ipa-ldap-updater, ipa-replica-prepare, ipa-*-install). I like ipa-replica-conncheck's approach: --debug (output everything) and --quiet (only output errors). I guess I'm sad because we spent a fair bit of time adding pretty output to the script in non-debug mode which we are not suppressing. ACK on quiet mode. rob Ok, you won, this actually makes sense - added a --quiet mode. I also played a bit with console level formatting and print the level only in --debug mode. Martin ACK, pushed to master and ipa-3-0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
