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

Alon Bar-Lev alonbl at redhat.com
Sat Apr 19 07:29:57 UTC 2014


Alon Bar-Lev has posted comments on this change.

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


Patch Set 12:

(5 comments)

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

Line 267:             os.chmod(pidfile, 0o664)
Line 268: 
Line 269:             if cfg_port and cfg_host != "None" \
Line 270:                     and not os.path.exists(CACert):
Line 271:                 if deployUtil.getRhevmCert(cfg_host, cfg_port):
I think we should modify this function to have optional parameter of output file, as we do not want to write anything to final location.
Line 272:                     if cfg_fprint != "None":
Line 273:                         engine_fprint = deployUtil.generateFingerPrint(CACert)
Line 274:                         with tempfile.NamedTemporaryFile(delete=False) as ftemp:
Line 275:                             ftemp.write(engine_fprint)


Line 275:                             ftemp.write(engine_fprint)
Line 276: 
Line 277:                         if cfg_fprint.upper() != engine_fprint.upper():
Line 278:                             log.error("Fingerprint provided in vdsm-reg" \
Line 279:                                       " differs from Engine")
and you must fail the entire process, quit vdsm-reg.
Line 280:                         else:
Line 281:                             os.rename(ftemp.name, CACert)
Line 282:                             ovirtfunctions.ovirt_store_config(CACert)
Line 283:                 else:


Line 277:                         if cfg_fprint.upper() != engine_fprint.upper():
Line 278:                             log.error("Fingerprint provided in vdsm-reg" \
Line 279:                                       " differs from Engine")
Line 280:                         else:
Line 281:                             os.rename(ftemp.name, CACert)
as with the sed, you cannot rename, you must read/write if already persisted.
Line 282:                             ovirtfunctions.ovirt_store_config(CACert)
Line 283:                 else:
Line 284:                     log.error("Cannot download Engine certificate file")
Line 285: 


Line 280:                         else:
Line 281:                             os.rename(ftemp.name, CACert)
Line 282:                             ovirtfunctions.ovirt_store_config(CACert)
Line 283:                 else:
Line 284:                     log.error("Cannot download Engine certificate file")
usually, short trivial block should be the first in conditionals, not important for now but for future.

however... if this is an error, and you cannot get the certificate and the certificate does not match the fingerprint, you must not proceed.
Line 285: 
Line 286:             itr = 0
Line 287:             while daemonize and not registered:
Line 288:                 oSetup = Setup(ticket=ticket)


Line 281:                             os.rename(ftemp.name, CACert)
Line 282:                             ovirtfunctions.ovirt_store_config(CACert)
Line 283:                 else:
Line 284:                     log.error("Cannot download Engine certificate file")
Line 285: 
let's start over... logic:

 if have certificate on filesystem
     if fingerprint provided
         if not matches requested fingerprint, remove it (unpersist, remove)
 if not have certificate on filesystem
     fetch certificate from host into temporary file (requires changing deployutils)
     if fails halt processing, so that we reach this at next iteration after timeout
     if fingerprint provided
         check fingerprint
         if not match halt vdsm-reg no sense to re-try
     move to final location and persist
Line 286:             itr = 0
Line 287:             while daemonize and not registered:
Line 288:                 oSetup = Setup(ticket=ticket)
Line 289:                 if oSetup.validateData():


-- 
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: 12
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