Hello Sandro Bonazzola,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/20204
to review the following change.
Change subject: packaging: setup: revert nfs config on cleanup
......................................................................
packaging: setup: revert nfs config on cleanup
Revert /etc/sysconfig/nfs to its original contents on cleanup.
Added a function editConfigContent in util to generalize edit loops
that read conf files and do some changes to them. Used it to edit
/etc/sysconfig/nfs as well as /etc/httpd/conf.d/ssl.conf and
/var/lib/pgsql/data/postgresql.conf.
Bug-Url: https://bugzilla.redhat.com/1013347
Change-Id: I74dd4c1556bd6479fcf10f85fcbe083f215e0854
Signed-off-by: Sandro Bonazzola <[email protected]>
Signed-off-by: Yedidyah Bar David <[email protected]>
---
M packaging/setup/ovirt_engine_setup/util.py
M packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
M packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
M packaging/setup/plugins/ovirt-engine-setup/system/nfs.py
4 files changed, 203 insertions(+), 79 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/04/20204/1
diff --git a/packaging/setup/ovirt_engine_setup/util.py
b/packaging/setup/ovirt_engine_setup/util.py
index ab3620e..852313c 100644
--- a/packaging/setup/ovirt_engine_setup/util.py
+++ b/packaging/setup/ovirt_engine_setup/util.py
@@ -19,6 +19,7 @@
"""Utils."""
+import re
import pwd
import grp
import gettext
@@ -39,6 +40,156 @@
@util.export
+def editConfigContent(
+ content,
+ params,
+ changed_lines=None,
+ comment_re='[#]*\s*',
+ new_comment_tpl='{spaces}# {original}',
+ separator_re='\s*=\s*',
+ new_line_tpl='{spaces}{param} = {value}',
+ added_params=None,
+):
+ """Return edited content of a config file.
+
+ Keyword arguments:
+ content - a list of strings, the content prior to calling us
+ params - a dict of params/values that should be in the output
+ If the value for a param is None, param is deleted
+ changed_lines - an output parameter, a list of dictionaries with
+ added and removed lines.
+ comment_re - a regular expression that a comment marker prefixed
+ to param should match. If a commented param line is found,
+ a new line will be added after it.
+ new_comment_tpl - a template for a comment. {original} will be replaced
+ with this template, {spaces} will be replaced with
+ original whitespace prefix.
+ separator_re - a regular expression that the separator between
+ param and value should match
+ new_line_tpl - a template for a new line. {param} will be replaced
+ with param, {value} with value.
+ added_params - an output parameter, a list of params that were added
+ in the end because they were not found in content.
+
+ Params that appear uncommented in the input, are commented, and new
+ values are added after the commented lines. Params that appear only
+ commented in the input, the comments are copied as-is, and new lines
+ are added after the comments. Params that do not appear in the input
+ are added in the end.
+ """
+ params = params.copy()
+
+ pattern = r"""
+ ^
+ (?P<spaces>\s*)
+ (?P<comment>{comment_re})
+ (?P<original>
+ (?P<param>\w+)
+ (?P<separator>{separator_re})
+ (?P<value>.*)
+ )
+ $
+ """.format(
+ comment_re=comment_re,
+ separator_re=separator_re,
+ )
+ re_obj = re.compile(flags=re.VERBOSE, pattern=pattern)
+
+ # Find params which are uncommented in the input.
+ uncommented = set()
+ for line in content:
+ f = re_obj.match(line)
+ if (
+ f is not None and
+ f.group('param') in params and
+ not f.group('comment')
+ ):
+ uncommented.add(f.group('param'))
+
+ if changed_lines is None:
+ changed_lines = []
+ if added_params is None:
+ added_params = []
+ newcontent = []
+ processed = set()
+ for line in content:
+ f = re_obj.match(line)
+ if (
+ f is not None and
+ f.group('param') in params and
+ not (
+ f.group('param') in uncommented and
+ f.group('comment')
+ )
+ # If param in uncommented and current line is comment,
+ # we do not need to process it - we process the uncommented
+ # line when we see it
+ ):
+ if (
+ not f.group('comment') and
+ f.group('value') == params[f.group('param')]
+ ):
+ # value is not changed, do nothing
+ processed.add(f.group('param'))
+ else:
+ if (
+ f.group('param') in uncommented and
+ not f.group('comment')
+ ):
+ # Add current line, commented, before new line
+ currentline = new_comment_tpl.format(
+ spaces=f.group('spaces'),
+ original=f.group('original'),
+ )
+ changed_lines.append(
+ {
+ 'added': currentline,
+ 'removed': line,
+ }
+ )
+ newcontent.append(currentline)
+ else:
+ # Only possible option here is that current line is
+ # a comment and param is not in uncommented. Keep it.
+ # Other two options are in "if"s above.
+ # The last option - param is not in uncommented
+ # and current line is not a comment - is not possible.
+ newcontent.append(line)
+
+ newline = new_line_tpl.format(
+ spaces=f.group('spaces'),
+ param=f.group('param'),
+ value=params[f.group('param')],
+ )
+ changed_lines.append(
+ {
+ 'added': newline,
+ }
+ )
+ processed.add(f.group('param'))
+ line = newline
+
+ newcontent.append(line)
+
+ # Add remaining params at the end
+ for param, value in params.items():
+ if not param in processed:
+ newline = new_line_tpl.format(
+ spaces='',
+ param=param,
+ value=value,
+ )
+ newcontent.append(newline)
+ changed_lines.append(
+ {
+ 'added': newline,
+ }
+ )
+ added_params.append(param)
+ return newcontent
+
+
[email protected]
def getUid(user):
return pwd.getpwnam(user)[2]
diff --git a/packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
b/packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
index 5431be7..c1bf259 100644
--- a/packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
+++ b/packaging/setup/plugins/ovirt-engine-setup/apache/ssl.py
@@ -20,7 +20,6 @@
import os
-import re
import gettext
_ = lambda m: gettext.dgettext(message=m, domain='ovirt-engine-setup')
@@ -33,56 +32,12 @@
from ovirt_engine_setup import constants as osetupcons
from ovirt_engine_setup import dialog
+from ovirt_engine_setup import util as osetuputil
@util.export
class Plugin(plugin.PluginBase):
"""Apache ssl plugin."""
-
- _RE_PARAM = re.compile(
- flags=re.VERBOSE,
- pattern=r"""
- ^
- (?P<spaces>\s*)
- [#]*
- (?P<param>\w+)
- \s*
- .*
- $
- """
- )
-
- def _editParams(self, params, content):
- newcontent = []
- for line in content.splitlines():
- f = self._RE_PARAM.match(line)
- if f is not None and f.group('param') in params:
- newline = '{spaces}{param} {value}'.format(
- spaces=f.group('spaces'),
- param=f.group('param'),
- value=params[f.group('param')],
- )
- self._changed_lines.append(
- {
- 'added': newline,
- 'removed': line,
- }
- )
- line = newline
- newcontent.append(line)
- return newcontent
-
- def _findMissingParams(self, params, content):
- missingParams = params.keys()
- for line in content.splitlines():
- f = self._RE_PARAM.match(line)
- if (
- f is not None and
- f.group('param') in missingParams
- ):
- missingParams.remove(f.group('param'))
-
- return missingParams
def __init__(self, context):
super(Plugin, self).__init__(context=context)
@@ -98,7 +53,6 @@
osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_CA_CERT
),
}
- self._changed_lines = []
@plugin.event(
stage=plugin.Stages.STAGE_INIT,
@@ -189,9 +143,13 @@
) as f:
self._sslData = f.read()
- missingParams = self._findMissingParams(
- self._params,
- self._sslData
+ missingParams = []
+ osetuputil.editConfigContent(
+ content=self._sslData.splitlines(),
+ params=self._params,
+ separator_re='\s+',
+ new_line_tpl='{spaces}{param} {value}',
+ added_params=missingParams,
)
if missingParams:
self.logger.warning(
@@ -214,14 +172,18 @@
condition=lambda self: self._enabled,
)
def _misc(self):
+ changed_lines = []
self.environment[otopicons.CoreEnv.MAIN_TRANSACTION].append(
filetransaction.FileTransaction(
name=self.environment[
osetupcons.ApacheEnv.HTTPD_CONF_SSL
],
- content=self._editParams(
- self._params,
- self._sslData
+ content=osetuputil.editConfigContent(
+ content=self._sslData.splitlines(),
+ params=self._params,
+ changed_lines=changed_lines,
+ separator_re='\s+',
+ new_line_tpl='{spaces}{param} {value}',
),
)
)
@@ -234,7 +196,7 @@
).addChanges(
'ssl',
self.environment[osetupcons.ApacheEnv.HTTPD_CONF_SSL],
- self._changed_lines,
+ changed_lines,
)
self.environment[
osetupcons.CoreEnv.UNINSTALL_UNREMOVABLE_FILES
diff --git
a/packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
b/packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
index c4f5219..e6113c3 100644
--- a/packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
+++ b/packaging/setup/plugins/ovirt-engine-setup/provisioning/postgres.py
@@ -59,18 +59,6 @@
$
"""
)
- _RE_POSTGRES_MAX_CONN = re.compile(
- flags=re.VERBOSE,
- pattern=r"""
- ^
- \s*
- max_connections
- \s*
- =
- .*
- $
- """
- )
class _alternateUser(object):
def __init__(self, user):
@@ -124,14 +112,13 @@
filename,
maxconn,
):
- content = []
with open(filename, 'r') as f:
- for line in f.read().splitlines():
- if self._RE_POSTGRES_MAX_CONN.match(line) is not None:
- line = 'max_connections = {maxconn}'.format(
- maxconn=maxconn,
- )
- content.append(line)
+ content = osetuputil.editConfigContent(
+ content=f.read().splitlines(),
+ params={
+ 'max_connections': maxconn,
+ },
+ )
transaction.append(
filetransaction.FileTransaction(
diff --git a/packaging/setup/plugins/ovirt-engine-setup/system/nfs.py
b/packaging/setup/plugins/ovirt-engine-setup/system/nfs.py
index a24fb0b..62eb77b 100644
--- a/packaging/setup/plugins/ovirt-engine-setup/system/nfs.py
+++ b/packaging/setup/plugins/ovirt-engine-setup/system/nfs.py
@@ -31,6 +31,10 @@
from otopi import constants as otopicons
from otopi import filetransaction
+
+from ovirt_engine import configfile
+
+
from ovirt_engine_setup import constants as osetupcons
from ovirt_engine_setup import util as osetuputil
from ovirt_engine_setup import dialog
@@ -41,6 +45,7 @@
"""
NFS and RPCbind services configuration plugin.
"""
+
def __init__(self, context):
super(Plugin, self).__init__(context=context)
self._distribution = platform.linux_distribution(
@@ -158,17 +163,36 @@
condition=lambda self: self._enabled,
)
def _misc(self):
+ config = configfile.ConfigFile([
+ osetupcons.FileLocations.OVIRT_NFS_RHEL_CONFIG
+ ])
+ changed_lines = []
+ with open(osetupcons.FileLocations.NFS_RHEL_CONFIG, 'r') as f:
+ content = f.read().splitlines()
self.environment[otopicons.CoreEnv.MAIN_TRANSACTION].append(
filetransaction.FileTransaction(
name=osetupcons.FileLocations.NFS_RHEL_CONFIG,
- content=osetuputil.processTemplate(
- osetupcons.FileLocations.OVIRT_NFS_RHEL_CONFIG,
- ),
- modifiedList=self.environment[
- otopicons.CoreEnv.MODIFIED_FILES
- ],
+ content=osetuputil.editConfigContent(
+ content=content,
+ params=config.values,
+ changed_lines=changed_lines,
+ )
)
)
+ self.environment[
+ osetupcons.CoreEnv.REGISTER_UNINSTALL_GROUPS
+ ].createGroup(
+ group='nfs_config',
+ description='NFS Configuration',
+ optional=True
+ ).addChanges(
+ 'nfs_config',
+ osetupcons.FileLocations.NFS_RHEL_CONFIG,
+ changed_lines,
+ )
+ self.environment[
+ osetupcons.CoreEnv.UNINSTALL_UNREMOVABLE_FILES
+ ].append(osetupcons.FileLocations.NFS_RHEL_CONFIG)
@plugin.event(
stage=plugin.Stages.STAGE_CLOSEUP,
--
To view, visit http://gerrit.ovirt.org/20204
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74dd4c1556bd6479fcf10f85fcbe083f215e0854
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
