Dan Kenigsberg has posted comments on this change.

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


Patch Set 22:

(3 comments)

http://gerrit.ovirt.org/#/c/26718/22/vdsm_reg/deployUtil.py.in
File vdsm_reg/deployUtil.py.in:

Line 1666: 
Line 1667: def getRhevmCert(IP, port, output=None):
Line 1668:     """Aquire CA certificate from SSL handshake
Line 1669: 
Line 1670:     This certificate is saved to a file and later used
the "output" arg name confused me. Please mention that it's the path to where 
the certificate is to be saved.
Line 1671:     to validate HTTPS connections with the engine web
Line 1672:     server.
Line 1673: 
Line 1674:     """


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

Line 202:         else:
Line 203:             fOK = self.renameBridge()
Line 204:             logging.debug("execute: after renameBridge: %s", fOK)
Line 205: 
Line 206:         if fOK and os.path.exists(self.engineWebCACert) and \
you have a similar, but different, treatment for unmatching certs, depending on 
whether engineWebCACert pre-exists or not.

Please add a comment on why is that (I suppose that we do not want to delete an 
existing certificate, even if it does not match the fingerprint.

Why do you log and bail out in line 236, and not here? Defining a helper 
function might assist my understanding.
Line 207:             self.cfg_fprint != "None":
Line 208:             if self.cfg_fprint.upper() != \
Line 209:                     
deployUtil.generateFingerPrint(self.engineWebCACert):
Line 210:                 ovirtfunctions.ovirt_safe_delete_config(


Line 225:                 )
Line 226:                 if not download_certificate_succeeded and \
Line 227:                         self.cfg_fprint != "None":
Line 228:                     logging.info(
Line 229:                         "Cannot download Engine Web CA certificate 
file: " \
trailing backslashes are unnecessary within parenthesis.
Line 230:                         "%s:%s",
Line 231:                         self.vdcName,
Line 232:                         self.vdcPORT
Line 233:                     )


-- 
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to