Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: update firewall for all services
......................................................................
Patch Set 13:
(6 comments)
....................................................
File packaging/setup/plugins/ovirt-engine-setup/network/firewall_manager.py
Line 155: osetupcons.ConfigEnv.SUPPORTED_FIREWALL_MANAGERS
Line 156: ].split(',')
Line 157: ]
Line 158:
Line 159: self._detected_managers = []
I would put this initialization in constructor to avoid runtime error if for
some reason it will be referred by other stages without being enabled (called
this stage).
Also self._supported_managers = [] for the same reason, not sure why it should
be stored in state.
Line 160: for manager in self._supported_managers:
Line 161: if self.services.exists(manager):
Line 162: self._detected_managers.append(manager)
Line 163:
Line 160: for manager in self._supported_managers:
Line 161: if self.services.exists(manager):
Line 162: self._detected_managers.append(manager)
Line 163:
Line 164: if (osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in
new line before statement.
if (
xxx and
yyyy
)
in this case if line is too long:
if (
osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in (
self._detected_managers
) and
not self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE]
):
or:
if (
(
osetupcons.Const.FIREWALL_MANAGER_FIREWALLD in
self._detected_managers
) and
not self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE]
)
Line 165: self._detected_managers and
Line 166: not self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE]
Line 167: ):
Line 168: self._detected_managers.remove(
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]
I just removed this from supported before trying to detect.... because of this
I call it available and not supported.
not sure the dependency between service name and firewall is correct anyway...
if should be:
if IPTABLES in supported and service.exists('iptables'):
available.append(IPTABLES)
if FIREWALLD in supported and
self.environment[otopicons.NetEnv.FIREWALLD_AVAILABLE]:
available.append(FIREWALLD)
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
if not detected_manager put False within
self.environment[osetupcons.ConfigEnv.UPDATE_FIREWALL] and avoid this and
future conditions.
Line 175: ):
Line 176: self.dialog.note(
Line 177: text=_(
Line 178: 'Setup can automatically configure the firewall '
Line 210: def _customization(self):
Line 211: active_managers = []
Line 212: for manager in self._detected_managers:
Line 213: if self.services.status(manager):
Line 214: active_managers.append(manager)
once again this is wrong, see above, not sure why we need it twice.
at enable we can see if there are some detected, we do not care if active or
anything and here check of the active.
Line 215:
Line 216: available_managers = (
Line 217: active_managers if active_managers
Line 218: else self._detected_managers
Line 258: osetupcons.ConfigEnv.FIREWALL_MANAGER
Line 259: ] = manager
Line 260: break
Line 261:
Line 262: self.environment[otopicons.NetEnv.IPTABLES_ENABLE] = (
where do you check that whatever you got from answer file or cmdline matches
your capability?
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
