Sandro Bonazzola has posted comments on this change.

Change subject: hosted-engine: WIP - allow to deploy hosted-engine
......................................................................


Patch Set 3:

(7 comments)

https://gerrit.ovirt.org/#/c/38547/3/src/plugins/ovirt-host-deploy/hosted-engine/configureha.py
File src/plugins/ovirt-host-deploy/hosted-engine/configureha.py:

Line 72:             odeploycons.HostedEngineEnv.STORAGE_DOMAIN_CONNECTION,
Line 73:             odeploycons.HostedEngineEnv.USE_SSL,
Line 74:             odeploycons.HostedEngineEnv.VM_UUID,
Line 75:         ):
Line 76:             self.environment.setdefault(env, '')
> None is for unset
Done
Line 77: 
Line 78:     @plugin.event(
Line 79:         stage=plugin.Stages.STAGE_PROGRAMS,
Line 80:     )


Line 146:         self.logger.info(_('Updating hosted-engine configuration'))
Line 147:         conf = {
Line 148:             'bridge': self.environment[
Line 149:                 odeploycons.VdsmEnv.MANAGEMENT_BRIDGE_NAME
Line 150:             ],
> this won't be set
I'll add a hosted engine env then. it's needed by the HA daemons.
Line 151:             'ca_cert': os.path.join(
Line 152:                 odeploycons.FileLocations.VDSM_TRUST_STORE,
Line 153:                 odeploycons.FileLocations.VDSM_SPICE_CA_FILE
Line 154:             ),


Line 149:                 odeploycons.VdsmEnv.MANAGEMENT_BRIDGE_NAME
Line 150:             ],
Line 151:             'ca_cert': os.path.join(
Line 152:                 odeploycons.FileLocations.VDSM_TRUST_STORE,
Line 153:                 odeploycons.FileLocations.VDSM_SPICE_CA_FILE
> why spice?
see below
Line 154:             ),
Line 155:             'ca_subject': '"{subject}"'.format(
Line 156:                 subject=self.environment[
Line 157:                     odeploycons.HostedEngineEnv.SPICE_SUBJECT


Line 154:             ),
Line 155:             'ca_subject': '"{subject}"'.format(
Line 156:                 subject=self.environment[
Line 157:                     odeploycons.HostedEngineEnv.SPICE_SUBJECT
Line 158:                 ],
> why do you care what ca subject is?
It's used by "hosted-engine --console" for calling
  /usr/bin/remote-viewer \
  --spice-ca-file=${ca_cert} \
  spice://localhost?tls-port=5900 \
  --spice-host-subject="${ca_subject}"
Line 159:             ),  # TODO: check if it's already passed for VDSM 
configuration
Line 160:             'connectionUUID': self.environment[
Line 161:                 odeploycons.HostedEngineEnv.CONNECTION_UUID
Line 162:             ],


Line 201:             ],
Line 202:             'sdUUID': self.environment[
Line 203:                 odeploycons.HostedEngineEnv.SD_UUID
Line 204:             ],
Line 205:             'service_start_time': 
odeploycons.Const.HOSTED_ENGINE_START_TIME,
> so you overwrite your own configuration each time you start?!?!
I don't think so, Martin, looking at the HA code it seems that this parameter 
is not used anymore. Can you confirm it so I'll remove it?
Line 206:             'spUUID': self.environment[
Line 207:                 odeploycons.HostedEngineEnv.SP_UUID
Line 208:             ],
Line 209:             'storage': self.environment[


Line 246:         self.logger.info(_('Enabling and starting HA services'))
Line 247:         for service in (
Line 248:             odeploycons.Const.HA_AGENT_SERVICE,
Line 249:             odeploycons.Const.HA_BROCKER_SERVICE,
Line 250:         ):
> this should be in packages
I'll move.
Line 251:             self.services.startup(
Line 252:                 name=service,
Line 253:                 state=True,
Line 254:             )


https://gerrit.ovirt.org/#/c/38547/3/src/plugins/ovirt-host-deploy/hosted-engine/configureqemu.py
File src/plugins/ovirt-host-deploy/hosted-engine/configureqemu.py:

Line 61:                 old_content = f.read().splitlines()
Line 62:         new_content = []
Line 63:         new_conf = 'lock_manager="sanlock"'
Line 64:         found = False
Line 65:         for line in old_content:
> I would expect a comment here explaining what this part does.
Done
Line 66:             match = self.RE_LOCK_MANAGER.match(line)
Line 67:             if match:
Line 68:                 found = True
Line 69:                 self.logger.debug(


-- 
To view, visit https://gerrit.ovirt.org/38547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia07992ccab2f745879c8d3d777e45b524bbdf6f8
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-host-deploy
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to