[node-patches] Change in ovirt-node[master]: Initial pass at cleaning up rhn_page
rbarry at redhat.com
rbarry at redhat.com
Sun Apr 26 17:48:19 UTC 2015
Ryan Barry has uploaded a new change for review.
Change subject: Initial pass at cleaning up rhn_page
......................................................................
Initial pass at cleaning up rhn_page
Move all the standalone functions into the class. They don't
get called from anything else. Model is next. If they're called
there, the code will get moved. Potentially dead code anyway. Clean
them up just in case it's not.
Remove unecessary imports. Get rid of some constants. Slight logic
cleanup instead of concatenating strings. Doesn't need to be that
dynamic. Sacrifice dynamic to gain readability, we can always add
it back later if we need it.
Hopefully a second pass later where a lot of code can be removed,
and model/page can work together with proper logic separation
Change-Id: Ide8dec4c91464a3c4a54a19890b3806b318f308a
Signed-off-by: Ryan Barry <rbarry at redhat.com>
---
M src/ovirt/node/setup/rhn/rhn_page.py
1 file changed, 102 insertions(+), 91 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/42/40242/1
diff --git a/src/ovirt/node/setup/rhn/rhn_page.py b/src/ovirt/node/setup/rhn/rhn_page.py
old mode 100644
new mode 100755
index 13d13a2..80a9bff
--- a/src/ovirt/node/setup/rhn/rhn_page.py
+++ b/src/ovirt/node/setup/rhn/rhn_page.py
@@ -19,88 +19,16 @@
# 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 plugins, valid, ui, utils
-import rhn_model
from ovirt.node.plugins import Changeset
from ovirt.node.utils import process, system
from ovirt.node.utils.network import NodeNetwork
+import rhn_model
import os.path
-import logging
+
+# Needed to import rhnreg later, but since that may be able to be
+# scrapped, we can hopefully get rid of this as well
import sys
sys.path.append("/usr/share/rhn/up2date_client")
-
-
-RHN_CONFIG_FILE = "/etc/sysconfig/rhn/up2date"
-RHSM_CONFIG_FILE = "/etc/rhsm/rhsm.conf"
-RHN_XMLRPC_ADDR = "https://xmlrpc.rhn.redhat.com/XMLRPC"
-SAM_REG_ADDR = "subscription.rhn.redhat.com"
-CANDLEPIN_CERT_FILE = "/etc/rhsm/ca/candlepin-local.pem"
-
-logger = logging.getLogger(__name__)
-
-
-def get_rhn_config():
- conf_files = []
- if os.path.exists(RHN_CONFIG_FILE):
- conf_files.append(RHN_CONFIG_FILE)
- if os.path.exists(RHSM_CONFIG_FILE):
- conf_files.append(RHSM_CONFIG_FILE)
- rhn_conf = {}
- for f in conf_files:
- rhn_config = open(f)
- for line in rhn_config:
- if "=" in line and "[comment]" not in line:
- item, value = line.replace(" ", "").split("=")
- rhn_conf[item] = value.strip()
- rhn_config.close()
- return rhn_conf
-
-
-def rhn_check():
- filebased = True
- registered = False
- if filebased:
- # The following file exists when the sys is registered with rhn
- registered = os.path.exists("/etc/sysconfig/rhn/systemid")
- else:
- if process.check_call("rhn_check"):
- registered = True
- return registered
-
-
-def sam_check():
- if system.SystemRelease().is_redhat():
- import rhnreg
- if rhnreg.rhsm_registered():
- return True
- return False
-
-
-def get_rhn_status():
- msg = ""
- status = 0
- rhn_conf = get_rhn_config()
- # local copy, blank proxy password
- # rhbz#837249
- for arg in "proxyPassword", "proxy_password":
- if arg in rhn_conf and rhn_conf[arg] != "":
- rhn_conf[arg] = "XXXXXXXX"
- if rhn_check(): # Is Satellite or Hosted
- status = 1
- try:
- if "serverURL" in rhn_conf:
- if RHN_XMLRPC_ADDR in rhn_conf["serverURL"]:
- msg = "RHN"
- else:
- msg = "Satellite"
- except:
- # corrupt up2date config in this case
- status = 0
- pass
- elif sam_check():
- status = 1
- msg = "SAM"
- return (status, msg)
-
class Plugin(plugins.NodePlugin):
_model = None
@@ -125,6 +53,14 @@
def model(self):
cfg = rhn_model.RHN().retrieve()
self.logger.debug(cfg)
+ # FIXME: the model should just return the right keys
+ # Lots of room for error and unnecessary logic if we're
+ # doing this over and over. Why convert all the values
+ # in the model only to convert them back in on_merge?
+ # Since we can't go back and use the right keys in past versions,
+ # maybe the update and retrieve methods in the model should handle
+ # this so we can just use the same (sane) names all over instead of
+ # re-interpreting all the values repeatedly
model = {"rhn.username": "",
"rhn.password": "",
"rhn.profilename": "",
@@ -139,7 +75,7 @@
"rhn.proxypassword": "",
}
- status, rhn_type = get_rhn_status()
+ rhn_type = self._get_rhn_status()
model["rhn.username"] = cfg["username"]
model["rhn.type"] = cfg["rhntype"]
model["rhn.profilename"] = cfg["profile"]
@@ -183,8 +119,7 @@
ui.Divider("notice.divider")])
else:
- status, rhn_type = get_rhn_status()
- if status == 0:
+ if not cfg["rhntype"]:
rhn_msg = ("RHN Registration is required only if you wish " +
"to use Red Hat Enterprise Linux with virtual " +
"guests subscriptions for your guests.")
@@ -239,6 +174,10 @@
return
def on_merge(self, effective_changes):
+ # FIXME: go back and clean this up once the model is sorted, so
+ # we don't need to have 10-15 lines of unnecessary code passing
+ # pulling values out of the effective model and passing them into
+ # model.update() in a specific order
self.logger.debug("Saving RHN page")
changes = Changeset(self.pending_changes(False))
effective_model = Changeset(self.model())
@@ -278,19 +217,15 @@
proxyuser = effective_model["rhn.proxyuser"]
proxypassword = effective_model["rhn.proxypassword"]
- warning_text = ""
+ warning_text = None
if rhn_type == "sam" or rhn_type == "satellite":
- if url == "" or url is None:
- warning_text += "URL "
+ if not url and not ca:
+ warning_text = "URL and CA path "
+ elif not ca:
+ warning text = "CA path "
- if ca == "" or ca is None:
- if warning_text is "":
- warning_text += "CA path "
- else:
- warning_text += "and CA path "
-
- if warning_text is not "":
+ if warning_text:
txt = "%s must not be empty!" % warning_text
self._error_dialog = ui.InfoDialog("dialog.error",
"RHN Error",
@@ -300,8 +235,6 @@
model = rhn_model.RHN()
model.clear()
# join proxy host/port
- self.logger.debug(proxyhost)
- self.logger.debug(proxyport)
proxy = None
if len(proxyhost) > 0 and len(proxyport) > 0:
proxy = "%s:%s" % (proxyhost, proxyport)
@@ -315,6 +248,84 @@
progress_dialog.run()
return self.ui_content()
+ def _get_rhn_config(self):
+ # Could be RHN classic/satellite or subscription, so check both
+ conf_files = ["/etc/sysconfig/rhn/up2date",
+ "/etc/rhsm/rhsm.conf"]
+
+ masked_fields = ["proxyPassword", "proxy_password"]
+
+ conf = {}
+ for c in conf_files:
+ if os.path.exists(c):
+ with open(c) as f:
+ for line in f:
+ if "=" in line and "[comment]" not in line:
+ k, v = line.replace(" ", "").split("=")
+ conf[k] = v.strip() if k not in masked_fields \
+ else "XXXXXXXX"
+
+ return conf
+
+ def _get_rhn_status(self):
+ # FIXME: what is the point of this? We don't ever appear to use these
+ # values anywhere, and we can just check model["rhn-type"] directly,
+ # which already happens anyway, since it only checks whether or not.
+ # This returns a non-empty value, then gets the type from the model.
+ # Review when finished. We may be able to drop all of these functions
+ #
+ # In theory, they provide the ability to display the registration
+ # status by checking activity that could have happened outside the TUI.
+ # In practice, that functionality is never used and it's unlikely that
+ # we even support that use case (or that it would be a bug if
+ # registrations from the rescue console did not appear in the TUI)
+ conf = self._get_rhn_config()
+ if self._rhn_check():
+ try:
+ if "serverURL" in conf:
+ # Check for the public RHN URL
+ if "https://xmlrpc.rhn.redhat.com/XMLRPC" in \
+ conf["serverURL"]:
+ return "RHN"
+ else:
+ return "Satellite"
+ except:
+ # corrupt up2date config? When does this happen?
+ return None
+ elif self._sam_check():
+ return "SAM"
+ else:
+ return None
+
+
+ def _sam_check(self):
+ if not system.SystemRelease().is_redhat():
+ return False
+ else:
+ try:
+ import rhnreg
+ if rhnreg.rhsm_registered():
+ return True
+ except:
+ self.logger.warning("Can't import rhnreg to check sam"
+ "registration! Did platform change?")
+
+ def _rhn_check(self):
+ # If /etc/sysconfig/rhn/systemid exists, it's been successfully
+ # registered at least once. But rhn_check already checks this.
+
+ # We incur a .5 second delay by actually calling rhn_check instead
+ # of just checking whether it exists, but the we're not making
+ # assumptions about state which rhn_check may actually check
+ try:
+ with open("/dev/null", "wb") as DEVNULL:
+ if process.check_call(["rhn_check"], stdout=DEVNULL,
+ stderr=DEVNULL):
+ return True
+ except process.CalledProcessError as e:
+ self.logger.debug("rhn_check returned a non-zero exit code %s. "
+ "Not registered?" % e.returncode)
+
class ProxyDialog(ui.Dialog):
"""A dialog to input proxy information
--
To view, visit https://gerrit.ovirt.org/40242
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide8dec4c91464a3c4a54a19890b3806b318f308a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
More information about the node-patches
mailing list