[node-patches] Change in ovirt-node[master]: tui: Fix Kdump, logging, and iscsi pages
fabiand at fedoraproject.org
fabiand at fedoraproject.org
Fri Dec 14 13:34:18 UTC 2012
Fabian Deutsch has uploaded a new change for review.
Change subject: tui: Fix Kdump, logging, and iscsi pages
......................................................................
tui: Fix Kdump, logging, and iscsi pages
kdump: Don't throw an exception on a successfull configuration
logging: Use correct validators
iscsi: Use correct validators
snmp: Disable page for now
Change-Id: Ib041b98e88462113602dc3a9a451c038e015bd05
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M scripts/tui/src/ovirt/node/config/defaults.py
M scripts/tui/src/ovirt/node/plugins.py
M scripts/tui/src/ovirt/node/setup/kdump_page.py
M scripts/tui/src/ovirt/node/setup/logging_page.py
M scripts/tui/src/ovirt/node/setup/remote_storage_page.py
M scripts/tui/src/ovirt/node/setup/snmp_page.py
M scripts/tui/src/ovirt/node/ui/__init__.py
M scripts/tui/src/ovirt/node/ui/builder.py
M scripts/tui/src/ovirt/node/utils/__init__.py
M scripts/tui/src/ovirt/node/utils/fs.py
M scripts/tui/src/ovirt/node/valid.py
11 files changed, 100 insertions(+), 139 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/62/10062/1
diff --git a/scripts/tui/src/ovirt/node/config/defaults.py b/scripts/tui/src/ovirt/node/config/defaults.py
index 9544fdf..06953f8 100644
--- a/scripts/tui/src/ovirt/node/config/defaults.py
+++ b/scripts/tui/src/ovirt/node/config/defaults.py
@@ -19,6 +19,7 @@
# MA 02110-1301, USA. A copy of the GNU General Public License is
# also available at http://www.gnu.org/copyleft/gpl.html.
from ovirt.node import base, exceptions, valid, utils, config
+from ovirt.node.utils import fs
import glob
import logging
import os
@@ -116,13 +117,24 @@
return cfg
def _write(self, cfg):
- # FIXME make atomic
- contents = []
+ lines = []
# Sort the dict, looks nicer
for key in sorted(cfg.iterkeys()):
- contents.append("%s=%s" % (key, cfg[key]))
- with open(self.filename, "w+") as dst:
- dst.write("\n".join(contents))
+ lines.append("%s=%s" % (key, cfg[key]))
+ contents = "\n".join(lines) + "\n"
+
+ # The following logic is mainly needed to allow an "offline" testing
+ config_fs = fs.Config()
+ if config_fs.is_enabled():
+ with config_fs.open_file(self.filename, "w") as dst:
+ dst.write(contents)
+ else:
+ try:
+ fs.atomic_write(self.filename, contents)
+ except Exception as e:
+ self.logger.warning("Atomic write failed: %s" % e)
+ with open(self.filename, "w") as dst:
+ dst.write(contents)
class ConfigFile(base.Base):
@@ -594,7 +606,7 @@
valid.Port()(port)
def transaction(self):
- self.__legacy_transaction()
+ return self.__legacy_transaction()
def __legacy_transaction(self):
cfg = dict(self.retrieve())
@@ -754,9 +766,9 @@
self.backups.restore("/etc/kdump.conf")
system("service kdump restart")
- raise RuntimeError("KDump configuration failed, " +
- "location unreachable. Previous " +
- "configuration was restored.")
+# raise RuntimeError("KDump configuration failed, " +
+# "location unreachable. Previous " +
+# "configuration was restored.")
ovirt_store_config("/etc/kdump.conf")
self.backups.remove()
@@ -890,7 +902,7 @@
def commit(self):
import ovirtnode.log as olog
- olog.ovirt_netconsole(server, port, "udp")
+ olog.ovirt_netconsole(server, port)
tx = utils.Transaction("Configuring netconsole")
tx.append(CreateNetconsoleConfig())
diff --git a/scripts/tui/src/ovirt/node/plugins.py b/scripts/tui/src/ovirt/node/plugins.py
index 6144c8b..0b98091 100644
--- a/scripts/tui/src/ovirt/node/plugins.py
+++ b/scripts/tui/src/ovirt/node/plugins.py
@@ -429,4 +429,4 @@
def elements(self):
"""Return the UI elements of this group
"""
- return self.widgethelper.subset(self)
\ No newline at end of file
+ return self.widgethelper.subset(self)
diff --git a/scripts/tui/src/ovirt/node/setup/kdump_page.py b/scripts/tui/src/ovirt/node/setup/kdump_page.py
index ba4296f..be6cff7 100644
--- a/scripts/tui/src/ovirt/node/setup/kdump_page.py
+++ b/scripts/tui/src/ovirt/node/setup/kdump_page.py
@@ -108,6 +108,7 @@
w = "kdump.%s_location" % changes["kdump.type"]
if w in net_types:
self._widgets[w].enabled(True),
+ self.validate({w: ""})
def on_merge(self, effective_changes):
"""Applies the changes to the plugins model, will do all required logic
diff --git a/scripts/tui/src/ovirt/node/setup/logging_page.py b/scripts/tui/src/ovirt/node/setup/logging_page.py
index 7147e19..74e8f8f 100644
--- a/scripts/tui/src/ovirt/node/setup/logging_page.py
+++ b/scripts/tui/src/ovirt/node/setup/logging_page.py
@@ -42,13 +42,7 @@
netconsole = defaults.Netconsole().retrieve()
syslog = defaults.Syslog().retrieve()
- model = {
- "logrotate.max_size": "1024",
- "rsyslog.address": "",
- "rsyslog.port": "514",
- "netconsole.address": "",
- "netconsole.port": "6666",
- }
+ model = {}
model["logrotate.max_size"] = logrotate["max_size"] or "1024"
model["rsyslog.address"] = syslog["server"] or ""
@@ -64,9 +58,10 @@
"""
return {
"logrotate.max_size": valid.Number(range=[0, None]),
- "rsyslog.address": valid.FQDNOrIPAddress(),
+ "rsyslog.address": (valid.Empty() | valid.FQDNOrIPAddress()),
"rsyslog.port": valid.Port(),
- "netconsole.address": valid.FQDNOrIPAddress(),
+ "netconsole.address": (valid.Empty() |
+ valid.FQDNOrIPAddress()),
"netconsole.port": valid.Port(),
}
@@ -109,13 +104,10 @@
self.logger.debug("Saving logging page: %s" % changes.changes)
self.logger.debug("Logging page model: %s" % effective_model.changes)
- logrotate_keys = ["logrotate.max_size"]
- rsyslog_keys = ["rsyslog.address", "rsyslog.port"]
- netconsole_keys = ["netconsole.address", "netconsole.port"]
-
txs = utils.Transaction("Updating logging related configuration")
# If any logrotate key changed ...
+ logrotate_keys = ["logrotate.max_size"]
if changes.any_key_in_change(logrotate_keys):
# Get all logrotate values fomr the effective model
model = defaults.Logrotate()
@@ -123,11 +115,13 @@
model.update(*effective_model.get_key_values(logrotate_keys))
txs += model.transaction()
+ rsyslog_keys = ["rsyslog.address", "rsyslog.port"]
if changes.any_key_in_change(rsyslog_keys):
model = defaults.Syslog()
model.update(*effective_model.get_key_values(rsyslog_keys))
txs += model.transaction()
+ netconsole_keys = ["netconsole.address", "netconsole.port"]
if changes.any_key_in_change(netconsole_keys):
model = defaults.Netconsole()
model.update(*effective_model.get_key_values(netconsole_keys))
diff --git a/scripts/tui/src/ovirt/node/setup/remote_storage_page.py b/scripts/tui/src/ovirt/node/setup/remote_storage_page.py
index 57c4823..bdbe402 100644
--- a/scripts/tui/src/ovirt/node/setup/remote_storage_page.py
+++ b/scripts/tui/src/ovirt/node/setup/remote_storage_page.py
@@ -47,8 +47,8 @@
def validators(self):
return {
- "iscsi.initiator_name": valid.IQN(),
- "nfsv4.domain": valid.FQDN(),
+ "iscsi.initiator_name": (valid.Empty() | valid.IQN()),
+ "nfsv4.domain": (valid.Empty() | valid.FQDN()),
}
def ui_content(self):
diff --git a/scripts/tui/src/ovirt/node/setup/snmp_page.py b/scripts/tui/src/ovirt/node/setup/snmp_page.py
index 0e8cb5d..3e65e1d 100644
--- a/scripts/tui/src/ovirt/node/setup/snmp_page.py
+++ b/scripts/tui/src/ovirt/node/setup/snmp_page.py
@@ -31,6 +31,10 @@
_model = None
_widgets = None
+ def has_ui(self):
+ # FIXME is SNMP in a plugin?
+ return False
+
def name(self):
return "SNMP"
diff --git a/scripts/tui/src/ovirt/node/ui/__init__.py b/scripts/tui/src/ovirt/node/ui/__init__.py
index 0e1c883..5c79fbf 100644
--- a/scripts/tui/src/ovirt/node/ui/__init__.py
+++ b/scripts/tui/src/ovirt/node/ui/__init__.py
@@ -384,6 +384,13 @@
def run(self):
self.plugin.application.ui.show_dialog(self)
self._close_button.enabled(False)
+ if self.transaction:
+ self.__run_transaction()
+ else:
+ self.add_update("There were no changes, nothing to do.")
+ self._close_button.enabled(True)
+
+ def __run_transaction(self):
try:
self.transaction.prepare() # Just to display something in dry mode
for idx, e in enumerate(self.transaction):
@@ -397,4 +404,3 @@
self.logger.warning("'%s' on transaction '%s': %s - %s" %
(type(e), self.transaction, e, e.message))
self.logger.debug(str(traceback.format_exc()))
- self._close_button.enabled(True)
diff --git a/scripts/tui/src/ovirt/node/ui/builder.py b/scripts/tui/src/ovirt/node/ui/builder.py
index b0f51d4..8a36e2b 100644
--- a/scripts/tui/src/ovirt/node/ui/builder.py
+++ b/scripts/tui/src/ovirt/node/ui/builder.py
@@ -165,7 +165,7 @@
LOGGER.error("Invalid data when updating: %s" % e)
if widget._selectable:
widget.notice = e.message
- widget.valid(False)
+ widget.valid(False)
tui._draw_screen()
urwid.connect_signal(widget, 'change', on_widget_value_change)
diff --git a/scripts/tui/src/ovirt/node/utils/__init__.py b/scripts/tui/src/ovirt/node/utils/__init__.py
index 897298c..90b10c2 100644
--- a/scripts/tui/src/ovirt/node/utils/__init__.py
+++ b/scripts/tui/src/ovirt/node/utils/__init__.py
@@ -124,23 +124,6 @@
return m.hexdigest()
-def is_bind_mount(filename, fsprefix="ext"):
- """Checks if a given file is bind mounted
-
- Args:
- filename: File to be checked
- Returns:
- True if the file is a bind mount target
- """
- bind_mount_found = False
- with open("/proc/mounts") as mounts:
- pattern = "%s %s" % (filename, fsprefix)
- for mount in mounts:
- if pattern in mount:
- bind_mount_found = True
- return bind_mount_found
-
-
def parse_bool(txt):
"""Parse common "bool" values (yes, no, true, false, 1)
diff --git a/scripts/tui/src/ovirt/node/utils/fs.py b/scripts/tui/src/ovirt/node/utils/fs.py
index 03303d1..96aa1af 100644
--- a/scripts/tui/src/ovirt/node/utils/fs.py
+++ b/scripts/tui/src/ovirt/node/utils/fs.py
@@ -27,9 +27,7 @@
import shutil
import os
-from ovirt.node.utils import checksum, is_bind_mount
from ovirt.node import base
-from process import system
LOGGER = logging.getLogger(__name__)
@@ -52,6 +50,23 @@
"Source and destination need to exist"
with open(src, "r") as srcf, open(dst, "wb") as dstf:
dstf.write(srcf.read())
+
+
+def atomic_write(self, filename, contents):
+ backup = BackupedFiles([filename], ".temp")
+ backup.create()
+ backup_filename = backup.of(filename)
+
+ with open(backup_filename, "wb") as dst:
+ dst.write(contents)
+
+ fns = (backup_filename, filename)
+ self.logger.debug("Moving '%s' to '%s' atomically" % fns)
+ try:
+ os.rename(*fns)
+ except Exception as e:
+ backup.remove()
+ raise e
class BackupedFiles(base.Base):
@@ -110,7 +125,9 @@
"""
for fn in self.files:
backup = "%s%s" % (fn, self.suffix)
- assert not os.path.exists(backup)
+ if os.path.exists(backup):
+ raise RuntimeError(("Backup '%s' for '%s " +
+ "already exists") % (backup, fn))
if os.path.exists(fn):
shutil.copy(fn, backup)
self.backups[fn] = backup
@@ -123,6 +140,7 @@
"""
for fn in self.files:
backup = self.backups[fn]
+ self.logger.debug("Removing backup of '%s': %s" % (fn, backup))
os.remove(backup)
def of(self, fn):
@@ -139,99 +157,42 @@
copy_contents(self.of(fn), fn)
-def persist_config(filename):
- LOGGER.info("Persisting: %s" % filename)
- filenames = []
-
-# if is_stateless():
-# return True
- if not os.path.ismount(persist_path()):
- LOGGER.warning("/config is not mounted")
- return False
- if type(filename) in [str, unicode]:
- filenames.append(filename)
- elif type(filename) is list:
- filenames = filename
- else:
- LOGGER.error("Unknown type: %s" % filename)
- return False
-
- persist_failed = False
- for f in filenames:
- filename = os.path.abspath(f)
-
- if os.path.isdir(filename):
- # ensure that, if this is a directory
- # that it's not already persisted
- if os.path.isdir(persist_path(filename)):
- LOGGER.warn("Directory already persisted: %s" % filename)
- LOGGER.warn("You need to unpersist its child directories " +
- "and/or files and try again.")
- continue
-
- elif os.path.isfile(filename):
- # if it's a file then make sure it's not already persisted
- persist_filename = persist_path(filename)
- if os.path.isfile(persist_filename):
- if checksum(filename) == checksum(persist_filename):
- # FIXME yes, there could be collisions ...
- LOGGER.info("Persisted file is equal: %s" % filename)
- continue
- else:
- # Remove persistent copy - needs refresh
- if system("umount -n %s 2> /dev/null" % filename):
- system("rm -f %s" % persist_filename)
-
- else:
- # skip if file does not exist
- LOGGER.warn("Skipping, file '%s' does not exist" % filename)
- continue
-
- # At this poitn we know that we want to persist the file.
-
- # skip if already bind-mounted
- if is_bind_mount(filename):
- LOGGER.warn("%s is already persisted" % filename)
- else:
- dirname = os.path.dirname(filename)
- system("mkdir -p %s" % persist_path(dirname))
- persist_filename = persist_path(filename)
- if system("cp -a %s %s" % (filename, persist_filename)):
- if not system("mount -n --bind %s %s" % (persist_filename,
- filename)):
- LOGGER.error("Failed to persist: " + filename)
- persist_failed = True
- else:
- LOGGER.info("Persisted: $s" % filename)
-
- with open(persist_path("files"), "r") as files:
- if filename not in files.read().split("\n"):
- # register in /config/files used by rc.sysinit
- system("echo " + filename + " >> /config/files")
- LOGGER.info("Successfully persisted (reg): %s" % filename)
- return not persist_failed
-
-
-def persist_path(filename=""):
- """Returns the path a file will be persisted in
-
- Returns:
- Path to the persisted variant of the file.
- """
- return os.path.join("/config", os.path.abspath(filename))
-
-
-def is_persisted(filename):
- """Check if the file is persisted
+def is_bind_mount(filename, fsprefix="ext"):
+ """Checks if a given file is bind mounted
Args:
- filename: Filename to be checked
+ filename: File to be checked
Returns:
- True if the file exists in the /config hierarchy
+ True if the file is a bind mount target
"""
- return os.path.exists(persist_path(filename))
+ bind_mount_found = False
+ with open("/proc/mounts") as mounts:
+ pattern = "%s %s" % (filename, fsprefix)
+ for mount in mounts:
+ if pattern in mount:
+ bind_mount_found = True
+ return bind_mount_found
-def unpersist_config(filename):
- LOGGER.info("Unpersisting: %s" % filename)
- # FIXME
+class Config(base.Base):
+ basedir = "/config"
+
+ def _config_path(self, fn=""):
+ return os.path.join(self.basedir, fn.strip("/"))
+
+ def persist(self, filename):
+ from ovirtnode import ovirtfunctions
+ return ovirtfunctions.ovirt_store_config(filename)
+
+ def unpersist(self, filename):
+ from ovirtnode import ovirtfunctions
+ return ovirtfunctions.remove_config(filename)
+
+ def exists(self, filename):
+ return os.path.exists(self._config_path(filename))
+
+ def is_enabled(self):
+ return is_bind_mount(self.basedir)
+
+ def open_file(self, filename, mode="r"):
+ return open(self._config_path(filename), mode)
diff --git a/scripts/tui/src/ovirt/node/valid.py b/scripts/tui/src/ovirt/node/valid.py
index 52f01a6..924d833 100644
--- a/scripts/tui/src/ovirt/node/valid.py
+++ b/scripts/tui/src/ovirt/node/valid.py
@@ -188,7 +188,7 @@
"""
description = "a string without spaces"
- pattern = "^\S*$"
+ pattern = "^\S+$"
class FQDN(RegexValidator):
--
To view, visit http://gerrit.ovirt.org/10062
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib041b98e88462113602dc3a9a451c038e015bd05
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Fabian Deutsch <fabiand at fedoraproject.org>
More information about the node-patches
mailing list