On 03/07/15 09:03, Tomas Babej wrote:

On 07/02/2015 02:03 PM, Petr Spacek wrote:
On 2.7.2015 13:54, Jan Cholasta wrote:
Dne 2.7.2015 v 13:34 Petr Spacek napsal(a):
On 2.7.2015 12:57, Tomas Babej wrote:

On 07/02/2015 08:50 AM, Petr Spacek wrote:
On 1.7.2015 20:29, Tomas Babej wrote:

On 07/01/2015 04:45 PM, Petr Spacek wrote:
On 1.7.2015 15:32, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/4058
Requires patch freeipa-pspacek-0052
ACK

I must admit I don't really like wrapping a constant in the method in
the TaskNamespace object.

We're interested in the constant itself - there's no case I can imagine
where the name of the freeipa's dns package will be dynamic.

For paths we have BasePathNamespace that contains all the paths, maybe
we should introduce something similar for the non-path platform
dependent constants?
Generally I support this but it seems like a 4.3 material (and out of
scope of
#4058). We need to finish 4.2 now.

Please ACK or NACK ASAP.

It's fairly straightforward to introduce a new platform namespace for
constants.

See attached patch, it implements the namespace and already contains the
proper values for the dns package name.

The original patch 274 would only need to use:

      >>> from ipaplatform.constants import constants
      >>> constants.DNS_PACKAGE_NAME
      'freeipa-server-dns'
I'm okay with that if Honza or somebody else knowledgable about the whole
platform-thingy can ACK this, amend Martin^2's patch 274 and test the whole
thing.

Unfortunately I do not have time for it myself. If nobody does that please
push the original patch (when it's dependency pspacek-0052 gets ACK).

I think you are overengineering this a little bit, adding whatever ipaplatform
stuff just because of an error message seems rather unnecessary to me. I think
changing the error message to "Integrated DNS requires 'freeipa-server-dns'
package" or even "Integrated DNS requires IPA DNS server package" would be
perfectly fine.
The message should be as specific as possible but I do not care how it will be
implemented.

Alright, let's not get stuck. Petr insists on specific message on each
platform. Given that package name is platform dependent, I think we
should keep it as platform constant, task makes little sense.

Given that Martin's not available right now, I'll amend his patches and
send the updated version.

Tomas
Updated patches attached.

ACK for 332
I just removed DNS constants from 332 patch

--
Martin Basti

From 4194a9df1e384b8713ecc92d1b8563c322e30dcf Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 1 Jul 2015 15:05:45 +0200
Subject: [PATCH 2/2] DNS: check if DNS package is installed

Instead of separate checking of DNS required packages, we need just
check if IPA DNS package is installed.

https://fedorahosted.org/freeipa/ticket/4058
---
 ipaplatform/base/constants.py           |  2 +-
 ipaplatform/base/paths.py               |  1 +
 ipaplatform/rhel/constants.py           |  2 +-
 ipaserver/install/bindinstance.py       | 19 +------------------
 ipaserver/install/dns.py                | 11 ++++++-----
 ipaserver/install/dnskeysyncinstance.py |  6 ------
 ipaserver/install/opendnssecinstance.py |  8 --------
 7 files changed, 10 insertions(+), 39 deletions(-)

diff --git a/ipaplatform/base/constants.py b/ipaplatform/base/constants.py
index 70485055fa5a12fac878ace3dea11ea442ebe6be..cef829e2d3886db00ae6d0299ddcf325d1add80e 100644
--- a/ipaplatform/base/constants.py
+++ b/ipaplatform/base/constants.py
@@ -8,4 +8,4 @@ This base platform module exports platform dependant constants.
 
 
 class BaseConstantsNamespace(object):
-    pass
+    IPA_DNS_PACKAGE_NAME = "freeipa-server-dns"
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 9fef3e7a1351dd42895fe560bb3c1bc5a1c852b4..6ca4965d3deab61030d09b4b07582ff27e6af120 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -218,6 +218,7 @@ class BasePathNamespace(object):
     GROUPADD = "/usr/sbin/groupadd"
     HTTPD = "/usr/sbin/httpd"
     IPA_CLIENT_INSTALL = "/usr/sbin/ipa-client-install"
+    IPA_DNS_INSTALL = "/usr/sbin/ipa-dns-install"
     SBIN_IPA_JOIN = "/usr/sbin/ipa-join"
     IPA_REPLICA_CONNCHECK = "/usr/sbin/ipa-replica-conncheck"
     IPA_RMKEYTAB = "/usr/sbin/ipa-rmkeytab"
diff --git a/ipaplatform/rhel/constants.py b/ipaplatform/rhel/constants.py
index eaca48030fa28804c70c161b07228646a95fc1a3..17abde1f861778bec83067cb01e9a1faae325527 100644
--- a/ipaplatform/rhel/constants.py
+++ b/ipaplatform/rhel/constants.py
@@ -11,6 +11,6 @@ from ipaplatform.redhat.constants import RedHatConstantsNamespace
 
 
 class RHELConstantsNamespace(RedHatConstantsNamespace):
-    pass
+    IPA_DNS_PACKAGE_NAME = "ipa-server-dns"
 
 constants = RHELConstantsNamespace()
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 2228342dc40ee415d1adf2687a7ae91a5963d3c7..9705e845a76191a252bfa963b54d9c31d83ad18e 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -62,25 +62,8 @@ named_conf_arg_options_template_nonstr = "%(indent)s%(name)s %(value)s;\n"
 named_conf_include_re = re.compile(r'\s*include\s+"(?P<path>)"\s*;')
 named_conf_include_template = "include \"%(path)s\";\n"
 
+
 def check_inst(unattended):
-    has_bind = True
-    named = services.knownservices.named
-    if not os.path.exists(named.get_binary_path()):
-        print "BIND was not found on this system"
-        print ("Please install the '%s' package and start the installation again"
-              % named.get_package_name())
-        has_bind = False
-
-    # Also check for the LDAP BIND plug-in
-    if not os.path.exists(paths.BIND_LDAP_SO) and \
-       not os.path.exists(paths.BIND_LDAP_SO_64):
-        print "The BIND LDAP plug-in was not found on this system"
-        print "Please install the 'bind-dyndb-ldap' package and start the installation again"
-        has_bind = False
-
-    if not has_bind:
-        return False
-
     if not unattended and os.path.exists(NAMED_CONF):
         msg = "Existing BIND configuration detected, overwrite?"
         return ipautil.user_input(msg, False)
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index d22bce7a7cd2e0e8a7ffe0ab4aa496634465903b..9430d189978b0984b0b71d7d754516a4135053fb 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -9,6 +9,7 @@ from subprocess import CalledProcessError
 from ipalib import api
 from ipalib import errors
 from ipaplatform.paths import paths
+from ipaplatform.constants import constants
 from ipaplatform import services
 from ipapython import ipautil
 from ipapython import sysrestore
@@ -96,6 +97,10 @@ def install_check(standalone, replica, options, hostname):
     global reverse_zones
     fstore = sysrestore.FileStore(paths.SYSRESTORE)
 
+    if not ipautil.file_exists(paths.IPA_DNS_INSTALL):
+        raise RuntimeError("Integrated DNS requires '%s' package" %
+                           constants.IPA_DNS_PACKAGE_NAME)
+
     if standalone:
         print "=============================================================================="
         print "This program will setup DNS for the FreeIPA Server."
@@ -141,8 +146,7 @@ def install_check(standalone, replica, options, hostname):
         sys.exit("Aborted")
 
     # Check bind packages are installed
-    if not (bindinstance.check_inst(options.unattended) and
-            dnskeysyncinstance.check_inst()):
+    if not bindinstance.check_inst(options.unattended):
         sys.exit("Aborting installation.")
 
     if options.disable_dnssec_master:
@@ -177,9 +181,6 @@ def install_check(standalone, replica, options, hostname):
             sys.exit("Only one DNSSEC key master is supported in current "
                      "version.")
 
-        # check opendnssec packages are installed
-        if not opendnssecinstance.check_inst():
-            sys.exit("Aborting installation")
         if options.kasp_db_file:
             dnskeysyncd = services.service('ipa-dnskeysyncd')
 
diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index eb6d07f014bce296a5b094f499194286c31c2489..7d1351ccc57a5dbd7d537741545ad44d0dcd5eb1 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -30,12 +30,6 @@ softhsm_token_label = u'ipaDNSSEC'
 softhsm_slot = 0
 replica_keylabel_template = u"dnssec-replica:%s"
 
-def check_inst():
-    if not os.path.exists(paths.DNSSEC_KEYFROMLABEL):
-        print ("Please install the 'bind-pkcs11-utils' package and start "
-               "the installation again")
-        return False
-    return True
 
 def dnssec_container_exists(fqdn, suffix, dm_password=None, ldapi=False,
                             realm=None, autobind=ipaldap.AUTOBIND_DISABLED):
diff --git a/ipaserver/install/opendnssecinstance.py b/ipaserver/install/opendnssecinstance.py
index d68691fa32f135c7527ce28ed771757eadab4831..0f1af828ea245046330fdfab77db130ca14faba3 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -55,14 +55,6 @@ def get_dnssec_key_masters(conn):
     return keymasters_list
 
 
-def check_inst():
-    if not os.path.exists(paths.ODS_KSMUTIL):
-        print ("Please install the 'opendnssec' package and start "
-               "the installation again")
-        return False
-    return True
-
-
 class OpenDNSSECInstance(service.Service):
     def __init__(self, fstore=None, dm_password=None, ldapi=False,
                  start_tls=False, autobind=ipaldap.AUTOBIND_ENABLED):
-- 
2.4.3

From 7f75224bb6c475f59800fa8622eda70b9f18b834 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 2 Jul 2015 12:38:43 +0200
Subject: [PATCH 1/2] ipaplatform: Add constants submodule

Introduce a ipaplatform/constants.py file to store platform related
constants, which are not paths.
---
 Makefile                        |  3 ++-
 freeipa.spec.in                 |  2 ++
 ipaplatform/base/constants.py   | 11 +++++++++++
 ipaplatform/fedora/constants.py | 16 ++++++++++++++++
 ipaplatform/redhat/constants.py | 17 +++++++++++++++++
 ipaplatform/rhel/constants.py   | 16 ++++++++++++++++
 6 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 ipaplatform/base/constants.py
 create mode 100644 ipaplatform/fedora/constants.py
 create mode 100644 ipaplatform/redhat/constants.py
 create mode 100644 ipaplatform/rhel/constants.py

diff --git a/Makefile b/Makefile
index abf58382960099a54b8920dd0e741b9fda17682f..3c81466d3728022c1d9cf5bb216990f14a59b7e5 100644
--- a/Makefile
+++ b/Makefile
@@ -159,10 +159,11 @@ version-update: release-update
 	if [ "$(SUPPORTED_PLATFORM)" != "" ]; then \
 		sed -e s/__PLATFORM__/$(SUPPORTED_PLATFORM)/ \
 			ipaplatform/__init__.py.in > ipaplatform/__init__.py; \
-		rm -f ipaplatform/paths.py ipaplatform/services.py ipaplatform/tasks.py; \
+		rm -f ipaplatform/paths.py ipaplatform/services.py ipaplatform/tasks.py ipaplatform/constants.py; \
 		ln -s $(SUPPORTED_PLATFORM)/paths.py ipaplatform/paths.py; \
 		ln -s $(SUPPORTED_PLATFORM)/services.py ipaplatform/services.py; \
 		ln -s $(SUPPORTED_PLATFORM)/tasks.py ipaplatform/tasks.py; \
+		ln -s $(SUPPORTED_PLATFORM)/constants.py ipaplatform/constants.py; \
 	fi
 
 	if [ "$(SKIP_API_VERSION_CHECK)" != "yes" ]; then \
diff --git a/freeipa.spec.in b/freeipa.spec.in
index fef20e1f7e6fde9b90851a2686e515a6a779f954..928425fdc65a092f67a28d97101c32b7392bf1c8 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -381,6 +381,7 @@ rm -f ipapython/version.py
 rm -f ipaplatform/services.py
 rm -f ipaplatform/tasks.py
 rm -f ipaplatform/paths.py
+rm -f ipaplatform/constants.py
 make version-update
 cd ipa-client; ../autogen.sh --prefix=%{_usr} --sysconfdir=%{_sysconfdir} --localstatedir=%{_localstatedir} --libdir=%{_libdir} --mandir=%{_mandir}; cd ..
 %if ! %{ONLY_CLIENT}
@@ -403,6 +404,7 @@ rm -f ipapython/version.py
 rm -f ipaplatform/services.py
 rm -f ipaplatform/tasks.py
 rm -f ipaplatform/paths.py
+rm -f ipaplatform/constants.py
 make version-update
 %if ! %{ONLY_CLIENT}
 make install DESTDIR=%{buildroot}
diff --git a/ipaplatform/base/constants.py b/ipaplatform/base/constants.py
new file mode 100644
index 0000000000000000000000000000000000000000..70485055fa5a12fac878ace3dea11ea442ebe6be
--- /dev/null
+++ b/ipaplatform/base/constants.py
@@ -0,0 +1,11 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+'''
+This base platform module exports platform dependant constants.
+'''
+
+
+class BaseConstantsNamespace(object):
+    pass
diff --git a/ipaplatform/fedora/constants.py b/ipaplatform/fedora/constants.py
new file mode 100644
index 0000000000000000000000000000000000000000..ce03f58cf95be1a72a9ce3da65e6d21ef193cefe
--- /dev/null
+++ b/ipaplatform/fedora/constants.py
@@ -0,0 +1,16 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+'''
+This Fedora base platform module exports platform related constants.
+'''
+
+# Fallback to default constant definitions
+from ipaplatform.redhat.constants import RedHatConstantsNamespace
+
+
+class FedoraConstantsNamespace(RedHatConstantsNamespace):
+    pass
+
+constants = FedoraConstantsNamespace()
diff --git a/ipaplatform/redhat/constants.py b/ipaplatform/redhat/constants.py
new file mode 100644
index 0000000000000000000000000000000000000000..7209947f8afbd688b02c8b134d33185e497befe0
--- /dev/null
+++ b/ipaplatform/redhat/constants.py
@@ -0,0 +1,17 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+'''
+This Red Hat OS family base platform module exports default platform
+related constants for the Red Hat OS family-based systems.
+'''
+
+# Fallback to default path definitions
+from ipaplatform.base.constants import BaseConstantsNamespace
+
+
+class RedHatConstantsNamespace(BaseConstantsNamespace):
+    pass
+
+constants = RedHatConstantsNamespace()
diff --git a/ipaplatform/rhel/constants.py b/ipaplatform/rhel/constants.py
new file mode 100644
index 0000000000000000000000000000000000000000..eaca48030fa28804c70c161b07228646a95fc1a3
--- /dev/null
+++ b/ipaplatform/rhel/constants.py
@@ -0,0 +1,16 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+'''
+This RHEL base platform module exports platform related constants.
+'''
+
+# Fallback to default constant definitions
+from ipaplatform.redhat.constants import RedHatConstantsNamespace
+
+
+class RHELConstantsNamespace(RedHatConstantsNamespace):
+    pass
+
+constants = RHELConstantsNamespace()
-- 
2.4.3

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

Reply via email to