Alon Bar-Lev has posted comments on this change.
Change subject: WIP: packaging: setup: host rename
......................................................................
Patch Set 11: (21 inline comments)
First quick pass.
....................................................
File packaging/bin/pki-pkcs12-nopasskey.sh
Line 1: #!/bin/sh
why do we need this script? anyway, if we do, please also extract the
certificate, and get as parameter the location you want to extract the key and
certificate.
Also, please call this pki-pkcs12-extract or something... and if output
password is empty then you extract without password.
And... do this in separate patch, as it is actually pki magic.
Line 2:
Line 3: extractkey() {
Line 4: local name="$1"
Line 5: local pass="$2"
....................................................
File packaging/setup/bin/ovirt-engine-rename
Line 31: Load configuration files.
Line 32: --config-append=file
Line 33: Load extra configuration files.
Line 34: --generate-answer=file
Line 35: Generate answer file.
I suggest adding --newname= or similar here.
Line 36:
Line 37: __EOF__
Line 38: exit 1
Line 39: }
....................................................
File packaging/setup/ovirt_engine_setup/constants.py
Line 975: summary=True,
Line 976: description=_('New FQDN'),
Line 977: )
Line 978: def FQDN(self):
Line 979: return 'OSETUP_RENAME/fqdn'
I think there must be space here, please run ./packaging/check
Line 980: CONFIRM_FORCE_OVERWRITE = 'OSETUP_RENAME/confirmForceOverwrite'
Line 981:
Line 982:
Line 983: @util.export
....................................................
File packaging/setup/plugins/ovirt-engine-common/core/misc.py
Line 48: ],
Line 49: )
Line 50: def _preinit(self):
Line 51: self.environment.setdefault(
Line 52: otopicons.CoreEnv.LOG_DIR,
I did not want to put it in common, as we may use the common for other cli. But
not that important right now.
Line 53: osetupcons.FileLocations.OVIRT_SETUP_LOGDIR
Line 54: )
Line 55: self.environment.setdefault(
Line 56: otopicons.CoreEnv.CONFIG_FILE_NAME,
....................................................
File packaging/setup/plugins/ovirt-engine-rename/core/database.py
Line 55: except RuntimeError:
Line 56: self.logger.debug("Option {option} not found in db".format(
Line 57: option=option
Line 58: )
Line 59: return
please do not return in middle of function.
Line 60:
Line 61: pattern=r"""
Line 62: (?P<proto>[a-z]+)
Line 63: ://
Line 57: option=option
Line 58: )
Line 59: return
Line 60:
Line 61: pattern=r"""
please compile patterns and set these at class level, see other places in code
for example.
but why not use[1]?
[1] http://docs.python.org/2/library/urlparse.html
Line 62: (?P<proto>[a-z]+)
Line 63: ://
Line 64: (?P<host>[a-zA-Z0-9.-]+)
Line 65: :
Line 72: if not match:
Line 73: self.logger.debug("Option {option} cannot be
parsed".format(
Line 74: option=option
Line 75: )
Line 76: return
please do not return at middle of functions.
Line 77:
Line 78: newfqdn = self.environment[osetupcons.RenameEnv.FQDN]
Line 79: replacement_pattern = '\g<proto>://' + newfqdn +
':\g<port>/\g<appname>'
Line 80: newvalue = match.expand(replacement_pattern)
Line 76: return
Line 77:
Line 78: newfqdn = self.environment[osetupcons.RenameEnv.FQDN]
Line 79: replacement_pattern = '\g<proto>://' + newfqdn +
':\g<port>/\g<appname>'
Line 80: newvalue = match.expand(replacement_pattern)
please construct url, do not perform re magic.
Line 81: self.environment[osetupcons.DBEnv.STATEMENT].updateVdcOptions(
Line 82: options=[
Line 83: {
Line 84: 'name': option,
Line 78: newfqdn = self.environment[osetupcons.RenameEnv.FQDN]
Line 79: replacement_pattern = '\g<proto>://' + newfqdn +
':\g<port>/\g<appname>'
Line 80: newvalue = match.expand(replacement_pattern)
Line 81: self.environment[osetupcons.DBEnv.STATEMENT].updateVdcOptions(
Line 82: options=[
please use tuple here as well
Line 83: {
Line 84: 'name': option,
Line 85: 'value': newvalue,
Line 86: },
....................................................
File packaging/setup/plugins/ovirt-engine-rename/core/engine.py
Line 53: )
Line 54: self.services.startup(
Line 55: name=osetupcons.Const.ENGINE_SERVICE_NAME,
Line 56: state=True,
Line 57: )
please do not change engine startup in rename
Line 58:
Line 59:
....................................................
File packaging/setup/plugins/ovirt-engine-rename/core/misc.py
Line 80: # TODO check resolve?
Line 81:
Line 82: @plugin.event(
Line 83: stage=plugin.Stages.STAGE_CLOSEUP,
Line 84: before=[
tuple
Line 85: osetupcons.Stages.DIALOG_TITLES_E_SUMMARY,
Line 86: ],
Line 87: after=[
Line 88: osetupcons.Stages.DIALOG_TITLES_S_SUMMARY,
....................................................
File packaging/setup/plugins/ovirt-engine-rename/core/pki.py
Line 67: osetupcons.Defaults.DEFAULT_PKI_COUNTRY
Line 68: )
Line 69: self.environment.setdefault(
Line 70: osetupcons.PKIEnv.ORG,
Line 71: None
why do you need the country and org? we should take it from existing
certificate not customizable.
Line 72: )
Line 73:
Line 74: @plugin.event(
Line 75: stage=plugin.Stages.STAGE_SETUP,
Line 126: ).format(
Line 127: aia=authorityInfoAccess,
Line 128: ),
Line 129: )
Line 130: if not dialog.queryBoolean(
we need environment for this too... and prompt only if it none... so we can
bypass this.
Line 131: dialog=self.dialog,
Line 132: name='OVESETUP_RENAME_AIA_BYPASS',
Line 133: note=_('Do you want to continue? (@VALUES@)
[@DEFAULT@]: '),
Line 134: prompt=True,
Line 166: osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_KEY,
Line 167: osetupcons.FileLocations.OVIRT_ENGINE_PKI_APACHE_CERT,
Line 168:
osetupcons.FileLocations.OVIRT_ENGINE_PKI_CERT_TEMPLATE[
Line 169: :-len('.in')],
Line 170: osetupcons.FileLocations.OVIRT_ENGINE_PKI_CERT_CONF
isn't that shared between setup?
Line 171: )
Line 172: )
Line 173:
Line 174: localtransaction = transaction.Transaction()
Line 189: self.environment[
Line 190: osetupcons.RenameEnv.FQDN
Line 191: ],
Line 192: self.environment[
Line 193: osetupcons.ConfigEnv.HTTP_PORT
this should be PUBLIC_HTTP_PORT now
Line 194: ],
Line 195: )
Line 196: }
Line 197: ),
Line 243: os.chown(
Line 244: f['name'],
Line 245: osetuputil.getUid(f['owner']),
Line 246: -1
Line 247: )
if owner is none in all the above, we really don't need the owner... and I do
think that the pki scripts sets 0600.
we can probably remove this set mode from setup as well, I will create a patch.
Line 248:
Line 249: @plugin.event(
Line 250: stage=plugin.Stages.STAGE_CLOSEUP,
Line 251: before=[
Line 254: after=[
Line 255: osetupcons.Stages.DIALOG_TITLES_S_SUMMARY,
Line 256: ],
Line 257: )
Line 258: def _closeup(self):
no need, the ca was not changed.
Line 259: rc, stdout, stderr = self.execute(
Line 260: (
Line 261: self.command.get('openssl'),
Line 262: 'x509',
....................................................
File packaging/setup/plugins/ovirt-engine-rename/core/protocols.py
Line 79: )
Line 80: self.environment.setdefault(
Line 81: osetupcons.ConfigEnv.JBOSS_DIRECT_HTTPS_PORT,
Line 82: None
Line 83: )
why do you need the defaults? if something is missing in the engine.conf.d then
we screwed and should abort.
Line 84:
Line 85: @plugin.event(
Line 86: stage=plugin.Stages.STAGE_SETUP,
Line 87: )
Line 135: )
Line 136: def _misc(self):
Line 137: def flag(o):
Line 138: return 'true' if o else 'false'
Line 139: content = (
I don't understand... why not just read the file, change the FQDN and write it
back?
Line 140: 'ENGINE_FQDN={fqdn}\n'
Line 141: 'ENGINE_PROXY_ENABLED={proxyFlag}\n'
Line 142: 'ENGINE_PROXY_HTTP_PORT={proxyHttpPort}\n'
Line 143: 'ENGINE_PROXY_HTTPS_PORT={proxyHttpsPort}\n'
Line 214: self.dialog.note(
Line 215: text=_(
Line 216: 'Web access is enabled at:\n'
Line 217: ' http://{fqdn}:{httpPort}{engineURI}\n'
Line 218: ' https://{fqdn}:{httpsPort}{engineURI}\n'
nothing has been changed... no need to print this.
Line 219: ).format(
Line 220: fqdn=self.environment[osetupcons.RenameEnv.FQDN],
Line 221: httpPort=self.environment[
Line 222: osetupcons.ConfigEnv.PUBLIC_HTTP_PORT
Line 227: engineURI=engineURI,
Line 228: )
Line 229: )
Line 230:
Line 231: if self.environment[osetupcons.CoreEnv.DEVELOPER_MODE]:
no need.
Line 232: self.dialog.note(
Line 233: text=_(
Line 234: 'JBoss is listening for debug connection at:
{address}'
Line 235: ).format(
--
To view, visit http://gerrit.ovirt.org/17408
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I28cb0285424236fd3e6907694f6bf1ce6ce3367f
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
