Change in vdsm[master]: vdsm_config: move download certificate

dougsland at redhat.com dougsland at redhat.com
Thu Apr 24 06:08:22 UTC 2014


Douglas Schilling Landgraf has posted comments on this change.

Change subject: vdsm_config: move download certificate
......................................................................


Patch Set 13:

(5 comments)

http://gerrit.ovirt.org/#/c/26718/13/vdsm_reg/vdsm-reg-setup.in
File vdsm_reg/vdsm-reg-setup.in:

Line 262
Line 263
Line 264
Line 265
Line 266
> the entire above code should go into this loop to allow retry, this is what
Done


Line 281:             if cfg_fprint.upper() != engine_fprint.upper():
Line 282:                 ovirtfunctions.ovirt_safe_delete_config(CACert)
Line 283:                 os.remove(CACert)
Line 284: 
Line 285:         if cfg_port and cfg_host != "None" and cfg_fprint != "None" \
> I do not understand... if cfg_host is "None" then you need to quit vdsm-reg
Done
Line 286:                 and not os.path.exists(CACert):
Line 287:             with tempfile.NamedTemporaryFile(delete=False) as ftemp:
Line 288:                 if not deployUtil.getRhevmCert(
Line 289:                     IP=cfg_host,


Line 283:                 os.remove(CACert)
Line 284: 
Line 285:         if cfg_port and cfg_host != "None" and cfg_fprint != "None" \
Line 286:                 and not os.path.exists(CACert):
Line 287:             with tempfile.NamedTemporaryFile(delete=False) as ftemp:
> why don't you use mkstemp? where do you delete the temp file in case of an 
I used it  in the benefit of with statement and not persisting the file but mkstemp is more secure, let's use it.
Line 288:                 if not deployUtil.getRhevmCert(
Line 289:                     IP=cfg_host,
Line 290:                     port=cfg_port,
Line 291:                     output=ftemp.name


Line 290:                     port=cfg_port,
Line 291:                     output=ftemp.name
Line 292:                 ):
Line 293:                     log.error("Cannot download Engine certificate file")
Line 294:                     sys.exit(1)
> this should wait for next iteration, to keep trying download
Done
Line 295: 
Line 296:             engine_fprint = deployUtil.generateFingerPrint(ftemp.name)
Line 297:             if cfg_fprint != "None":
Line 298:                 if cfg_fprint.upper() != engine_fprint.upper():


Line 299:                     log.error(
Line 300:                         "Fingerprint in vdsm-reg.conf differs from Engine"
Line 301:                     )
Line 302:                     sys.exit(1)
Line 303:                 else:
> no need else after exit
Done
Line 304:                     shutil.move(ftemp.name, CACert)
Line 305:                     ovirtfunctions.ovirt_store_config(CACert)
Line 306: 
Line 307:         itr = 0


-- 
To view, visit http://gerrit.ovirt.org/26718
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c025eedd2be92b9418ddbe01efc02c913af2a7
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list