From: Aniss Loughlam aloughla@redhat.com
Signed-off-by: Aniss Loughlam aloughla@redhat.com --- lnst/Recipes/ENRT/BondRecipe.py | 111 ++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/lnst/Recipes/ENRT/BondRecipe.py b/lnst/Recipes/ENRT/BondRecipe.py index 07d7cfc..0941100 100644 --- a/lnst/Recipes/ENRT/BondRecipe.py +++ b/lnst/Recipes/ENRT/BondRecipe.py @@ -11,6 +11,33 @@ from lnst.Devices import BondDevice
class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, BaseEnrtRecipe): + """ + This recipe implements Enrt testing for a network scenario that looks + as follows + + .. code-block:: none + + .--------. + .----------------+ | + | .-------+ switch +-------. + | | '--------' | + .-------------------. | + | | bond0 | | | + | .---'--. .---'--. | .---'--. + .----|-| eth0 |-| eth1 |-|----. .----| eth0 |----. + | | '------' '------' | | | '------' | + | '-------------------' | | | + | | | | + | host1 | | host2 | + '-----------------------------' '----------------' + + All sub configurations are included via Mixin classes. + + The actual test machinery is implemented in the :any:`BaseEnrtRecipe` class. + """ + + + host1 = HostReq() host1.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver")) host1.eth1 = DeviceReq(label="net1", driver=RecipeParam("driver")) @@ -28,6 +55,10 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, miimon_value = IntParam(mandatory=True)
def test_wide_configuration(self): + """ + Test wide configuration for this recipe involves creating a bond0 + device using two slave device eth0 and eth1 on host1 + """ host1, host2 = self.matched.host1, self.matched.host2 host1.bond0 = BondDevice(mode=self.params.bonding_mode, miimon=self.params.miimon_value) @@ -53,6 +84,10 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, return configuration
def generate_test_wide_description(self, config): + """ + Test wide description is extended with the configured VLAN tunnels + and their IP addresses + """ host1, host2 = self.matched.host1, self.matched.host2 desc = super().generate_test_wide_description(config) desc += [ @@ -79,39 +114,115 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, return desc
def test_wide_deconfiguration(self, config): + "" # overriding the parent docstring del config.test_wide_devices
super().test_wide_deconfiguration(config)
def generate_ping_endpoints(self, config): + """ + The ping endpoints are host1.bond0 and host2.eht0 + """ return [PingEndpoints(self.matched.host1.bond0, self.matched.host2.eth0)]
def generate_perf_endpoints(self, config): + """ + The perf endpoints for this recipe are the host1.bond0 and host2.eth0 + + Returned as:: + + [(self.matched.host1.bond0, self.matched.host2.eth0)] + + """ return [(self.matched.host1.bond0, self.matched.host2.eth0)]
@property def offload_nics(self): + """ + The `offload_nics` property value for this scenario is a list of the physical + devices carrying data of the configured bond: + + | host1.eth0, host.eth1 + | host2.eth0 + + For detailed explanation of this property see :any:`OffloadSubConfigMixin` + class and :any:`OffloadSubConfigMixin.offload_nics`. + """ return [self.matched.host1.bond0, self.matched.host2.eth0]
@property def mtu_hw_config_dev_list(self): + """ + The `mtu_hw_config_dev_list` property value for this scenario is a + list of the configured bond device and the underlying physical + devices: + + | host1.eth0, host1.eth1, host1.bond0 + | host2.eth0 + + For detailed explanation of this property see :any:`MTUHWConfigMixin` + class and :any:`MTUHWConfigMixin.mtu_hw_config_dev_list`. + """ return [self.matched.host1.bond0, self.matched.host2.eth0]
@property def coalescing_hw_config_dev_list(self): + """ + The `mtu_hw_config_dev_list` property value for this scenario is a + is a list of the physical devices carrying data of the configured + bond: + + | host1.eth0, host.eth1 + | host2.eth0 + + For detailed explanation of this property see :any:`CoalescingHWConfigMixin` + class and :any:`CoalescingHWConfigMixin.coalescing_hw_config_dev_list`. + """ return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0]
@property def dev_interrupt_hw_config_dev_list(self): + """ + The `dev_interrupt_hw_config_dev_list` property value for this scenario + is a list of the physical devices carrying data of the configured bond: + + | host1.eth0, host.eth1 + | host2.eth0 + + For detailed explanation of this property see :any:`DevInterruptHWConfigMixin` + class and :any:`DevInterruptHWConfigMixin.dev_interrupt_hw_config_dev_list`. + """ return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0]
@property def parallel_stream_qdisc_hw_config_dev_list(self): + """ + The `parallel_stream_qdisc_hw_config_dev_list` property value for + this scenario is a list of the physical devices carrying data of the + configured bond: + + | host1.eth0, host.eth1 + | host2.eth0 + + For detailed explanation of this property see + :any:`ParallelStreamQDiscHWConfigMixin` class and + :any:`ParallelStreamQDiscHWConfigMixin.parallel_stream_qdisc_hw_config_dev_list`. + """ return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0]
@property def no_pause_frames_dev_list(self): + """ + The `no pause frames dev list` property value for this scenario is a list of + the physical devices carrying data of the configured bond: + + | host1.eth0, host1.eht1 + | host2.eth0 + + For detailed explanation of this property see + :any:`class DisablePauseFramesHWConfigMixin`. + """ return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0]
Thu, Apr 30, 2020 at 09:22:54PM CEST, aloughla@redhat.com wrote:
From: Aniss Loughlam aloughla@redhat.com
Signed-off-by: Aniss Loughlam aloughla@redhat.com
Few overall comments. I did not receive the first patch of the series. (the subject of the patch says 2/2). Could you confirm that you have sent the first patch of the series?
Maybe because of that I was not able to apply the patch:
Applying: docs:add Recipes.ENRT.BondRecipe documentation .git/rebase-apply/patch:16: trailing whitespace.
.git/rebase-apply/patch:18: trailing whitespace.
.git/rebase-apply/patch:34: trailing whitespace.
.git/rebase-apply/patch:49: trailing whitespace. device using two slave device eth0 and eth1 on host1 error: patch failed: lnst/Recipes/ENRT/BondRecipe.py:79 error: lnst/Recipes/ENRT/BondRecipe.py: patch does not apply
From the log you can see that you also have trailing whitespaces in the patch. These are unnecessary whitespace characters at the end of a line. Please add a checker in your development environment (or simply add a syntax highlight for these patterns) to avoid this in future.
lnst/Recipes/ENRT/BondRecipe.py | 111 ++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/lnst/Recipes/ENRT/BondRecipe.py b/lnst/Recipes/ENRT/BondRecipe.py index 07d7cfc..0941100 100644 --- a/lnst/Recipes/ENRT/BondRecipe.py +++ b/lnst/Recipes/ENRT/BondRecipe.py @@ -11,6 +11,33 @@ from lnst.Devices import BondDevice
class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, BaseEnrtRecipe):
- """
- This recipe implements Enrt testing for a network scenario that looks
- as follows
- .. code-block:: none
.--------.
.----------------+ |
| .-------+ switch +-------.
| | '--------' |
.-------------------. |
| | bond0 | | |
| .---'--. .---'--. | .---'--.
.----|-| eth0 |-| eth1 |-|----. .----| eth0 |----.
| | '------' '------' | | | '------' |
| '-------------------' | | |
| | | |
| host1 | | host2 |
'-----------------------------' '----------------'
- All sub configurations are included via Mixin classes.
- The actual test machinery is implemented in the :any:`BaseEnrtRecipe` class.
- """
- host1 = HostReq() host1.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver")) host1.eth1 = DeviceReq(label="net1", driver=RecipeParam("driver"))
@@ -28,6 +55,10 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, miimon_value = IntParam(mandatory=True)
def test_wide_configuration(self):
"""
Test wide configuration for this recipe involves creating a bond0
device using two slave device eth0 and eth1 on host1
"""
I believe there's much more here to document. Please check the SimpleNetworkRecipe and VlansRecipe to get the idea.
Be more verbose about what is being configured, e.g. do not mention bond0 because the reader does not know what it is. So, "involves creating a bonding device ..." is better.
Also, there are important test parameters that user should be aware of: bonding_mode and miimon parameter!
Finally as in other already documented recipes, please include the IP address configuration.
host1, host2 = self.matched.host1, self.matched.host2 host1.bond0 = BondDevice(mode=self.params.bonding_mode, miimon=self.params.miimon_value)
@@ -53,6 +84,10 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, return configuration
def generate_test_wide_description(self, config):
"""
Test wide description is extended with the configured VLAN tunnels
and their IP addresses
""" host1, host2 = self.matched.host1, self.matched.host2 desc = super().generate_test_wide_description(config) desc += [
@@ -79,39 +114,115 @@ class BondRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin, return desc
def test_wide_deconfiguration(self, config):
"" # overriding the parent docstring
This should not be necessary anymore. I sent a patch for the sphinx configuration that disables automatic inclusion of parent methods docstrings.
Hmm, compared to the VlansRecipe doc, you can improve on that, too. Check the VlansRecipe and do similar here.
del config.test_wide_devices super().test_wide_deconfiguration(config) def generate_ping_endpoints(self, config):
"""
The ping endpoints are host1.bond0 and host2.eht0
"""
You have a typo in host2.eth0.
return [PingEndpoints(self.matched.host1.bond0, self.matched.host2.eth0)] def generate_perf_endpoints(self, config):
"""
The perf endpoints for this recipe are the host1.bond0 and host2.eth0
Returned as::
[(self.matched.host1.bond0, self.matched.host2.eth0)]
"""
I'd be more reader-friendly here. The docstring you wrote is actually what user can read from the code (but unfortunately does not provide any useful information) and just copies the return value.
The documentation should help the reader to understand the scenario without reading the code.
In this case I'd write: "The perf endpoints for this recipe are the bonding device on the first host and the matched ethernet device on the second host."
return [(self.matched.host1.bond0, self.matched.host2.eth0)] @property def offload_nics(self):
"""
The `offload_nics` property value for this scenario is a list of the physical
devices carrying data of the configured bond:
| host1.eth0, host.eth1
| host2.eth0
For detailed explanation of this property see :any:`OffloadSubConfigMixin`
class and :any:`OffloadSubConfigMixin.offload_nics`.
"""
Based on the docstring the device list is only host1.eth0 and host1.eth1. The host2.eth0 does not fit the description. Notice also the typo you have in host.eth1, should be host1.eth1.
I wouldn't use the "carrying" here (yeah, I know I used it in VlansRecipe) but instead I'd simplify to:
"is a list of the physical NICs bonded by the configured bonding device on the first host and the matched ethernet device on the second host"
return [self.matched.host1.bond0, self.matched.host2.eth0] @property def mtu_hw_config_dev_list(self):
"""
The `mtu_hw_config_dev_list` property value for this scenario is a
list of the configured bond device and the underlying physical
devices:
| host1.eth0, host1.eth1, host1.bond0
| host2.eth0
For detailed explanation of this property see :any:`MTUHWConfigMixin`
class and :any:`MTUHWConfigMixin.mtu_hw_config_dev_list`.
"""
How about "... is a list containing the configured bond device ..." ?
I'm proposing this since "... a list of the configured bond device ..." sounds a bit odd to me. But not sure, as I'm not the native speaker.
return [self.matched.host1.bond0, self.matched.host2.eth0] @property def coalescing_hw_config_dev_list(self):
"""
The `mtu_hw_config_dev_list` property value for this scenario is a
is a list of the physical devices carrying data of the configured
bond:
| host1.eth0, host.eth1
| host2.eth0
For detailed explanation of this property see :any:`CoalescingHWConfigMixin`
class and :any:`CoalescingHWConfigMixin.coalescing_hw_config_dev_list`.
"""
Same comment as for the offload_nics property.
return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0] @property def dev_interrupt_hw_config_dev_list(self):
"""
The `dev_interrupt_hw_config_dev_list` property value for this scenario
is a list of the physical devices carrying data of the configured bond:
| host1.eth0, host.eth1
| host2.eth0
For detailed explanation of this property see :any:`DevInterruptHWConfigMixin`
class and :any:`DevInterruptHWConfigMixin.dev_interrupt_hw_config_dev_list`.
"""
Same comment as for the offload_nics property.
return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0] @property def parallel_stream_qdisc_hw_config_dev_list(self):
"""
The `parallel_stream_qdisc_hw_config_dev_list` property value for
this scenario is a list of the physical devices carrying data of the
configured bond:
| host1.eth0, host.eth1
| host2.eth0
For detailed explanation of this property see
:any:`ParallelStreamQDiscHWConfigMixin` class and
:any:`ParallelStreamQDiscHWConfigMixin.parallel_stream_qdisc_hw_config_dev_list`.
"""
Same comment as for the offload_nics property.
return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0] @property def no_pause_frames_dev_list(self):
"""
The `no pause frames dev list` property value for this scenario is a list of
the physical devices carrying data of the configured bond:
| host1.eth0, host1.eht1
| host2.eth0
For detailed explanation of this property see
:any:`class DisablePauseFramesHWConfigMixin`.
"""
Same comment as for the offload_nics property.
return [self.matched.host1.eth0, self.matched.host1.eth1, self.matched.host2.eth0]
-- 2.24.1 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@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.fedorahos...
lnst-developers@lists.fedorahosted.org