Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: fixing remote engine pki common code usage on upgrades ......................................................................
Patch Set 1: (5 comments) http://gerrit.ovirt.org/#/c/33786/1/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-reports/pki/apache.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-reports/pki/apache.py: Line 81 Line 82 Line 83 Line 84 Line 85 I'd simply move this line, Line 93 Line 94 Line 95 Line 96 Line 97 to here Line 173 Line 174 Line 175 Line 176 Line 177 > On upgrades self._enrolldata will be none cause certs are already in place. Indeed, sorry... Thanks! But as I commented on jboss.py, enroll_data was supposed to be callable "unconditionally", so this could be fixed also by moving the call to outside of 'if not engine_apache_pki_found' and just keep there the special treatment of the ca cert. If, OTOH, you think I was wrong in thinking that the common code should also take care of the decision whether to run it or not, and better to keep all these checks in the plugins using it (which might make sense), you can also remove the checks there. Line 128: oreportscons.FileLocations. Line 129: OVIRT_ENGINE_PKI_REPORTS_APACHE_CA_CERT Line 130: ) Line 131: else: Line 132: self._enabled = False instead of this. Line 133: Line 134: tries_left = 30 Line 135: while ( Line 136: self._need_ca_cert and http://gerrit.ovirt.org/#/c/33786/1/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-reports/pki/jboss.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine-reports/pki/jboss.py: Line 81: os.path.exists( Line 82: oreportscons.FileLocations.OVIRT_ENGINE_PKI_REPORTS_JBOSS_KEY Line 83: ) and os.path.exists( Line 84: oreportscons.FileLocations.OVIRT_ENGINE_PKI_REPORTS_JBOSS_CERT Line 85: ) Is this really needed? The common code in enroll_cert already does this. Apache is a special case, because it also needs the ca cert. It might be possible to remove some checks there as well, not sure. Line 86: ) Line 87: Line 88: if not reports_jboss_pki_found: Line 89: self._enrolldata = remote_engine.EnrollCert( -- To view, visit http://gerrit.ovirt.org/33786 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I011db10e16bb931c1610bc6645410917e722218d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-reports Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Lev Veyde <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Shirly Radco <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yaniv Dary <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
