[Freeipa-devel] patch for trac 2575
Martin Kosek
mkosek at redhat.com
Thu Feb 21 13:39:40 UTC 2013
Thanks Brian. I still see few issues though:
1) The patch adds a whitespace error:
$ git apply ~/freeipa-bcook-0001-Add-DNS-Setup-Prompt-to-Install.patch
/home/mkosek/freeipa-bcook-0001-Add-DNS-Setup-Prompt-to-Install.patch:41:
trailing whitespace.
warning: 1 line adds whitespace errors
2) It does unrelated and unnecessary changes to the main function:
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -560,10 +560,16 @@ def set_subject_in_config(realm_name, dm_password,
suffix, subject_base):
conn.disconnect()
def main():
+ """
+
+
+ :return:
+ """
global ds
global pw_name
global uninstalling
global installation_cleanup
+
ds = None
safe_options, options = parse_options()
3) In the question, I would replace "bind" with "BIND" as this is how the
project should be spelled. I see that few lines above we also refer to BIND
with "bind" (it may have caused the confusion), I think this can be fixed too.
Martin
On 02/15/2013 03:07 AM, Brian Cook wrote:
> Thanks Martin and Dmitri. I have attached a patch that I -think- is formatted
> correctly... I removed the new variable and added check for --unattended.
>
> Thanks,
> Brian
>
>
> -------------------------------------------------------------------------------
> *From: *"Martin Kosek" <mkosek at redhat.com>
> *To: *dpal at redhat.com
> *Cc: *freeipa-devel at redhat.com
> *Sent: *Wednesday, February 13, 2013 11:16:51 PM
> *Subject: *Re: [Freeipa-devel] patch for trac 2575
>
> On 02/14/2013 03:49 AM, Dmitri Pal wrote:
>> On 02/13/2013 05:20 PM, Brian Cook wrote:
>>> Please disregard the first patch as it still asked the user if they want to
> install DNS even if --setup-dns was passed, this one is fixed.
>>>
>>> Brian
>>
>> Brian,
>>
>> Thanks for the patch.
>> Can you please format it following these guidelines:
>> https://fedorahosted.org/freeipa/wiki/PatchFormat
>>
>> Thanks
>> Dmitri
>
> Hello Brian,
>
> Thanks for the patch! Also few technical notes:
>
> 1) There is no need to invent the new variable, you can ask and set
> options.setup_dns to True. We already to this in other parts incode
>
> 2) This patch would --unattended mode when no --setup-dns is passed
>
> Martin
>
>>>
>>>
>>>
>>> diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
>>> index 1559107..96ef802 100755
>>> --- a/install/tools/ipa-server-install
>>> +++ b/install/tools/ipa-server-install
>>> @@ -564,6 +564,7 @@ def main():
>>> global pw_name
>>> global uninstalling
>>> global installation_cleanup
>>> +
>>> ds = None
>>>
>>> safe_options, options = parse_options()
>>> @@ -740,8 +741,18 @@ def main():
>>> admin_password = ""
>>> reverse_zone = None
>>>
>>> - # check bind packages are installed
>>> + # Setup a variable to use instead of options.setup_dns to enable
> interactive DNS selection
>>> + setup_dns=False
>>> if options.setup_dns:
>>> + setup_dns=True
>>> + else:
>>> + # Ask user if they want to install DNS
>>> + if ipautil.user_input("Do you want to configure integrated DNS
> (bind)?", False):
>>> + setup_dns=True
>>> +
>>> +
>>> + # check bind packages are installed
>>> + if setup_dns:
>>> if not bindinstance.check_inst(options.unattended):
>>> sys.exit("Aborting installation")
>>>
>>> @@ -827,7 +838,7 @@ def main():
>>> else:
>>> admin_password = options.admin_password
>>>
>>> - if options.setup_dns:
>>> + if setup_dns:
>>> if options.no_forwarders:
>>> dns_forwarders = ()
>>> elif options.forwarders:
>>> @@ -858,7 +869,7 @@ def main():
>>> print "Realm name: %s" % realm_name
>>> print
>>>
>>> - if options.setup_dns:
>>> + if setup_dns:
>>> print "BIND DNS server will be configured to serve IPA domain with:"
>>> print "Forwarders: %s" % ("No forwarders" if not dns_forwarders \
>>> else ", ".join([str(ip) for ip in dns_forwarders]))
>>> @@ -1102,7 +1113,7 @@ def main():
>>> persistent_search=options.persistent_search,
>>> serial_autoincrement=options.serial_autoincrement,
>>> ca_configured=not options.selfsign)
>>> - if options.setup_dns:
>>> + if setup_dns:
>>> api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')),
> bind_pw=dm_password)
>>>
>>> bind.create_instance()
>>> @@ -1147,11 +1158,11 @@ def main():
>>> print "\t\t * 80, 443: HTTP/HTTPS"
>>> print "\t\t * 389, 636: LDAP/LDAPS"
>>> print "\t\t * 88, 464: kerberos"
>>> - if options.setup_dns:
>>> + if setup_dns:
>>> print "\t\t * 53: bind"
>>> print "\t\tUDP Ports:"
>>> print "\t\t * 88, 464: kerberos"
>>> - if options.setup_dns:
>>> + if setup_dns:
>>> print "\t\t * 53: bind"
>>> if options.conf_ntp:
>>> print "\t\t * 123: ntp"
>>>
>>>
>>>
>>>
>>>> Message: 8
>>>> Date: Wed, 13 Feb 2013 13:39:32 -0800
>>>> From: Brian Cook <bcook at redhat.com>
>>>> To: "freeipa-devel at redhat.com" <freeipa-devel at redhat.com>
>>>> Subject: [Freeipa-devel] patch for trac 2575
>>>> Message-ID: <9DD1D1BB-6B86-4EA1-B61B-B208E6BC7152 at redhat.com>
>>>> Content-Type: text/plain; charset="windows-1252"
>>>>
>>>> This is a patch for ticket 2575 on trac: [RFE] Installer wizard should
> prompt for DNS. This is my first time submitting a patch so I was looking for
> something that seemed relatively easy?
>>>>
>>>> Thanks,
>>>> Brian
>>>>
>>>>
>>>> diff --git a/install/tools/ipa-server-install
> b/install/tools/ipa-server-install
>>>> index 1559107..d8c4ae5 100755
>>>> --- a/install/tools/ipa-server-install
>>>> +++ b/install/tools/ipa-server-install
>>>> @@ -564,6 +564,7 @@ def main():
>>>> global pw_name
>>>> global uninstalling
>>>> global installation_cleanup
>>>> +
>>>> ds = None
>>>>
>>>> safe_options, options = parse_options()
>>>> @@ -740,8 +741,18 @@ def main():
>>>> admin_password = ""
>>>> reverse_zone = None
>>>>
>>>> - # check bind packages are installed
>>>> + # Setup a variable to use instead of options.setup_dns to enable
> interactive DNS selection
>>>> + setup_dns=False
>>>> if options.setup_dns:
>>>> + setup_dns=True
>>>> +
>>>> + # Ask user if they want to install DNS
>>>> + if ipautil.user_input("Do you want to cnfigure integrated DNS
> (bind)?", false):
>>>> + setup_dns=True
>>>> +
>>>> +
>>>> + # check bind packages are installed
>>>> + if setup_dns:
>>>> if not bindinstance.check_inst(options.unattended):
>>>> sys.exit("Aborting installation")
>>>>
>>>> @@ -827,7 +838,7 @@ def main():
>>>> else:
>>>> admin_password = options.admin_password
>>>>
>>>> - if options.setup_dns:
>>>> + if setup_dns:
>>>> if options.no_forwarders:
>>>> dns_forwarders = ()
>>>> elif options.forwarders:
>>>> @@ -858,7 +869,7 @@ def main():
>>>> print "Realm name: %s" % realm_name
>>>> print
>>>>
>>>> - if options.setup_dns:
>>>> + if setup_dns:
>>>> print "BIND DNS server will be configured to serve IPA domain with:"
>>>> print "Forwarders: %s" % ("No forwarders" if not dns_forwarders \
>>>> else ", ".join([str(ip) for ip in dns_forwarders]))
>>>> @@ -1102,7 +1113,7 @@ def main():
>>>> persistent_search=options.persistent_search,
>>>> serial_autoincrement=options.serial_autoincrement,
>>>> ca_configured=not options.selfsign)
>>>> - if options.setup_dns:
>>>> + if setup_dns:
>>>> api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')),
> bind_pw=dm_password)
>>>>
>>>> bind.create_instance()
>>>> @@ -1147,11 +1158,11 @@ def main():
>>>> print "\t\t * 80, 443: HTTP/HTTPS"
>>>> print "\t\t * 389, 636: LDAP/LDAPS"
>>>> print "\t\t * 88, 464: kerberos"
>>>> - if options.setup_dns:
>>>> + if setup_dns:
>>>> print "\t\t * 53: bind"
>>>> print "\t\tUDP Ports:"
>>>> print "\t\t * 88, 464: kerberos"
>>>> - if options.setup_dns:
>>>> + if setup_dns:
>>>> print "\t\t * 53: bind"
>>>> if options.conf_ntp:
>>>> print "\t\t * 123: ntp"
>>>>
>>>>
>>>>
>>>>
>>>> -------------- next part --------------
>>>> An HTML attachment was scrubbed...
>>>> URL:
> <https://www.redhat.com/archives/freeipa-devel/attachments/20130213/8be3e343/attachment.html>
>>>>
>>>> ------------------------------
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> End of Freeipa-devel Digest, Vol 69, Issue 49
>>>> *********************************************
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
More information about the Freeipa-devel
mailing list