Thu, Mar 26, 2020 at 11:56:49AM CET, olichtne(a)redhat.com wrote:
On Tue, Mar 24, 2020 at 05:49:10PM +0100, Jan Tluka wrote:
> These mixins will be used to override the default Ping evaluator of the
> BaseEnrtRecipe class.
>
> The module provides VlanPingEvaluator mixin that will be used in all
> vlan-like (e.g. VLAN, VXLAN) tests where we try to ping from a vlan to
> another one that is meant to be unreachable.
>
> The module also defines PingCondition mixins that define test specific
> ping_endpoints_match() method. This method is used by VlanPingEvaluator
> to determine whether the ping should succeed or fail.
>
> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
> ---
> .../ENRT/PingMixins/PingConditionMixins.py | 34 +++++++++++++++++++
> .../ENRT/PingMixins/PingEvaluatorMixins.py | 15 ++++++++
> lnst/Recipes/ENRT/PingMixins/__init__.py | 9 +++++
> 3 files changed, 58 insertions(+)
> create mode 100644 lnst/Recipes/ENRT/PingMixins/PingConditionMixins.py
> create mode 100644 lnst/Recipes/ENRT/PingMixins/PingEvaluatorMixins.py
> create mode 100644 lnst/Recipes/ENRT/PingMixins/__init__.py
>
> diff --git a/lnst/Recipes/ENRT/PingMixins/PingConditionMixins.py
b/lnst/Recipes/ENRT/PingMixins/PingConditionMixins.py
> new file mode 100644
> index 00000000..2adb07bd
> --- /dev/null
> +++ b/lnst/Recipes/ENRT/PingMixins/PingConditionMixins.py
> @@ -0,0 +1,34 @@
> +class PingConditionMixin(object):
> + def ping_endpoints_match(self, src_addr, dst_addr):
> + pass
> +
> +class VlanPingConditionMixin(PingConditionMixin):
> + def ping_endpoints_match(self, src_addr, dst_addr):
> + host1, host2 = self.matched.host1, self.matched.host2
> + devs = []
> + for dev in (host1.devices + host2.devices):
> + if src_addr in dev.ips or dst_addr in dev.ips:
> + devs.append(dev)
> + try:
> + return devs[0].vlan_id == devs[1].vlan_id
> + except (IndexError, AttributeError):
> + return False
> +
> +class VirtualVlanPingConditionMixin(PingConditionMixin):
> + def ping_endpoints_match(self, src_addr, dst_addr):
> + guest1, guest2, guest3, guest4 = (self.matched.guest1,
> + self.matched.guest2, self.matched.guest3, self.matched.guest4)
I'm not sure I like this hard coded list of host references based on
where the class is defined. The number and names of the hosts is defined
by the recipe, but this is a mixin class that doesn't know what recipe
it will be mixed into - you could mix it into a recipe that just calls
the hosts something different and it won't work.
The method is named ping_ENDPOINTS_match, but the parameter names are
{src, dst}_address, maybe if we used some more generic concept of an
"Endpoint" we could pass in more than just the information about the ip
addresses but also about the configuration and whether or not the
endpoints are in the same vlan?
OR, if we can't do that, then that means that these conditions are
specific to the recipes that they are used in and they should therefore
be defined with their respective recipe, either as a helper class in the
same module, or just directly in the recipe itself...
What do you think? Does my thought process make sense or am I
misunderstanding something?
It makes sense.
Another approach could be following. We could add additional value that
is returned by generate_ping_endpoints() method along with the (src, dst)
to indicate whether they "match".
E.g. in case of VlansRecipe, that would be:
for src in [host1.vlan0, host1.vlan1, host1.vlan2]:
for dst in [host2.vlan0, host2.vlan1, host2.vlan2]:
match = (src.vlan_id == dst.vlan_id) # <-----------
result += [(src, dst, match)]
This way we can remove PingConditionMixins completely and just keep
the PingEvaluatorMixins that would override the get_ping_evaluators() and
use the 'match' value to decide which evaluator to return.
> +
> + matching_pairs = []
> + for pair in [(guest1, guest3), (guest2, guest4)]:
> + matching_pairs.extend([pair, pair[::-1]])
> +
> + devs = []
> + for dev in (guest1.devices + guest2.devices + guest3.devices +
> + guest4.devices):
> + if src_addr in dev.ips or dst_addr in dev.ips:
> + devs.append(dev)
> + try:
> + return (devs[0].host, devs[1].host) not in matching_pairs
> + except IndexError:
> + return False
> diff --git a/lnst/Recipes/ENRT/PingMixins/PingEvaluatorMixins.py
b/lnst/Recipes/ENRT/PingMixins/PingEvaluatorMixins.py
> new file mode 100644
> index 00000000..67f4fa56
> --- /dev/null
> +++ b/lnst/Recipes/ENRT/PingMixins/PingEvaluatorMixins.py
> @@ -0,0 +1,15 @@
> +from lnst.RecipeCommon.Ping.Evaluators import (
> + ZeroPassPingEvaluator, RatePingEvaluator)
> +
> +class PingEvaluatorMixin(object):
> + def get_ping_evaluators(self, ping_config):
> + pass
> +
> +class VlanPingEvaluatorMixin(PingEvaluatorMixin):
> + def get_ping_evaluators(self, ping_config):
> + endpoints_match = self.ping_endpoints_match(ping_config.client_bind,
> + ping_config.destination_address)
> + if endpoints_match:
> + return [RatePingEvaluator(min_rate=50)]
> + else:
> + return [ZeroPassPingEvaluator()]
> diff --git a/lnst/Recipes/ENRT/PingMixins/__init__.py
b/lnst/Recipes/ENRT/PingMixins/__init__.py
> new file mode 100644
> index 00000000..a4f2da09
> --- /dev/null
> +++ b/lnst/Recipes/ENRT/PingMixins/__init__.py
> @@ -0,0 +1,9 @@
> +from lnst.Recipes.ENRT.PingMixins.PingEvaluatorMixins import (
> + VlanPingEvaluatorMixin,
> + )
> +
> +from lnst.Recipes.ENRT.PingMixins.PingConditionMixins import (
> + VlanPingConditionMixin,
> + VirtualVlanPingConditionMixin,
> + )
> +
> --
> 2.21.1
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...