Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: Adding a dialog to let the user review iptables changes ......................................................................
Patch Set 1: (8 comments) http://gerrit.ovirt.org/#/c/33085/1/packaging/setup/ovirt_engine_setup/constants.py File packaging/setup/ovirt_engine_setup/constants.py: Line 380: def UPDATE_FIREWALL(self): Line 381: return 'OVESETUP_CONFIG/updateFirewall' Line 382: Line 383: FIREWALL_MANAGERS = 'OVESETUP_CONFIG/firewallManagers' Line 384: SKIP_FIREWALL_REVIEW = 'OVESETUP_CONFIG/skipFirewallReview' I'd call it something like IPTABLES_CHANGES_CONFIRMED Also perhaps keep in answer file Line 385: VALID_FIREWALL_MANAGERS = 'OVESETUP_CONFIG/validFirewallManagers' Line 386: FQDN_REVERSE_VALIDATION = 'OVESETUP_CONFIG/fqdnReverseValidation' Line 387: FQDN_NON_LOOPBACK_VALIDATION = 'OVESETUP_CONFIG/fqdnNonLoopback' Line 388: http://gerrit.ovirt.org/#/c/33085/1/packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py File packaging/setup/plugins/ovirt-engine-common/base/network/firewall_manager_iptables.py: Line 51: class _IpTablesManager(firewall_manager_base.FirewallManagerBase): Line 52: Line 53: _SERVICE = 'iptables' Line 54: Line 55: _REDHAT_IPTABLES = '/etc/sysconfig/iptables' You already have SYSCONFIG_IPTABLES Line 56: Line 57: def _get_rules(self): Line 58: if self._rules is None: Line 59: self._rules = outil.processTemplate( Line 138: fromfile=_('current'), Line 139: tofile=_('proposed'), Line 140: ) Line 141: for line in diff: Line 142: diffl += line Perhaps: diffl = ''.join(diff) Or diffl = reduce(lambda x, y: '%s%s' % (x, y), diff) Line 143: if len(diffl) > 0: Line 144: confirmed = dialog.queryBoolean( Line 145: dialog=self.plugin.dialog, Line 146: name='OVESETUP_RPMDISTRO_REQUIRE_ROLLBACK', Line 140: ) Line 141: for line in diff: Line 142: diffl += line Line 143: if len(diffl) > 0: Line 144: confirmed = dialog.queryBoolean( save to env, so that it's in the answer file Line 145: dialog=self.plugin.dialog, Line 146: name='OVESETUP_RPMDISTRO_REQUIRE_ROLLBACK', Line 147: note=_( Line 148: 'Generated iptables rules diverge from current ones.\n' Line 142: diffl += line Line 143: if len(diffl) > 0: Line 144: confirmed = dialog.queryBoolean( Line 145: dialog=self.plugin.dialog, Line 146: name='OVESETUP_RPMDISTRO_REQUIRE_ROLLBACK', Change the name after copying :-) Line 147: note=_( Line 148: 'Generated iptables rules diverge from current ones.\n' Line 149: 'Please review the changes:\n\n' Line 150: '{diff}\n\n' Line 144: confirmed = dialog.queryBoolean( Line 145: dialog=self.plugin.dialog, Line 146: name='OVESETUP_RPMDISTRO_REQUIRE_ROLLBACK', Line 147: note=_( Line 148: 'Generated iptables rules diverge from current ones.\n' diverge? I am not a native English speaker but it sounds a bit weird. I'd just say "are different". Also consider checking actual ones (output of iptables-save). Line 149: 'Please review the changes:\n\n' Line 150: '{diff}\n\n' Line 151: 'Do you want to proceed with firewall configuration? ' Line 152: '(@VALUES@) [@DEFAULT@]: ' http://gerrit.ovirt.org/#/c/33085/1/packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py File packaging/setup/plugins/ovirt-engine-setup/base/network/firewall_manager.py: Line 238: ), Line 239: ) Line 240: def _review_config(self): Line 241: if not self.environment[ Line 242: osetupcons.ConfigEnv.SKIP_FIREWALL_REVIEW I'd not check it here but only in iptables plugin Line 243: ]: Line 244: for manager in self.environment[ Line 245: osetupcons.ConfigEnv.FIREWALL_MANAGERS Line 246: ]: Line 242: osetupcons.ConfigEnv.SKIP_FIREWALL_REVIEW Line 243: ]: Line 244: for manager in self.environment[ Line 245: osetupcons.ConfigEnv.FIREWALL_MANAGERS Line 246: ]: All of them? Why not only self.environment[osetupcons.ConfigEnv.FIREWALL_MANAGER] ? Line 247: manager.review_config() Line 248: Line 249: @plugin.event( Line 250: stage=plugin.Stages.STAGE_MISC, -- To view, visit http://gerrit.ovirt.org/33085 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I63e0eeb26d925c8c79b9c8e55da64c57ce94a3f6 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Lev Veyde <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[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
