Alon Bar-Lev has posted comments on this change. Change subject: packaging: setup: update firewall for all services ......................................................................
Patch Set 13: (3 comments) .................................................... File packaging/setup/plugins/ovirt-engine-setup/network/firewall_manager.py Line 162: self._detected_managers.append(manager) Line 163: Line 164: if (osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in Line 165: self._detected_managers and Line 166: not self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE] each manager should have its own detection algorithm, please do not relay on service names as general test, it is only available for iptables as firewalld is already reported by otopi. so if you want to do this right, without 'hard coded' tests, as there actually there anyway... but having the following will enable it to be constraint. class _Manager(object): def __init__(self, plugin): self._plugin = plugin @property def name(self): raise RuntimeError('Unset') def detect(self): return False def active(self): return False class _IpTables(_Manager): _SERVICE = 'iptables' def __init__(self, plugin): super(IpTables, self).__init__(plugin) @property def name(self): return 'iptables' detect(self): return self._plugin.services.exists(self._SERVICE) active(self): return self._plugin.services.status(self._SERVICE) class _Firewalld(_Manager): def __init__(self, plugin): super(IpTables, self).__init__(plugin) @property def name(self): return 'iptables' detect(self): return self._plugin.environment(....) active(self): return self._plugin.services.status(self._SERVICE) _MANAGERS = [ _IpTables, _Firewalld, ] Now use _MANAGERS to perform the detection and activation. Line 167: ): Line 168: self._detected_managers.remove( Line 169: osetupcons.Const.FIREWALL_MANAGER_FIREWALLD Line 170: ) Line 170: ) Line 171: Line 172: if ( Line 173: self.environment[osetupcons.ConfigEnv.UPDATE_FIREWALL] is None and Line 174: len(self._detected_managers) > 0 no point to ask if no manager, if no manager is as if no update of firewall, this style prevents conditionals in code. Line 175: ): Line 176: self.dialog.note( Line 177: text=_( Line 178: 'Setup can automatically configure the firewall ' Line 258: osetupcons.ConfigEnv.FIREWALL_MANAGER Line 259: ] = manager Line 260: break Line 261: Line 262: self.environment[otopicons.NetEnv.IPTABLES_ENABLE] = ( If we do no set firewall, we do not care what manager is set. If we are to set firewall then the manager must meet something we actually can work with. Please give examples of where we ignore invalid settings when actually using them. Line 263: self.environment[ Line 264: osetupcons.ConfigEnv.FIREWALL_MANAGER Line 265: ] == osetupcons.Const.FIREWALL_MANAGER_IPTABLES Line 266: ) -- To view, visit http://gerrit.ovirt.org/20737 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If3c1a634b2e8539ebd604205b5487290c8d8a1a9 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches