On 12/03/15 16:10, David Kupka wrote:
On 03/06/2015 06:00 PM, Martin Basti wrote:
Upgrade plugins which modify LDAP data directly should not be executed
in --test mode.
This patch is a workaround, to ensure update with --test option will not
modify any LDAP data.
https://fedorahosted.org/freeipa/ticket/3448
Patch attached.
Ideally we want to fix all plugins to dry-run the upgrade not just
skip when there is '--test' option but it is a good first step.
Works for me, ACK.
We had long discussion, and we decided to remove this option from upgrade.
Reasons:
* users are not supposed to use this option to test if upgrade will be
successful, it can not guarantee it.
* option is not used for developing, as it can not catch all issues with
upgrade, using snapshots is better
Attached patch removes the option.
--
Martin Basti
From 0ec9ebfbc55aa2e749d8f9155f41d5913e0832dd Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 17 Mar 2015 12:23:06 +0100
Subject: [PATCH] Server Upgrade: remove --test option
As --test option is not used for developing, and it is not recommended
to test if upgrade will pass, this path removes it copmletely.
https://fedorahosted.org/freeipa/ticket/3448
---
install/tools/man/ipa-ldap-updater.1 | 5 ---
ipaserver/install/ipa_ldap_updater.py | 21 ++++++------
ipaserver/install/ldapupdate.py | 53 +++++++++++++------------------
ipaserver/install/plugins/updateclient.py | 21 ++++++------
ipaserver/install/schemaupdate.py | 10 ++----
ipaserver/install/upgradeinstance.py | 8 ++---
ipatests/test_install/test_updates.py | 2 +-
7 files changed, 47 insertions(+), 73 deletions(-)
diff --git a/install/tools/man/ipa-ldap-updater.1 b/install/tools/man/ipa-ldap-updater.1
index 79cc316501512879fa39ba4c15fd898b976eb25e..5ab77e047d523975caeb13943d9a14069f1c3a6d 100644
--- a/install/tools/man/ipa-ldap-updater.1
+++ b/install/tools/man/ipa-ldap-updater.1
@@ -81,9 +81,6 @@ Schema files should be in LDIF format, and may only specify attributeTypes and o
\fB\-d\fR, \fB\-\-debug\fR
Enable debug logging when more verbose output is needed
.TP
-\fB\-t\fR, \fB\-\-test\fR
-Run through the update without changing anything. If changes are available then the command returns 2. If no updates are available it returns 0.
-.TP
\fB\-y\fR
File containing the Directory Manager password
.TP
@@ -108,5 +105,3 @@ Specify a schema file. May be used multiple times. Implies \-\-schema.
0 if the command was successful
1 if an error occurred
-
-2 if run with in test mode (\-t) and updates are available
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index 5df7cdf4275b75cc63045e29b92902698dc011a9..3d6c8043764d7f5ad398bf43606927e3a7b8bb1a 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -46,9 +46,6 @@ class LDAPUpdater(admintool.AdminTool):
def add_options(cls, parser):
super(LDAPUpdater, cls).add_options(parser, debug_option=True)
- parser.add_option("-t", "--test", action="store_true", dest="test",
- default=False,
- help="run through the update without changing anything")
parser.add_option("-y", dest="password",
help="file containing the Directory Manager password")
parser.add_option("-l", '--ldapi', action="store_true", dest="ldapi",
@@ -139,7 +136,7 @@ class LDAPUpdater_Upgrade(LDAPUpdater):
updates = None
realm = krbV.default_context().default_realm
- upgrade = IPAUpgrade(realm, self.files, live_run=not options.test,
+ upgrade = IPAUpgrade(realm, self.files,
schema_files=options.schema_files)
upgrade.create_instance()
upgradefailed = upgrade.upgradefailed
@@ -149,9 +146,10 @@ class LDAPUpdater_Upgrade(LDAPUpdater):
'Bad syntax detected in upgrade file(s).', 1)
elif upgrade.upgradefailed:
raise admintool.ScriptError('IPA upgrade failed.', 1)
- elif upgrade.modified and options.test:
- self.log.info('Update complete, changes to be made, test mode')
- return 2
+ elif upgrade.modified:
+ self.log.info('Update complete')
+ else:
+ self.log.info('Update complete, no data were modified')
class LDAPUpdater_NonUpgrade(LDAPUpdater):
@@ -195,13 +193,11 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
modified = schemaupdate.update_schema(
options.schema_files,
dm_password=self.dirman_password,
- live_run=not options.test,
ldapi=options.ldapi) or modified
ld = LDAPUpdate(
dm_password=self.dirman_password,
sub_dict={},
- live_run=not options.test,
ldapi=options.ldapi,
plugins=options.plugins or self.run_plugins)
@@ -210,6 +206,7 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
modified = ld.update(self.files) or modified
- if modified and options.test:
- self.log.info('Update complete, changes to be made, test mode')
- return 2
+ if modified:
+ self.log.info('Update complete')
+ else:
+ self.log.info('Update complete, no data were modified')
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 3e59a91153e01223a114365568c4a97b7f66efba..199b23ba8ec0c7d040a4be260440476f1a1e65a8 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -112,7 +112,7 @@ def safe_output(attr, values):
class LDAPUpdate:
action_keywords = ["default", "add", "remove", "only", "onlyifexist", "deleteentry", "replace", "addifnew", "addifexist"]
- def __init__(self, dm_password, sub_dict={}, live_run=True,
+ def __init__(self, dm_password, sub_dict={},
online=True, ldapi=False, plugins=False):
'''
:parameters:
@@ -120,8 +120,6 @@ class LDAPUpdate:
Directory Manager password
sub_dict
substitution dictionary
- live_run
- Apply the changes or just test
online
Do an online LDAP update or use an experimental LDIF updater
ldapi
@@ -158,7 +156,6 @@ class LDAPUpdate:
'''
log_mgr.get_logger(self, True)
self.sub_dict = sub_dict
- self.live_run = live_run
self.dm_password = dm_password
self.conn = None
self.modified = False
@@ -391,8 +388,7 @@ class LDAPUpdate:
"""Create a task to update an index for an attribute"""
# Sleep a bit to ensure previous operations are complete
- if self.live_run:
- time.sleep(5)
+ time.sleep(5)
cn_uuid = uuid.uuid1()
# cn_uuid.time is in nanoseconds, but other users of LDAPUpdate expect
@@ -412,8 +408,7 @@ class LDAPUpdate:
self.info("Creating task to index attribute: %s", attribute)
self.debug("Task id: %s", dn)
- if self.live_run:
- self.conn.add_entry(e)
+ self.conn.add_entry(e)
return dn
@@ -423,10 +418,6 @@ class LDAPUpdate:
assert isinstance(dn, DN)
- if not self.live_run:
- # If not doing this live there is nothing to monitor
- return
-
# Pause for a moment to give the task time to be created
time.sleep(1)
@@ -642,19 +633,18 @@ class LDAPUpdate:
updated = False
if not found:
try:
- if self.live_run:
- if len(entry):
- # addifexist may result in an entry with only a
- # dn defined. In that case there is nothing to do.
- # It means the entry doesn't exist, so skip it.
- try:
- self.conn.add_entry(entry)
- except errors.NotFound:
- # parent entry of the added entry does not exist
- # this may not be an error (e.g. entries in NIS container)
- self.info("Parent DN of %s may not exist, cannot create the entry",
- entry.dn)
- return
+ if len(entry):
+ # addifexist may result in an entry with only a
+ # dn defined. In that case there is nothing to do.
+ # It means the entry doesn't exist, so skip it.
+ try:
+ self.conn.add_entry(entry)
+ except errors.NotFound:
+ # parent entry of the added entry does not exist
+ # this may not be an error (e.g. entries in NIS container)
+ self.info("Parent DN of %s may not exist, cannot create the entry",
+ entry.dn)
+ return
added = True
self.modified = True
except Exception, e:
@@ -669,8 +659,8 @@ class LDAPUpdate:
for (type, attr, values) in changes:
safe_changes.append((type, attr, safe_output(attr, values)))
self.debug("%s" % safe_changes)
- self.debug("Live %d, updated %d" % (self.live_run, updated))
- if self.live_run and updated:
+ self.debug("Updated %d" % updated)
+ if updated:
self.conn.update_entry(entry)
self.info("Done")
except errors.EmptyModlist:
@@ -709,8 +699,7 @@ class LDAPUpdate:
dn = updates['dn']
try:
self.info("Deleting entry %s", dn)
- if self.live_run:
- self.conn.delete_entry(dn)
+ self.conn.delete_entry(dn)
self.modified = True
except errors.NotFound, e:
self.info("%s did not exist:%s", dn, e)
@@ -757,7 +746,8 @@ class LDAPUpdate:
self.create_connection()
if self.plugins:
self.info('PRE_UPDATE')
- updates = api.Backend.updateclient.update(PRE_UPDATE, self.dm_password, self.ldapi, self.live_run)
+ updates = api.Backend.updateclient.update(
+ PRE_UPDATE, self.dm_password, self.ldapi)
# flush out PRE_UPDATE plugin updates before we begin
self._run_updates(updates)
@@ -779,7 +769,8 @@ class LDAPUpdate:
if self.plugins:
self.info('POST_UPDATE')
- updates = api.Backend.updateclient.update(POST_UPDATE, self.dm_password, self.ldapi, self.live_run)
+ updates = api.Backend.updateclient.update(
+ POST_UPDATE, self.dm_password, self.ldapi)
self._run_updates(updates)
finally:
self.close_connection()
diff --git a/ipaserver/install/plugins/updateclient.py b/ipaserver/install/plugins/updateclient.py
index 85ee3f8a0d17710f99a4d18c95fa3c4f93cba672..824d32b65d5a9a809a44a2313f4e3f7328c18b0d 100644
--- a/ipaserver/install/plugins/updateclient.py
+++ b/ipaserver/install/plugins/updateclient.py
@@ -111,21 +111,21 @@ class updateclient(backend.Executioner):
ordered.sort(key=lambda p: p.order)
return ordered
- def update(self, updatetype, dm_password, ldapi, live_run):
+ def update(self, updatetype, dm_password, ldapi):
"""
Execute all update plugins of type updatetype.
"""
self.create_context(dm_password)
- kw = dict(live_run=live_run)
+ kw = dict()
result = []
- ld = LDAPUpdate(dm_password=dm_password, sub_dict={}, live_run=live_run, ldapi=ldapi)
+ ld = LDAPUpdate(dm_password=dm_password, sub_dict={}, ldapi=ldapi)
for update in self.order(updatetype):
(restart, apply_now, res) = self.run(update.name, **kw)
if restart:
# connection has to be closed before restart, otherwise
# ld instance will try to reuse old non-valid connection
ld.close_connection()
- self.restart(dm_password, live_run)
+ self.restart(dm_password)
if apply_now:
ld.update_from_dict(res)
@@ -142,16 +142,13 @@ class updateclient(backend.Executioner):
"""
return self.Updater[method](**kw)
- def restart(self, dm_password, live_run):
+ def restart(self, dm_password):
dsrestart = DSRestart()
socket_name = paths.SLAPD_INSTANCE_SOCKET_TEMPLATE % \
api.env.realm.replace('.','-')
- if live_run:
- self.destroy_context()
- dsrestart.create_instance()
- wait_for_open_socket(socket_name)
- self.create_context(dm_password)
- else:
- self.log.warn("Test mode, skipping restart")
+ self.destroy_context()
+ dsrestart.create_instance()
+ wait_for_open_socket(socket_name)
+ self.create_context(dm_password)
api.register(updateclient)
diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index eaa45d5e947803bb339b9462df666f58f06e0a2d..9a882c9b610ccfe7c9b4d88724d40b1ebe52aca6 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -83,7 +83,7 @@ def _get_oid_dependency_order(schema, cls):
return ordered_oid_groups
-def update_schema(schema_files, ldapi=False, dm_password=None, live_run=True):
+def update_schema(schema_files, ldapi=False, dm_password=None,):
"""Update schema to match the given ldif files
Schema elements present in the LDIF files but missing from the DS schema
@@ -99,11 +99,9 @@ def update_schema(schema_files, ldapi=False, dm_password=None, live_run=True):
:param schema_files: List of filenames to update from
:param ldapi: if true, use ldapi to connect
:param dm_password: directory manager password
- :live_run: if false, changes will not be applied
:return:
True if modifications were made
- (or *would be* made, for live_run=false)
"""
SCHEMA_ELEMENT_CLASSES_KEYS = [x[0] for x in SCHEMA_ELEMENT_CLASSES]
@@ -162,11 +160,9 @@ def update_schema(schema_files, ldapi=False, dm_password=None, live_run=True):
if new_elements:
modlist = schema_entry.generate_modlist()
log.debug("Schema modlist:\n%s", pprint.pformat(modlist))
+ conn.update_entry(schema_entry)
- if live_run:
- conn.update_entry(schema_entry)
-
- if not (modified and live_run):
+ if not modified:
log.info('Not updating schema')
return modified
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index 018db87a33c5f29aa65836de3f649141fc163acd..2f9039dd7f6b37e89deb02a2a6856a61ec32f51c 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -39,11 +39,10 @@ class IPAUpgrade(service.Service):
listeners and updating over ldapi. This way we know the server is
quiet.
"""
- def __init__(self, realm_name, files=[], live_run=True, schema_files=[]):
+ def __init__(self, realm_name, files=[], schema_files=[]):
"""
realm_name: kerberos realm name, used to determine DS instance dir
files: list of update files to process. If none use UPDATEDIR
- live_run: boolean that defines if we are in test or live mode.
"""
ext = ''
@@ -55,7 +54,6 @@ class IPAUpgrade(service.Service):
serverid = dsinstance.realm_to_serverid(realm_name)
self.filename = '%s/%s' % (paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid, DSE)
self.savefilename = '%s/%s.ipa.%s' % (paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid, DSE, ext)
- self.live_run = live_run
self.files = files
self.modified = False
self.badsyntax = False
@@ -124,11 +122,11 @@ class IPAUpgrade(service.Service):
def __update_schema(self):
self.modified = schemaupdate.update_schema(
self.schema_files,
- dm_password='', ldapi=True, live_run=self.live_run) or self.modified
+ dm_password='', ldapi=True) or self.modified
def __upgrade(self):
try:
- ld = ldapupdate.LDAPUpdate(dm_password='', ldapi=True, live_run=self.live_run, plugins=True)
+ ld = ldapupdate.LDAPUpdate(dm_password='', ldapi=True, plugins=True)
if len(self.files) == 0:
self.files = ld.get_all_files(ldapupdate.UPDATES_DIR)
self.modified = (ld.update(self.files) or self.modified)
diff --git a/ipatests/test_install/test_updates.py b/ipatests/test_install/test_updates.py
index ce932ae0017c0ce990bccfa55b3dacf1e015c21e..9e92aea6ce0e52afd65e3f7a7f19a3dbae9a21dc 100644
--- a/ipatests/test_install/test_updates.py
+++ b/ipatests/test_install/test_updates.py
@@ -59,7 +59,7 @@ class test_update(unittest.TestCase):
fp.close()
else:
raise nose.SkipTest("No directory manager password")
- self.updater = LDAPUpdate(dm_password=self.dm_password, sub_dict={}, live_run=True)
+ self.updater = LDAPUpdate(dm_password=self.dm_password, sub_dict={})
self.ld = ipaldap.IPAdmin(fqdn)
self.ld.do_simple_bind(bindpw=self.dm_password)
if ipautil.file_exists("0_reset.update"):
--
2.1.0
--
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
